Repository navigation
[FLINK-36902][core] Keep process exit working on JDK 24+ where the Security Manager is disabled - #29547
[FLINK-36902][core] Keep process exit working on JDK 24+ where the Security Manager is disabled #29547sheinbergon wants to merge 4 commits into
Conversation
…Factory SecurityManager#getThreadGroup returns the current thread's group by default and Flink's own security manager never overrides it, so the lookup was a no-op that only kept a dependency on the deprecated-for- removal Security Manager API (JEP 486). Also fixes stale "FlinkUserSecurityManager" names in the javadoc of FlinkSecurityManager. Generated-by: Claude Code (Claude Fable 5.1)
…curity Manager is disabled Since JDK 24 (JEP 486) System#setSecurityManager always throws an UnsupportedOperationException, even when called with null. As a consequence FlinkSecurityManager#forceProcessExit threw instead of terminating the JVM, so fatal errors handled by FatalExitExceptionHandler, ClusterEntrypoint#onFatalError or TaskManagerRunner left the process running. Only unset the security manager if one is actually installed, which can never be the case on JDK 24+. Additionally, enabling cluster.intercept-user-system-exit or cluster.processes.halt-on-fatal-error on a JVM without Security Manager support (JDK 24+, or JDK 18-23 without -Djava.security.manager=allow) now fails with a message explaining the cause instead of a misleading "no permission" error. ClusterUncaughtExceptionHandlerITCase no longer hangs on JDK 25 and is untagged from FailsOnJava25. FlinkSecurityManagerITCase gets two tests that run on every JDK: a forced exit without a security manager and the descriptive error for an unavailable security manager (child JVM started with -Djava.security.manager=disallow via a new TestJvmProcess hook). Generated-by: Claude Code (Claude Fable 5.1)
…unavailable on JDK 24+ cluster.intercept-user-system-exit and cluster.processes.halt-on-fatal-error can only work where a Java Security Manager can be installed, which is no longer possible since JDK 24 (JEP 486). Say so in the option descriptions and regenerate the configuration docs. Generated-by: Claude Code (Claude Fable 5.1)
|
@snuyanzin can you please see if this fix is worthy? |
snuyanzin
left a comment
There was a problem hiding this comment.
thank you for the contribution
why do we need it working in jdk25 if in previous versions it was caught?
Ideally we should have own implementation rather than make it working in complete opposite way
see appendix https://openjdk.org/jeps/486
Thanks for the review. To make sure I read you right: this PR does not touch System.exit interception. On JDK <= 23 cluster.intercept-user-system-exit behaves exactly as before. It only fixes FlinkSecurityManager#forceProcessExit, Flink's own exit on fatal errors. Since JDK 24 System.setSecurityManager(null) throws, so on JDK 24+ a fatal error currently leaves the JobManager/TaskManager process hanging instead of exiting (that is why ClusterUncaughtExceptionHandlerITCase hangs on JDK 25). Does this hang fix work for you on its own, with an agent-based replacement for interception (JEP 486 appendix) handled as a separate ticket? |
What is the purpose of the change
Since JDK 24 the Java Security Manager is permanently disabled (JEP 486):
System#setSecurityManageralways throwsUnsupportedOperationException, even when passednull.FlinkSecurityManager#forceProcessExitcalled it unconditionally, so on JDK 24+ the fatal-error paths (FatalExitExceptionHandler,ClusterEntrypoint#onFatalError,TaskManagerRunner#terminateJVM) threw instead of terminating the JVM and left the process running. This is also whyClusterUncaughtExceptionHandlerITCasehad to be taggedFailsOnJava25.This PR makes the exit helpers independent of an installed Security Manager and reports a clear error when the Security Manager based options are enabled on a JVM that cannot install one. Behaviour on JDK 11-23 is unchanged. Full removal of
FlinkSecurityManagerandcluster.intercept-user-system-exit(as attempted in #26491) is intentionally out of scope and needs a dev-list discussion first.Brief change log
FlinkSecurityManager#forceProcessExit/#checkExitonly unset the Security Manager when one is actually installed, which can never be the case on JDK 24+.cluster.intercept-user-system-exitorcluster.processes.halt-on-fatal-errorwithout Security Manager support (JDK 24+, or JDK 18-23 without-Djava.security.manager=allow) fails with a message naming JEP 486 and the options to disable, instead of a misleading "no permission" error.ExecutorThreadFactoryno longer consultsSystem#getSecurityManagerfor the thread group (the lookup was a no-op).ClusterUncaughtExceptionHandlerITCaseuntagged fromFailsOnJava25;TestJvmProcessgained agetJvmArgs()hook.Verifying this change
This change added tests and can be verified as follows:
FlinkSecurityManagerITCase#testForcedJVMExitWithoutSecurityManager: a child JVM callsforceProcessExitwithout any Security Manager configured and must exit with the requested code. Fails on JDK 25 without the fix (UnsupportedOperationException: Setting a Security Manager is not supported).FlinkSecurityManagerITCase#testUnavailableSecurityManagerIsReported: a child JVM started with-Djava.security.manager=disallowmust fail with the descriptiveIllegalConfigurationException. Runs on JDK 12+.ClusterUncaughtExceptionHandlerITCasepasses on JDK 25 again (hung before the fix).FlinkSecurityManagerTestand theFailsOnJava25-tagged tests are unchanged and pass on JDK 21.Verified locally on JDK 21 and JDK 25 (Zulu).
Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Claude Fable 5.1)