Open
Conversation
8df2cab to
c161cef
Compare
new test and build range: JDK 21-26 few modules are still stuck on 17 and got special treatment
3c0334b to
267581c
Compare
- added unimportant classes to the ignore list so that the test data doesn't have to be updated. - refactored file read monitoring from SecurityManager to JFR
- remove no longer needed SM usage in ide/xsl test - remove NBBundleWithSecurityManager test - remove NbLifecycleManagerEDTTest which is similar to LifecycleManagerEDTTest in openide.util.ui
e958a12 to
0e4b268
Compare
mbien
commented
Jan 27, 2026
Comment on lines
-181
to
+208
| js.runUserActionTask(new Task<CompilationController>() { | ||
| public void run(CompilationController parameter) throws Exception { | ||
| parameter.toPhase(Phase.RESOLVED); | ||
|
|
||
| TypeElement string = parameter.getElements().getTypeElement("test.test2"); | ||
|
|
||
| SecurityManager old = System.getSecurityManager(); | ||
|
|
||
| System.setSecurityManager(new SecMan()); | ||
|
|
||
| TreePathHandle.create(string, parameter); | ||
|
|
||
| System.setSecurityManager(old); | ||
| AtomicBoolean wasRead = new AtomicBoolean(); | ||
|
|
||
| js.runUserActionTask(controller -> { | ||
| controller.toPhase(Phase.RESOLVED); | ||
|
|
||
| TypeElement string = controller.getElements().getTypeElement("test.test2"); | ||
|
|
||
| try (RecordingStream rs = new RecordingStream()) { | ||
| rs.setMaxSize(Long.MAX_VALUE); | ||
| rs.enable("jdk.FileRead").withoutThreshold().withStackTrace(); | ||
| rs.onEvent("jdk.FileRead", e -> { | ||
| if (e.getString("path").endsWith("test2.java")) { | ||
| wasRead.set(true); | ||
| System.err.println(e); | ||
| } | ||
| }); | ||
| rs.startAsync(); | ||
| // file2.getInputStream().read(); // check: commenting this in should fail the test | ||
| TreePathHandle.create(string, controller); | ||
| // TODO call directly post JDK 21 bump | ||
| RecordingStream.class.getMethod("stop").invoke(rs); // flushes stream | ||
| } | ||
| }, true); | ||
|
|
||
| assertFalse("file was read", wasRead.get()); |
Member
Author
There was a problem hiding this comment.
POC of how to replace SecurityManager based IO monitoring with JFR. This wont work in all cases since the jdk.FileRead/Write events are for read/write operations in the literal sense, not for file attribute access. (e.g the tests in the versioning module will likely require a different approach to replace SM usage there)
Member
Author
|
squashed a little bit, will squash more before merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
new test and build range for NB 30+: JDK 21-26
some remain on 17 for due to #7871
this is also updating tests / test data if needed
Some of the tests still use the SM to inspect changing state. I began reimplementing it using JFR, but then realized that we can't do that yet due to an chicken-egg problem. It would require bumping some modules to JDK 21 which we could do now, but it would cause CI failures due to #7871. Workaround is to set
-Djava.security.manager=allowwhile trying to decide what do with the GraalVM job etc. Maybe we could exclude clusters from the GraalVM job.todo: update readme