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 #3939

Open
snazy opened this issue Aug 30, 2024 · 6 comments
Open

Add "thread per test class" execution model #3939

snazy opened this issue Aug 30, 2024 · 6 comments

Comments

@snazy
Copy link

snazy commented Aug 30, 2024

I recently ran into an ugly OOM caused by ThreadLocals produced via different class loaders (Quarkus test class loaders) leaking into the singleton Test worker thread. Some background on the issue is here and on the Quarkus chat.

Long story short: I was able to work around the ThreadLocal-OOM by running each test class on its own dedicated thread, as implemented in this PR.

I wonder whether the JUnit project would be open for a PR to implement such a "thread per test class" model - basically a 3rd implementation in addition to SameThreadHierarchicalTestExecutorService and ForkJoinPoolHierarchicalTestExecutorService. This new execution model seems useful for other Quarkus users as well.

@sormuras
Copy link
Member

Sounds interesting to me.

snazy added a commit to snazy/junit5 that referenced this issue 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 `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 added a commit to snazy/junit5 that referenced this issue 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 `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 added a commit to snazy/junit5 that referenced this issue 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 `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 added a commit to snazy/junit5 that referenced this issue 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 `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 added a commit to snazy/junit5 that referenced this issue 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 `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 added a commit to snazy/junit5 that referenced this issue 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 `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
@marcphilipp
Copy link
Member

Team decision: We don't think such an executor is generally useful enough to justify its inclusion in JUnit. However, providing a mechanism to configure a custom HierarchicalTestExecutorService is something we are willing to consider.

@snazy Would that meet your needs?

Copy link

github-actions bot commented Dec 7, 2024

If you would like us to be able to process this issue, please provide the requested information. If the information is not provided within the next 3 weeks, we will be unable to proceed and this issue will be closed.

@gsmet
Copy link

gsmet commented Dec 7, 2024

/cc @geoand @holly-cummins ^^

@geoand
Copy link

geoand commented Dec 7, 2024

Thanks for the heads up!

Copy link

If you would like us to be able to process this issue, please provide the requested information. If the information is not provided within the next 3 weeks, we will be unable to proceed and this issue will be closed.

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

Successfully merging a pull request may close this issue.

5 participants