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

[Java 21] "Setting Security Manager is not supported" errors #709

Closed
iloveeclipse opened this issue Sep 21, 2023 · 36 comments · Fixed by eclipse-platform/eclipse.platform.releng.aggregator#1469
Assignees
Labels
bug Something isn't working
Milestone

Comments

@iloveeclipse
Copy link
Member

After starting I20230921-0530 on Java 21 I see following errors reported in the log:

eclipse.buildId=4.30.0.I20230921-0530
java.version=21
java.vendor=Oracle Corporation
BootLoader constants: OS=linux, ARCH=x86_64, WS=gtk, NL=en_US
Command-line arguments:  -data /data/4x_platform_workspace -os linux -ws gtk -arch x86_64

org.eclipse.ant.core
Error
Thu Sep 21 13:33:52 CEST 2023
Setting Security Manager is not supported

We should do something on Java 21 to avoid that code being executed or at least mute the error if we can't avoid that.

@iloveeclipse iloveeclipse added the bug Something isn't working label Sep 21, 2023
@iloveeclipse iloveeclipse added this to the 4.30 milestone Sep 21, 2023
@laeubi
Copy link
Contributor

laeubi commented Sep 21, 2023

Platform is currently Java 17 so why is this a bug? What has changed since last discussion here? Any new decisions?

@iloveeclipse
Copy link
Member Author

What has changed since last discussion here?

Java 21 (next LTS) was released

@laeubi
Copy link
Contributor

laeubi commented Sep 21, 2023

So Platform will upgrade to Java 21 for the 2023-12 release? That would be great 👍

@iloveeclipse
Copy link
Member Author

So Platform will upgrade to Java 21 for the 2023-12 release? That would be great 👍

No, for sure not, but people / organisations will start Java 21 evaluation & migration and we should make sure that we don't ship software that report errors "by default".

E.g. in Advantest we monitor error log during test executions and report every unexpected warning/error as test fail. So switching our build to Java 21 would lead to test errors in all places where ant could be involved. Of course we can suppress that specific error in tests, but our customers, once we switch product to run on Java 21, will see the errors and report bugs.

That all shouldn't happen if we know where the problem is, so a generic (not client specific) solution is needed.

@laeubi
Copy link
Contributor

laeubi commented Sep 21, 2023

No, for sure not, but people / organisations will start Java 21 evaluation & migration and we should make sure that we don't ship software that report errors "by default".

The default Eclipse Packages use Java 17 as a JVM so no errors are reported, this was discussed already here:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=575210

so for me the question is what "bug" in Platform is here to solve?

This is the corresponding bug/issue at ANT: https://bz.apache.org/bugzilla/show_bug.cgi?id=65381
as far as I know @merks is already working on upgrading ant.

So from platform side Java 21+ Ant is currently not supported and Platform is targeting Java 17 right now and also ships with this JVM.

@iloveeclipse
Copy link
Member Author

So from platform side

I see it from user side, and it is clearly a bug if software reports errors.
We can't avoid that users would start using Java 21 and have to deal with that.
If #701 solves the problem, even better.
If not, we should provide a solution.

@akurtakov
Copy link
Member

Our code would have to be changed to version guard and not set SecurityManager if running on Java 21+.
The fact that EPP packages ship with Java 17 doesn't mean we shouldn't care about newer JVMs and from from Platform side we require minimum of Java 17 with no max version set. Please note that Platform doesn't ship a JVM (https://download.eclipse.org/eclipse/downloads/drops4/I20230921-0530/) and IMHO that's a good thing as it broadens the testing matrix and eases switches to newer JVM later.
P.S. As per https://www.eclipse.org/lists/eclipse.org-planning-council/msg03578.html Java 21 is fine for releases targeting 2024-06, plugins that are not part of simrel can require Java 21 even today.

@laeubi
Copy link
Contributor

laeubi commented Sep 21, 2023

I see it from user side

I as a user don't see the error ...

If #701 solves the problem, even better.

Depends on how one defines "solve" ... you won't see the error anymore but Ant can now kill your eclipse, similar to this m2e issue:

If not, we should provide a solution.

The solution is to use the JVM we recommend, test and ship to the user :-)

@akurtakov
Copy link
Member

Please note that Eclipse platform is not only an IDE - it's an RCP platform used in very different environments, requirements and etc. thus there is no way that "one size fits all" works.

@tjwatson
Copy link
Contributor

The solution is to use the JVM we recommend, test and ship to the user :-)

The Eclipse project does not ship the JVM (EPPs do). We should work (and test) on the latest Java LTS release.

@merks
Copy link
Contributor

merks commented Sep 21, 2023

I wonder, does -Djava.security.manager=allowed still work?

@iloveeclipse
Copy link
Member Author

I wonder, does -Djava.security.manager=allowed still work?

It depends how you define "works" :-)

With the argument above Eclipse crashes reporting on the shell this:

./eclipse 
WARNING: Using incubator modules: jdk.incubator.vector
Error occurred during initialization of VM
java.lang.InternalError: Could not create SecurityManager
        at java.lang.System.initPhase3(java.base@21/System.java:2301)
Caused by: java.lang.ClassNotFoundException: allowed
        at jdk.internal.loader.BuiltinClassLoader.loadClass(java.base@21/BuiltinClassLoader.java:641)
        at jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(java.base@21/ClassLoaders.java:188)
        at java.lang.ClassLoader.loadClass(java.base@21/ClassLoader.java:526)
        at java.lang.Class.forName0(java.base@21/Native Method)
        at java.lang.Class.forName(java.base@21/Class.java:534)
        at java.lang.Class.forName(java.base@21/Class.java:513)
        at java.lang.System.initPhase3(java.base@21/System.java:2285)

But using allow like here -Djava.security.manager=allow helps to avoid errors :-)

Does this mean, we should add this argument to the eclipse ini as solution but only if we detect Java 21+ runtime? Or are there better alternatives?

@tjwatson
Copy link
Contributor

Does this mean, we should add this argument to the eclipse ini as solution but only if we detect Java 21+ runtime? Or are there better alternatives?

Given we only support Java 17+ we could set that by default in eclipse.ini. That should work for Java 17 also.

@merks
Copy link
Contributor

merks commented Sep 21, 2023

Google turned up wrong answer. You can't trust anything anymore. 😱

Yes, as @tjwatson suggests, this has been working for quite some time. I believe in ant 1.10.13 they defined such a class to make it work with older JVMs, but that's removed in 1.10.14 (and that caused problems BND problems).

Let's hold off any decisions until we see how 1.0.14 works. Maybe they didn't something super smart?

@iloveeclipse
Copy link
Member Author

Google turned up wrong answer

