[#389] Add option 'blockSystemExit' to 'java' mojo

[kriegaex]

This new option enables users to stop programs called by exec:java from calling System::exit, terminating not just the mojo but the whole Maven JVM.

Closes #389.
Relates to #388.

[kriegaex]

@slawekjaranowski and other maintainers: Please review.

[kriegaex]

I force-pushed a fix for the problem that only JDK 17+ needs the extra mavenOpts for Maven Invoker ITs. I thought, the system property would just be ignored, but older JDKs interpret the value "allow" for the security manager (SM) as a class name, trying to instantiate a SM of that name.

Please run the build again.

[kriegaex]

@slawekjaranowski, another question: You seem to be re-starting some Windows ITs unrelated to my change. They also pass locally on my machine under Windows in the same JDK. Does that imply that you already know they are flaky? And do you know the reason why they are flaky? Is it because they run concurrently?

[slawekjaranowski]

I will try to fix a ITs on windows ....

[slawekjaranowski]

I hope fix ITs by #392 - please rebase

[kriegaex]

Rebased and pushed. There actually was a merge conflict, because we both added a new Maven profile in the same place. but no big deal.

[kriegaex]

@slawekjaranowski, is there anything else stopping you from merging? I want to keep the difference between touch time and cycle time for this PR as low as possible, because frequent context switches are just bad for productivity.

On a side note, I am surprised that you need to explicitly approve not just the first build for a pull request but every single one. This kind of regulation is kind of unusual, most OSS projects only block the first one. It forces you to micro-manage the PR and wait for the build result while maybe you do not have much else to do with regard to reviewing it. So, probably you will also switch the context to something else first and then back again.

[TobiX]

Using this makes the mojo non-threadSafe and might introduce "funny" race-conditions. This should be at least documented.

(See also the similar feature in GMavenPlus: https://github.com/groovy/GMavenPlus/pull/146/files)

[kriegaex]

@TobiX, interesting. Please elaborate. The link does not help much. What kind of race conditions do you envision? And what could be worse than the whole Maven process just shutting down hard when dealing with processes using System.exit()?

[TobiX]

I don't want to argue that a spurious System.exit() is a bad problem. If you cannot change the code running inside the maven process to avoid it, using a SecurityManager currently seems like the only solution. I'd just like to argue that it should be AT LEAST documented (or even changed in the mojo definition) that fiddling with the SecurityManager makes the mojo inherintly not thread-safe, which might lead to hard-to-debug problems when using this mojo in a threaded environment (M2E, mvnd or even -T) due to leaked SecurityMaangers (Due to race-conditions, an instance of SystemExitManager might linger after this mojo is finished)...

[kriegaex]

In all cases but System.exit being called, SystemExitManager passes through permission requests to the original anyway, so that would not be bad. The only effect of it lingering would be that no-one else could use System::exit anymore to shut down the JVM. But how would you imagine that even being even possible? The value is being reset in a finally block.

Please, like I said, give a concrete example. I am not against documenting possible problems or side effects, but there is no point in scaring users, leading them to maybe not use a useful feature, just because of something that will never happen. I am not a security manager expert. Actually, I am happy they will go away from the JVM. OTOH, I do not like System::exit either and find its existence harmful. There are better ways to terminate an application and its host JVM.