search

quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.7k stars 2.66k forks source link

TCKs - `QuarkusTestNgCallbacks.invokeTestNgAfterClasses` executed using a class loader that has been closed #41743

Open gsmet opened 3 months ago

gsmet commented 3 months ago

[!NOTE] This is part of my efforts to improve our CL infrastructure and is related to this PR: https://github.com/quarkusio/quarkus/pull/41608 . In this PR, I throw an exception when we access a closed CL to identify issues. At some point, I hope it could be the default behavior as it could allow us to catch some issues early. For now, we are unfortunately not there yet.

When running CI with this PR ^ applied, I get a lot of TCK failures similar to:

2024-07-06T09:31:19.4409078Z Caused by: java.lang.IllegalStateException: Class loader QuarkusClassLoader:Quarkus Base Runtime ClassLoader: TEST@513b83f0 has been closed and may not be accessed anymore
2024-07-06T09:31:19.4409805Z    at io.quarkus.bootstrap.classloading.QuarkusClassLoader.ensureOpen(QuarkusClassLoader.java:743)
2024-07-06T09:31:19.4410497Z    at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:507)
2024-07-06T09:31:19.4411310Z    at io.quarkus.arquillian.QuarkusTestNgCallbacks.invokeTestNgAfterClasses(QuarkusTestNgCallbacks.java:47)

This issue is that the test instances are instantiated using the base runtime of the started Quarkus app and by the time we try to execute the after classes callback, the application is stopped and the base runtime class loader is closed.

I'm not entirely sure there is a way to work around this problem but still from a design point of view it seems inappropriate to load classes from a closed class loader.

I would be interested in feedback from @mkouba @Ladicek and @manovotn .

manovotn commented 3 months ago

IIRC, QuarkusTestNgCallbacks.invokeTestNgAfterClasses was a workaround to be able to invoke test lifecycle callbacks which was basically implemented because of the need to have Quarkus specific CL to begin with. Actually, from the GH history, it seems like we implemented this because MP TCKs used this behavior even back then.

I'm not entirely sure there is a way to work around this problem but still from a design point of view it seems inappropriate to load classes from a closed class loader.

I don't think you can use different CL for these callbacks; it should be the same one that was used for tests. I assume there is a reason we cannot close the base CL only after these callbacks have been executed?

quarkus-bot[bot] commented 3 months ago

/cc @geoand (testing)

dmlloyd commented 3 months ago

A possibly simple/effective workaround for this situation is to pre-load the classes that are going to be loaded at the end, before the CL is closed. It requires a little bit of manual work though.