Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VSNetBeans: Disable SecurityManager [compat:jdk19-24]. Avoid repeated starts on failure. #8159

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Jan 16, 2025

This should go to VSNetbeans release branch as well.
This PR fixes two issues:

  1. NBLS now fails to start on JDK19+, because of
java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release
	at java.base/java.lang.System.setSecurityManager(System.java:425)
	at org.netbeans.TopSecurityManager.install(TopSecurityManager.java:525)
	at org.netbeans.core.NbLifecycleManager.advancePolicy(NbLifecycleManager.java:69)
	at org.netbeans.core.GuiRunLevel.run(GuiRunLevel.java:84)

The option controlling SecurityManager is missing from the NBLS app configuration. For compatibility with JDK24, I've decided to disable TopSecurityManager completely (-J-DTopSecurityManager.disable=true).

  1. During debugging, I have noticed that NBLS starts repeatedly over and over again. It's annoying especially since the dialogs printed steal focus from e.g. command palette etc.
    This patch changes the startup so that automatic restart after 10sec happes ONLY if the language server initializes correctly; if it fails to start completely, it is not respawned.

If you test this, please test:

  • pristine start with NO JAVA on PATH and in JAVA_HOME
  • configure with unsuitable JDK, e.g. JDK8-11
  • valid configuration
  • start with no or invalid JDK then accept the offer to configure JDK for NBLS
  • crash (kill) of a NBLS server after successful startup
  • change to vital parameter such as "netbeans.javaSupport.enabled" that should purposely restart NBLS

@sdedic sdedic added kind:bug Bug report or fix VSCode Extension [ci] enable VSCode Extension tests labels Jan 16, 2025
@sdedic sdedic added this to the NB25 milestone Jan 16, 2025
@sdedic sdedic self-assigned this Jan 16, 2025
Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't test all the combinations, but I did try some cases, and it seems reasonable to me.

@jglick
Copy link
Contributor

jglick commented Jan 16, 2025

Maybe I am missing something but should

if (!Boolean.getBoolean("TopSecurityManager.disable")) {
// set security manager
TopSecurityManager.install();
not automatically skip this when running on Java 21, since it is guaranteed to throw this exception? I just tried ant tryme on trunk and it failed for this reason, which seems like a pretty basic problem.

@mbien
Copy link
Member

mbien commented Jan 16, 2025

Hi @jglick!

not automatically skip this when running on Java 21, since it is guaranteed to throw this exception?

SM itself works fine up until JDK 23 (inclusive) (outside of deprecation warnings and that JDK 18 made 'disallow' the default which can still be changed to 'allow'). NB 24 can be built and run on JDK 17-23 with SM still active.

JDK 24 is more interesting since it disabled SM. If you grab a dev build from master, you can start it on JDK 24 as experiment if you replace -J-Djava.security.manager=allow with -J-DTopSecurityManager.disable=true in the conf. But those flags made it into everything by now, so to run tests on JDK 24 etc more has to be done #7928.

Might be good to add VSNetbeans to the title so that this is not confused with classic NB - since NB doesn't restart ;)

Copy link
Contributor

@dbalek dbalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine.

@sdedic sdedic changed the title Disable SecurityManager [compat:jdk19-24]. Avoid repeated starts on failure. VSNetBeans: Disable SecurityManager [compat:jdk19-24]. Avoid repeated starts on failure. Jan 17, 2025
@sdedic sdedic merged commit cb55d6b into apache:master Jan 17, 2025
32 checks passed
@sdedic
Copy link
Member Author

sdedic commented Jan 17, 2025

Cherrypicked into vsnetbeans_2499 as 43ceb60

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug Bug report or fix VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants