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

DefaultThreadService: allow faster JVM shutdown #394

Closed
wants to merge 1 commit into from

Conversation

maarzt
Copy link
Contributor

@maarzt maarzt commented Aug 11, 2020

I noticed that even a simple program as the following will keep the JVM alive for several seconds:

public class Main {
     public static void main(String... args) {
           new Context();
     }
}

This is caused by threads created by the DefaultThreadService, which run for a view seconds even after all the tasks are completed.

This PR fixes the late JVM shutdown. The threads created by DefaultThreadService are made deamon threads and will no longer block the JVM shutdown. This unfortunately causes the JVM to shutdown before all tasks submitted to ThreadService are completed. This is fixed by adding a LifeCounter, that counts the tasks submitted to ThreadService and keeps the JVM alive until all tasks are completed.

The JVM shutdown is usally blocked by threads of DefaultTreadService.
This commit solves this issue by making the threads deamon threads.
This causes the JVM to quit even before all submitted tasks are completed.

Therefore a "LifeCounter" is added. The life counter counts the active
tasks, and keeps the JVM alive until all submitted tasks are completed.
@hinerm
Copy link
Member

hinerm commented Aug 14, 2020

@maarzt @ctrueden I don't think this PR is necessary as-is because Context creation has an implicit expectation of calling Context#dispose when ready to shut down, in which case the process quickly exits.

But I do think we could do more to make this expectation clearer. One option would be javadoc, but another thought was having Context implement AutoCloseable with a trivial implementation:

@Override
public void close() throws Exception {
	dispose();
}

which would facilitate simple use cases:

public static void main(String... args) throws Exception {
	try (Context context = new Context()) { }
}

and, if not closed, will trigger Resource leak warnings in Eclipse. What do you think?

@maarzt
Copy link
Contributor Author

maarzt commented Aug 14, 2020

This PR breaks the behavior of System.exit() by adding the blocking shutdown hook. Thank @tpietzsch for pointing that out.

@maarzt
Copy link
Contributor Author

maarzt commented Aug 14, 2020

I'm in favor of @hinerm's suggestion to make Context implement AutoClosable.

@maarzt maarzt closed this Aug 14, 2020
hinerm added a commit that referenced this pull request Aug 14, 2020
This will provide developers hints that the Context should be disposed
after creation.

See #394
@ctrueden ctrueden deleted the faster-jvm-shutdown branch August 16, 2020 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants