Runtime init the cleaner instance in ResourceCleaner

[jerboaa]

Inject an accessor for the ResourceCleaner.CLEANER static variable so that the associated cleaner won't get initialized at build time and ends up being in the native image heap.

Closes: #31440

[quarkus-bot]

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

• title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

[jerboaa]

Mandrel workflow with this is running here: https://github.com/graalvm/mandrel/actions/runs/4285051293

[jerboaa]

It should probably be in the resteasy-common extension rather than resteasy-client as I had it before.

[jerboaa]

@zakkak Please take a look when you get a chance! Thanks.

[zakkak]

LGTM, but did you try just reinitializing the class instead?

[jerboaa]

... but did you try just reinitializing the class instead?

I did not. Then again, wouldn't for runtime-reinited classes the same restrictions apply (instances of those classes are disallowed in the image heap)? I'll have a look and will report back.

[zakkak]

... but did you try just reinitializing the class instead?

I did not. Then again, wouldn't for runtime-reinited classes the same restrictions apply (instances of those classes are disallowed in the image heap)?

No, fields of reinited classes are treated differently. The issue with instances of certain classes being present in the native heap is that those instances were generated at build time and they are thus not valid at runtime. When reiniting a class the said fields are "uninitialized" in the initial native heap and get initialized at run time which is valid.

[jerboaa]

... but did you try just reinitializing the class instead?

I did not. Then again, wouldn't for runtime-reinited classes the same restrictions apply (instances of those classes are disallowed in the image heap)?

No, fields of reinited classes are treated differently. The issue with instances of certain classes being present in the native heap is that those instances were generated at build time and they are thus not valid at runtime. When reiniting a class the said fields are "uninitialized" in the initial native heap and get initialized at run time which is valid.

I thought this is for runtime initialized classes. Runtime re-inited classes are initialized both at build time and again at runtime:
https://github.com/oracle/graal/blob/master/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/InitKind.java#L44..L45

Anyway, in this case both seem to work.

Runtime-reinit:
https://github.com/quarkusio/quarkus/compare/main...jerboaa:quarkus:runtime-reinit-resource?expand=1
Runtime-init:
https://github.com/quarkusio/quarkus/compare/main...jerboaa:quarkus:runtime-init-resource?expand=1

Shall I update this patch with the last (runtime-init) option?

[zakkak]

I thought this is for runtime initialized classes. Runtime re-inited classes are initialized both at build time and again at runtime: https://github.com/oracle/graal/blob/master/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/InitKind.java#L44..L45

Correct, they are initialized at build time for the build to proceed and to increase the level of optimization, but the values from the static fields are not used at runtime (if I am not mistaken they are not even placed in the generated native heap), instead they get re-initialized at run time. Because of this they are allowed to be present in the heap.

Anyway, in this case both seem to work.

I didn't suggest pure runtime initialization because I thought the class needed to be build-time initialized. Great that you tried both.

Runtime-reinit: https://github.com/quarkusio/quarkus/compare/main...jerboaa:quarkus:runtime-reinit-resource?expand=1 Runtime-init: https://github.com/quarkusio/quarkus/compare/main...jerboaa:quarkus:runtime-init-resource?expand=1

Shall I update this patch with the last (runtime-init) option?

Yes, please.

[zakkak]

Runtime-init: https://github.com/quarkusio/quarkus/compare/main...jerboaa:quarkus:runtime-init-resource?expand=1

@jerboaa I suggest doing the runtime initialization registration in https://github.com/quarkusio/quarkus/blob/main/extensions/resteasy-classic/resteasy-common/deployment/src/main/java/io/quarkus/resteasy/common/deployment/ResteasyCommonProcessor.java instead of creating a new class.

[jerboaa]

@zakkak Done now. Thanks for the reviews. I've also rescheduled a run of the mandrel tests with this:
https://github.com/graalvm/mandrel/actions/runs/4285051293

[zakkak]

LGTM, thanks!

[jerboaa]

@gsmet Hi! Could you please help approving the workflow and getting this integrated? It fixes many broken integration tests on the mandrel CI side. Thanks!

[gsmet]

@zakkak just to be sure, you have the permission to approve a workflow run, right? Wondering if it's only admins.

[zakkak]

@zakkak just to be sure, you have the permission to approve a workflow run, right? Wondering if it's only admins.

Yes, I was the one who approved it this morning. I am also going to merge this once the CI completes. @jerboaa was probably not aware that I have the power to do both :)

Probably it didn't help that I didn't notice that the workflow was blocked for some unknown (since this is not the first contribution of @jerboaa) reason.

[quarkus-bot]

Failing Jobs - Building 37b5375

Status	Name	Step	Failures	Logs	Raw logs
✔️	Devtools Tests - JDK 11
✔️	Devtools Tests - JDK 11 Windows
✖	Devtools Tests - JDK 17	Build	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Devtools Tests - JDK 17 #

- Failing: integration-tests/devtools 

📦 integration-tests/devtools

✖ io.quarkus.devtools.commands.CreateProjectTest.createMultipleTimes line 254 - More details - Source on GitHub

java.util.concurrent.TimeoutException: createMultipleTimes() timed out after 3 seconds
	at org.junit.jupiter.engine.extension.TimeoutExceptionFactory.create(TimeoutExceptionFactory.java:29)
	at org.junit.jupiter.engine.extension.SameThreadTimeoutInvocation.proceed(SameThreadTimeoutInvocation.java:58)

[zakkak]

The failure is unrelated to the PR. Merging it.

[zakkak]

Marking this for backport since although the Cleaner instances were disallowed only recently the issue is actually present in older versions as well.

Not sure if it's worth backporting to 2.7 as well.

[gsmet]

Actually, AFAICS, ResourceCleaner doesn't exist in 2.16.

[zakkak]

Indeed, the ResourceCleaner was added in 6.1.0.Final and 2.16 is using 4.7.7.