Depends on the point of view :-) If AI's final goal is the death of mankind, the first step is to give seemingly right answers leading to crashes. Next time it will be not an IDE but some missile launch engineer copy/pasting AI's proposal :-(

@merks
Copy link
Contributor

merks commented Sep 23, 2023

The next I-Build will be based on ant 1.10.14.

This page still suggestion Temurin 21 should arrive this month:

https://adoptium.net/support/

@iloveeclipse
Copy link
Member Author

The next I-Build will be based on ant 1.10.14.

I see same errors reported with I20230924-0600 (without -Djava.security.manager=allow option set):

eclipse.buildId=4.30.0.I20230924-0600
java.version=21
java.vendor=Oracle Corporation
BootLoader constants: OS=linux, ARCH=x86_64, WS=gtk, NL=en_US
Command-line arguments:  -os linux -ws gtk -arch x86_64

org.eclipse.ant.core
Error
Mon Sep 25 10:04:51 CEST 2023
Setting Security Manager is not supported

Eclipse says it is using 1.10.14.v20230922-1200 version of org.apache.ant.
What exactly the 1.10.14 and should have fixed?

@akurtakov
Copy link
Member

From ant 1.10.14 whatsnew file:

When using Java 18 or higher, Ant will no longer use Java SecurityManager
   because it has been deprecated for removal and by default is disallowed
   to be set at runtime https://openjdk.org/jeps/411.
   This will mean that the "<permissions>" type is no longer functional when
   using Java 18 or higher.
   Furthermore, when using Java 18 or higher, if the build executes
   tasks that call "java.lang.System.exit()" and if those tasks aren't
   running in a forked VM of their own, then such tasks will now kill
   the entire Ant build process. It is recommended that such tasks be
   updated to launch in a forked JVM so that the System.exit() call
   will not impact the JVM in which Ant process runs.

@iloveeclipse
Copy link
Member Author

OK, so "Setting Security Manager is not supported" error is not reported by actual ant code, it is our own error?

@akurtakov
Copy link
Member

Yes, it is

@SarikaSinha
Copy link
Member

Yes, we need to update out Ant code. As Ant is not supporting the setting of SecurityManager any more.

The change with Ant 1.10.14 is that Ant is not adding any "allow" class any more.
The use of a "allow" class as a workaround to older versions of JDK
considering this value as a classname for -Djava.security.manager system
property.

@laeubi
Copy link
Contributor

laeubi commented Oct 20, 2023

I just wanted to add that I came across a very neat solution (described by @bmarwell) for this (@tjwatson maybe this would be an option for Equinox / OSGi as well):

  1. You encapsulates the access to SM into an own class, lets call this SecurityMangerAccessor
  2. You have one implementation of this that uses the SecurityManger as is
  3. create a Multi-Release-Jar and have a second version that is only a NO-OP and does not access SecurityManger at all and place that compiled class in /META-INF/versions/21/....

As a result, your jar will use the SM when running on JVM < 21 and will not use it when running on JVM >= 21 🥇

@bmarwell
Copy link

Oh hey @tjwatson again 👋🏻😅

Here's the needed setup:
groovy/GMavenPlus#287

Alternative: parse the output of a static initialiser block of the system property "java.version".

@bmarwell
Copy link

FYI, Setting outputDirectory will probably become valid:

apache/maven-compiler-plugin#202

@tjwatson
Copy link
Contributor

Thanks @bmarwell.

As a result, your jar will use the SM when running on JVM < 21 and will not use it when running on JVM >= 21

The framework has no need for this and I would not be for this existing in the framework. What is the stacktrace of what is setting the security manager? If it is coming from Ant then I doubt we would convince Ant to make calls to some org.eclipse API to work around it. If it is coming from the framework itself then it is a bug we must fix in the framework, but I doubt that would involve some complicated MR JAR solution.

@bmarwell
Copy link

bmarwell commented Oct 23, 2023

If it is coming from the framework itself then it is a bug we must fix in the framework, but I doubt that would involve some complicated MR JAR solution.

Ha, I thought it is already clear where the message originated.
Anyway, as Security Manager is deprecated for removal, you cannot load nor import it starting with Java 21/24 or so?. At the same time you must load it for 8-21, for backwards compatibility.

The other option, if you find a perfectly valid MR Jar complicated, is a static initialiser loading one of two classes (one which does the SM logic, one which doesn't)

However, you will instead need to parse the Java version for this. Not sure MR Jar is more complicated than parsing Java versions, but YMMV.

As this might need a backport, you probably cannot use Java 11's Runtime.getVersion().

@iloveeclipse
Copy link
Member Author

iloveeclipse commented Oct 24, 2023

Given we only support Java 17+ we could set that by default in eclipse.ini. That should work for Java 17 also.

This looks for me as the best proposal so far. I've played a bit with current platform & I don't see any issues with this preference on Java 21 / latest ant.

Example project with the ant build file/launch config that runs System.exit() from ant running from IDE:
system_exit_from_ant.zip

The output looks like:


Buildfile: /data/runtime-Java_21/A/build.xml

build:
        [java] Callling System.exit() now!
        [java] org.eclipse.ant.core.AntSecurityException
        [java] 	at org.eclipse.ant.internal.core.AntSecurityManager.checkExit(AntSecurityManager.java:62)
        [java] 	at java.base/java.lang.Runtime.exit(Runtime.java:186)
        [java] 	at java.base/java.lang.System.exit(System.java:1916)
        [java] 	at A.main(A.java:4)
        [java] 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        [java] 	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        [java] 	at org.apache.tools.ant.taskdefs.ExecuteJava.run(ExecuteJava.java:220)
        [java] 	at org.apache.tools.ant.taskdefs.ExecuteJava.execute(ExecuteJava.java:155)
        [java] 	at org.apache.tools.ant.taskdefs.Java.run(Java.java:892)
        [java] 	at org.apache.tools.ant.taskdefs.Java.executeJava(Java.java:232)
        [java] 	at org.apache.tools.ant.taskdefs.Java.executeJava(Java.java:136)
        [java] 	at org.apache.tools.ant.taskdefs.Java.execute(Java.java:109)
        [java] 	at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:299)
        [java] 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        [java] 	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        [java] 	at org.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:99)
        [java] 	at org.apache.tools.ant.Task.perform(Task.java:350)
        [java] 	at org.apache.tools.ant.Target.execute(Target.java:449)
        [java] 	at org.apache.tools.ant.Target.performTasks(Target.java:470)
        [java] 	at org.apache.tools.ant.Project.executeSortedTargets(Project.java:1401)
        [java] 	at org.apache.tools.ant.Project.executeTarget(Project.java:1374)
        [java] 	at org.apache.tools.ant.helper.DefaultExecutor.executeTargets(DefaultExecutor.java:41)
        [java] 	at org.eclipse.ant.internal.core.ant.EclipseDefaultExecutor.executeTargets(EclipseDefaultExecutor.java:34)
        [java] 	at org.apache.tools.ant.Project.executeTargets(Project.java:1264)
        [java] 	at org.eclipse.ant.internal.core.ant.InternalAntRunner.run(InternalAntRunner.java:716)
        [java] 	at org.eclipse.ant.internal.core.ant.InternalAntRunner.run(InternalAntRunner.java:532)
        [java] 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        [java] 	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        [java] 	at org.eclipse.ant.core.AntRunner.run(AntRunner.java:369)
        [java] 	at org.eclipse.ant.internal.launching.launchConfigurations.AntLaunchDelegate.lambda$0(AntLaunchDelegate.java:279)
        [java] 	at java.base/java.lang.Thread.run(Thread.java:1583)
        [java] Java Result: -1
BUILD SUCCESSFUL
Total time: 3 seconds

I would propose a PR adding this system property in platform SDK product and open an issue on https://github.com/eclipse-packaging/packages/issues to add -Djava.security.manager=allow by default to epp packages. This would avoid silly error logs for those running on Java 21+.

FYI: we set security manager in two places, to run ant / resolve ant build files, see https://github.com/search?q=%28org%3Aeclipse-platform+setSecurityManager%29+&type=code

iloveeclipse added a commit to iloveeclipse/eclipse.platform.releng.aggregator that referenced this issue Oct 24, 2023
This avoids "Setting Security Manager is not supported" errors logged if
running ant related tooling on Java 21.

Fixes eclipse-platform/eclipse.platform#709
@iloveeclipse
Copy link
Member Author

I've created eclipse-packaging/packages#71 for epp.

@iloveeclipse
Copy link
Member Author

For the completeness: there is a Java bug asking to provide a replacement API that would disallow System.exit() being called from the code, but as far as I can see no intents to provide that: https://bugs.openjdk.org/browse/JDK-8199704.

@tjwatson
Copy link
Contributor

I've created eclipse-packaging/packages#71 for epp.

Isn't there an eclipse.ini seeded from the platform that gets fed into EPP where we can set this such that it applies to every EPP automatically as well as the packages the Eclipse project itself publishes?

