JDK 11 compatibility: Work around removed API in SecurityManager

[jan-z]

This works on both JDK 8 and JDK 11.

[fhoeben]

I saw this problem as well. I haven't applied this change yet as it possibly changes the behaviour experienced (if the security manager that is delegated to has a different implementation for the removed methods).

Although the current code does not compile on JDK 11, it doesn't create a problem at runtime on Java 11 (the removed methods will never be called, but having them is ok).

So I see no advantage in merging this change until we remove support for Java 8 and switch to Java 11+ fully.

Do you see an advantage of changing this now?

[jan-z]

The advantage is just for developers and contributors like me. It is somewhat embarrassing to fail to compile on JDK 11 these days, as JDK 8 is no longer easily available from Oracle. That will discourage community participation.

There is a risk that a delegate might have a different implementation, yes. But those four methods have already been deprecated in JDK 8.

An alternative would be to check at runtime if the methods are available on the delegate.

[zshensley]

Interesting.... does the @override present any problems, would assume only at compile time. We are seeing an issue with trying to run on java11.. Could be something else but this is the first place we went and just happened to create a multi-release jar. Not a contributor but happy to share my changes (attached a patch). May not want to accept them all but maybe some of the gradle changes to enforce compilation with jdk8, not just sourceCompatability. but build it with java 8 and still allow you to run your apps with another version..

0001-Add-logic-to-make-a-multi-release-jar-that-works-on-.txt

[fhoeben]

Can you explain what problems you faced on Java 11? I never experienced any problems running on Java 11 myself. As far as I'm aware the only problem was compiling.

[zshensley]

I will let you know if i find an issue specifically with Fitnesse.. can't say that is a fitnesse issue but we do start getting some random exceptions when we run on java11..

[tcnh]

The methods you refer to can not be removed without breaking Java 8 compatibility. As many organisations still rely on JDK 8, and oracle supports it until end of 2020, I'd opt to not remove them yet, at the cost of having to compile against JDK8.

[fhoeben]

For now FitNesse will continue to support Java 8, so will be compiled using JDK 8 and therefore this PR will not get merged. It can serve as a reference for people wanting to build/debug locally using higher Java versions.

There is no issue, as far as I'm aware, running FitNesse using Java 11 (or higher). But for compiling Java 8 is used.

[superzaky]

The methods you refer to can not be removed without breaking Java 8 compatibility. As many organisations still rely on JDK 8, and oracle supports it until end of 2020, I'd opt to not remove them yet, at the cost of having to compile against JDK8.

But I did compile against JDK 8 and nothing got broken. What error did you get?

Also the methods aren't being used anywhere in the code. Please show me where the methods are being used, because they aren't being used anywhere. If that's the case then they should be removed. Unless there's a good reason for it, which I would like to hear.

[jan-z]

@superzaky The methods are needed for runtime compatibility on JDK 8. SystemExitSecurityManager delegates to SecurityManager. We don't know which SecurityManager implementations are used.

[superzaky]

@superzaky The methods are needed for runtime compatibility on JDK 8. SystemExitSecurityManager delegates to SecurityManager. We don't know which SecurityManager implementations are used.

If you don't know then why did you respond with the thumbs down? Please don't feel attacked it's just a question. And I do know which implementations of SecurityManager are being used. For example activateIfWanted() is being used in Instructions.java. And checkAwtEventQueueAccess() isn't being used anywhere hence it's a code smell that needs to be tackled.

I'm just applying the boy scout rule here and I'm putting effort explaining things to you.

[jan-z]

The issue here is API compatibility at runtime. It is not about compiling FitNesse. You can't know which SecurityManager implementations are used at runtime. FitNesse can be used in a number of different environments with different SecurityManagers.

We only know that System.getSecurityManager() returns a class that implements the SecurityManager interface. And if we want to keep JDK8 compatibility we need to implement all methods of that interface.

[superzaky]

The issue here is API compatibility at runtime. It is not about compiling FitNesse. You can't know which SecurityManager implementations are used at runtime. FitNesse can be used in a number of different environments with different SecurityManagers.

We only know that System.getSecurityManager() returns a class that implements the SecurityManager interface. And if we want to keep JDK8 compatibility we need to implement all methods of that interface.

I know what runtime is. But the methods in SecurityManager.java aren't even required to be implemented. So even if the implementation on runtime was SystemExitSecurityManager.java nothing would've happened, because SystemExitSecurityManager.java is not required to implement the checkTopLevelWindow() method.

So I don't quite understand you that you're saying that it's a API compatibility issue at runtime?

[jan-z]

SystemExitSecurityManager delegates to SecurityManager. All method calls need to be forwarded to the delegate, so that the behavior of the delegate is preserved.

[teunisv]

@fhoeben Java 8 Support has ended, so when are we updating to a new version ?

[fhoeben]

@teunisv as far as I'm aware Java 8 is still supported by Zulu, Amazon Cornetto and Eclipse Temurin. It is just Oracle that is no longer supporting. Since a Java 8 build will run fine on a Java 11 I see no immediate reason to upgrade. Only the build process is locked to (also) support Java 8.

Of course we will have to upgrade eventually and I see two main stumbling blocks: usage of the security manager and the need of a JavaScript runtime (Nashorn).

• I believe the SecurityManager's API changed in Java 11 (what this issue is about) but furthermore it is deprecate in Java 18 and will be removed in a future version. I believe we only have this security manager to prevent System.exix(). We should probably drop support for this in FitNesse altogether. People relying on their SUT not actually terminating the JVM should probably find a solution in their own fixture code (which they can then tailor to the JVM they use)
• The scripting (JavaScript) support I would actually like to keep. And we might be able to do that by including a separate one, since one is no longer included beyond Java 11. The GraalVM offers one, but maybe there is also a (lighter/smaller) one available.

But like I said I see no immediate need to upgrade. There are supported Java 8 runtimes to compile the code with, and they will run fine on at least Java 11.

[tcnh]

For the scripting, I have a branch that uses the openjdk/nashorn dependency instead of jdk.nashorn. This works without issues, but requires to build with Java 11. Needless to say it also includes the necessary fixes around security manager (but that will still require refactoring later for use with a jvm that actually has the Security manager removed)

Will share in a PR when I find some time