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

Add thread-per-test-class execution model #3941

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

snazy
Copy link

@snazy snazy commented Aug 31, 2024

The thread-per-test-class execution model runs tests via one thread per test class. This model is primarily useful in situations when test executions attach ThreadLocals to the test worker thread, which "leak" into other test class executions. This problem can lead to out-of-memory errors, if the ThreadLocals reference (large) object trees, either as its value or via its initial-value Supplier, which cannot be cleaned up / garbage collected, because the ThreadLocal is referenced by the test worker thread.

The problem becomes even worse, if the test class creates its own class loader and attaches ThreadLocals that reference classes loaded by such class loaders. In such cases the whole class loader including all its loaded classes and (static) state is not eligible for garbage collection, leaking more heap (and non-heap) memory.

Using one thread per test class works around the above problem(s), because once the (ephemeral) thread per test-class finishes, the whole thread and all its ThreadLocals become eligible for garbage collection.

Particularly Quarkus unit tests (@QuarkusTest annotated test classes) benefit from this execution model.

This change cannot eliminate other sources of similar leaks, like threads spawned from tests or not removed MBeans. Those kinds of leaks are better handled by the test code or the tested code providing "proper" cleanup mechanisms.

This new execution is implemented via the introduced ThreadPerClassHierarchicalTestExecutorService, a 3rd model in addition to SameThreadHierarchicalTestExecutorService and ForkJoinPoolHierarchicalTestExecutorService. It is enabled if junit.jupiter.execution.threadperclass.enabled is set to true and junit.jupiter.execution.parallel.enabled is false.

Resolves #3939.

Overview


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

@snazy snazy force-pushed the thread-per-test-class branch 2 times, most recently from d466d4c to 784cfc2 Compare August 31, 2024 16:53
The thread-per-test-class execution model runs tests via one thread per test class. This model is primarily useful in situations when test executions attach `ThreadLocal`s to the test worker thread, which "leak" into other test class executions. This problem can lead to out-of-memory errors, if the `ThreadLocal`s reference (large) object trees, either as its value or via its initial-value `Supplier`, which cannot be cleaned up / garbage collected, because the `ThreadLocal` is referenced by the test worker thread.

The problem becomes even worse, if the test class creates its own class loader and attaches `ThreadLocal`s that reference classes loaded by such class loaders. In such cases the whole class loader including all its loaded classes and (static) state is not eligible for garbage collection, leaking more heap (and non-heap) memory.

Using one thread per test class works around the above problem(s), because once the (ephemeral) thread per test-class finishes, the whole thread and all its `ThreadLocal`s become eligible for garbage collection.

Particularly Quarkus unit tests (`@QuarkusTest` annotated test classes) benefit from this execution model.

This change cannot eliminate other sources of similar leaks, like threads spawned from tests or not removed MBeans. Those kinds of leaks are better handled by the test code or the tested code providing "proper" cleanup mechanisms.

This new execution is implemented via the introduced `ThreadPerClassHierarchicalTestExecutorService`, a 3rd model in addition to `SameThreadHierarchicalTestExecutorService` and `ForkJoinPoolHierarchicalTestExecutorService`. It is enabled if `junit.jupiter.execution.threadperclass.enabled` is set to `true` and `junit.jupiter.execution.parallel.enabled` is `false`.

Issue: junit-team#3939
@snazy snazy marked this pull request as draft August 31, 2024 18:18
@snazy
Copy link
Author

snazy commented Aug 31, 2024

Marking this PR as draft, I'm reworking the changes. The current state is rather a preparation.
(Sorry, commented on the wrong PR)

@snazy
Copy link
Author

snazy commented Sep 7, 2024

/cc @sormuras


UniqueId.Segment lastSegment = testDescriptor.getUniqueId().getLastSegment();

if ("class".equals(lastSegment.getType())) {
Copy link
Contributor

@mpkorstanje mpkorstanje Sep 9, 2024

Choose a reason for hiding this comment

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

This is an implementation detail of the JUnit Jupiter Engine so it should not be used in the platform module which is test engine agnostic. Perhaps you can use TestDescriptor.getSource instead to potentially obtain the class source?

Note: I'm not part of the JUnit Team, just commenting.

Copy link
Member

@marcphilipp marcphilipp left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I'm not convinced whether it's addressing the problem on the right level, though. An alternative would be for Jupiter to provide a new ClassExecutionInterceptor extension point (or a new method in InvocationInterceptor) that would be called from ClassTestDescriptor.around. I'll bring this up for discussion with the team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "thread per test class" execution model
3 participants