@iloveeclipse
Copy link
Member Author

No idea.

@merks
Copy link
Contributor

merks commented Oct 24, 2023

No, each EPP product specifies such things itself. E.g.,

https://github.com/eclipse-packaging/packages/blob/8b88a53ec2a07b81eb8ada028bd235f4197c310b/packages/org.eclipse.epp.package.committers.product/epp.product#L29

iloveeclipse added a commit to eclipse-platform/eclipse.platform.releng.aggregator that referenced this issue Oct 24, 2023
This avoids "Setting Security Manager is not supported" errors logged if
running ant related tooling on Java 21.

Fixes eclipse-platform/eclipse.platform#709
@jonahgraham
Copy link
Contributor

I've created eclipse-packaging/packages#71 for epp.

Isn't there an eclipse.ini seeded from the platform that gets fed into EPP where we can set this such that it applies to every EPP automatically as well as the packages the Eclipse project itself publishes?

There used to be, but EPP no longer uses Platform's product as starting point due to limitations in the p2 model that meant we had duplicate entries for -Dosgi.requiredJavaVersion= with EPP providing =17 and Platform providing =11 and both ending up in the ini file.

@HannesWell
Copy link
Member

I've created eclipse-packaging/packages#71 for epp.

Isn't there an eclipse.ini seeded from the platform that gets fed into EPP where we can set this such that it applies to every EPP automatically as well as the packages the Eclipse project itself publishes?

There used to be, but EPP no longer uses Platform's product as starting point due to limitations in the p2 model that meant we had duplicate entries for -Dosgi.requiredJavaVersion= with EPP providing =17 and Platform providing =11 and both ending up in the ini file.

I don't think =11 for platform is right since most platform bundles require 17 anyway.
I wonder if it is even necessary to declare osgi.requiredJavaVersion at all since Java-17 is implied by the highest bundles EE requirements?
I assume osgi.requiredJavaVersionis only usefull if one wants an even higher Java version (like 21 for now)? Or is there a deeper meaning of osgi.requiredJavaVersion?

@jonahgraham
Copy link
Contributor

I don't think =11 for platform is right since most platform bundles require 17 anyway.

Sorry - I was imprecise with the wording. I was trying to provide historical context from when Platform did used to provide =11.

I wonder if it is even necessary to declare osgi.requiredJavaVersion at all since Java-17 is implied by the highest bundles EE requirements?
I assume osgi.requiredJavaVersionis only usefull if one wants an even higher Java version (like 21 for now)? Or is there a deeper meaning of osgi.requiredJavaVersion?

IIUC requiredJavaVersion will raise an error if JVM is < than requiredJavaVersion. If not specified, the bundles with BREE > than JVM will simply not be used. Therefore I think we need requiredJavaVersion to be the minimum version so we don't surprise users.

@merks
Copy link
Contributor

merks commented Oct 24, 2023

Yes it is supposed to cause an explicit failure on startup even before bundles fail to start.

@iloveeclipse iloveeclipse self-assigned this Oct 25, 2023
@iloveeclipse iloveeclipse modified the milestones: 4.30, 4.30 M3 Oct 25, 2023
iloveeclipse added a commit that referenced this issue Oct 25, 2023
jonahgraham added a commit to jonahgraham/packages that referenced this issue Oct 26, 2023
The security manager is needed if ant is installed (either pre-installed
or added by the user). This change synchronizes all the EPP packages
with the change made to Eclipse Platform/SDK products in
eclipse-platform/eclipse.platform#709

This also addresses the "Synchronize any changes to platform.product
into all the epp.product files." step from RELEASING.md

Fixes eclipse-packaging#71
Part of eclipse-packaging#72
Michael5601 pushed a commit to CodeLtDave/eclipse.platform that referenced this issue Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
9 participants