On Tue, 2 Dec 2025 16:43:44 GMT, Artur Barashev <[email protected]> wrote:
>> Francisco Ferrari Bihurriet has updated the pull request incrementally with >> two additional commits since the last revision: >> >> - Address review comments >> - Slightly improve ConfigFileTestDirPermissions >> >> Extract restrictedAcl() AutoCloseable and also use AutoCloseable for the >> temporary directory cleanup. > > src/java.base/share/classes/java/security/Security.java line 116: > >> 114: private static Path currentPath; >> 115: >> 116: private static final List<Path> activePaths = new ArrayList<>(); > > Consider using `LinkedHashSet` instead. Hi, Since I've [been previously told](#issuecomment-3414764843) to use `Files::isSameFile`, we are no longer relying on the `Set` features to check the uniqueness of the paths we include. As a consequence, we are iterating `activePaths` each time we need to check for a circular inclusion. https://github.com/openjdk/jdk/blob/4e7206c082e4b0152d999b32eba0063a506eff67/src/java.base/share/classes/java/security/Security.java#L269-L282 Please also note that `checkCyclicInclude(path)` will reject duplicated entries before adding them to `activePaths`. https://github.com/openjdk/jdk/blob/4e7206c082e4b0152d999b32eba0063a506eff67/src/java.base/share/classes/java/security/Security.java#L289-L293 With this in mind, I think we shouldn't be paying the `LinkedHashSet` hashing costs given we aren't going to use the `Set` features, i.e. we won't be inserting duplicated entries nor checking `activePaths.contains(path)`. Please let me know if I'm missing anything. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24465#discussion_r2582326379
