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

Don't invoke shutdown signal handler until JVM init completes #18085

Merged
merged 1 commit into from Sep 7, 2023

Conversation

babsingh
Copy link
Contributor

@babsingh babsingh commented Sep 6, 2023

JVM init path: J9_CreateJavaVM.
JVM exit paths: protectedDestroyJavaVM and exitJavaVM.

A segfault or other side-effects can happen if the JVM init and
exit paths execute concurrently.

The exit path can be taken if a shutdown signal is raised and the
shutdown handler is invoked. JVM shutdown signals are SIGINT, SIGTERM
and SIGHUP.

Preventing invocation of the exit path from the shutdown signal handler
until the JVM initialization completes resolves the above side-effects.

Related:

@babsingh babsingh marked this pull request as draft September 6, 2023 19:03
if (J9_ARE_ANY_BITS_SET(vm->runtimeFlags, J9_RUNTIME_EXIT_STARTED)) {
shutdownStarted = TRUE;
/* Don't invoke handler if JVM hasn't initialized or JVM exit has started. */
issueReadBarrier();
Copy link
Contributor Author

@babsingh babsingh Sep 6, 2023

Choose a reason for hiding this comment

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

Signal handlers should not incorporate lock operations since they are not signal-safe. I have replaced the lock with a read barrier to read the most up-to-date vm->runtimeFlags value.

@babsingh
Copy link
Contributor Author

babsingh commented Sep 6, 2023

Running a personal build to check if there are any side-effects. I will move the PR out of the draft state once the personal build looks good.

@gacholio Requesting your review.

@pshipton @tajila fyi, this is related to internal issue 148771.

@babsingh
Copy link
Contributor Author

babsingh commented Sep 7, 2023

The personal build looks good; no side-effects seen. This PR is ready to be reviewed.

@babsingh babsingh marked this pull request as ready for review September 7, 2023 13:42
@gacholio
Copy link
Contributor

gacholio commented Sep 7, 2023

jenkins test sanity zlinux jdk21

@gacholio
Copy link
Contributor

gacholio commented Sep 7, 2023

jenkins compile win jdk8

JVM init path: J9_CreateJavaVM.
JVM exit paths: protectedDestroyJavaVM and exitJavaVM.

A segfault or other side-effects can happen if the JVM init and
exit paths execute concurrently.

The exit path can be taken if a shutdown signal is raised and the
shutdown handler is invoked. JVM shutdown signals are SIGINT, SIGTERM
and SIGHUP.

Preventing invocation of the exit path from the shutdown signal handler
until the JVM initialization completes resolves the above side-effects.

Related:
- eclipse-openj9#17101
- eclipse-openj9#17438

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
@babsingh babsingh force-pushed the fix_scopedvaluecache branch 2 times, most recently from 6d73779 to d4dfa6c Compare September 7, 2023 18:53
@babsingh
Copy link
Contributor Author

babsingh commented Sep 7, 2023

Accidentally pushed into this PR. The impact of the accidental push has been reverted. Old PR builds:

@gacholio gacholio merged commit 47d4e20 into eclipse-openj9:master Sep 7, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants