On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> wrote:
>> This PR contains the API and implementation changes for JEP-412 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/412 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address first batch of review comments src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java line 40: > 38: > 39: private IllegalNativeAccessChecker(Set<String> allowedModuleNames, > boolean allowAllUnnamedModules) { > 40: this.allowedModuleNames = > Collections.unmodifiableSet(allowedModuleNames); Should that be Set.copyOf() to take advantage of the immutability of `SetN` (but at the expense of additional copying)... src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java line 78: > 76: int index = 0; > 77: // the system property is removed after decoding > 78: String value = getAndRemoveProperty(prefix + index); I am not sure what is going on with the removal of the properties, but if I'm not mistaken this is racy: from the implementation of the checker() method above, it looks as if two different threads could trigger a call to the decode() function concurrently, which can result in a random partitioning of the properties against the two checkers being instantiated, with one of them being eventually set as the system-wide checker. ------------- PR: https://git.openjdk.java.net/jdk/pull/3699
