On Tue, 2 Dec 2025 18:17:24 GMT, Francisco Ferrari Bihurriet <[email protected]> wrote:
>> 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. Indeed, I missed the fact that we no longer call `activePaths.contains(path)`, thanks! ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24465#discussion_r2582345758
