Quarkus - SmallryeHealth - PreDestroy not called on HealthCheck about quarkus

[Ladicek]

Ah the README in the reproducer has an interesting bit of information: the issue occurs when calling the health check, but when calling any other endpoint, it doesn't. So the links above are indeed very much relevant.

[olivierbeltrandocintoo]

I'll clone the reproducer and will investigate properly, but at the first sight, the @ActivateRequestContext annotation on a @RequestScoped bean seems 1. unnecessary, 2. potentially dangerous.

Also, the fact that a request-scoped bean is injected anywhere or what's the scope of the bean that injects it should not matter. A request-scoped bean must be destroyed when the request context terminates.

Hi @Ladicek ,
Sorry, the @ActivateRequestContext is an attempt of workaround (that did not work). Removing it from the sample. Thanks for the help :)

[xstefank]

@olivierbeltrandocintoo There was another issue that collided with this and will be fixed with #38888. With this PR, your reproducer works.

@gsmet I will need to fix it. In SR Health, when you invoke health, we compute a Uni for every major health group, let's say for readiness, we have readinessUni that is saved. On the next invocation, we just return this saved readinessUni once called again so Quarkus can resubscribe and call all end Uni subscriptions (health checks) again. But we save the finding and collecting of health checks into a parent Uni.

[quarkus-bot]

/cc @Ladicek (smallrye), @geoand (kotlin), @jmartisk (health,smallrye), @phillip-kruger (smallrye), @radcortez (smallrye), @xstefank (health)

[Ladicek]

I'll clone the reproducer and will investigate properly, but at the first sight, the @ActivateRequestContext annotation on a @RequestScoped bean seems 1. unnecessary, 2. potentially dangerous.

Also, the fact that a request-scoped bean is injected anywhere or what's the scope of the bean that injects it should not matter. A request-scoped bean must be destroyed when the request context terminates.

[gsmet]

@Ladicek for your awareness, if you are having a look to this one. There are a few other oddities in progress:

• #38864 (comment)
• #38866

Might not be related but better have the big picture :)

[Ladicek]

Thanks @gsmet, I'll keep that in mind!

[xstefank]

This is caused by manual Uni caching done in SmallRye Health for performance reasons. To correctly handle CDI context propagation, you need to set the quarkus.smallrye-health.context-propagation=true property - https://quarkus.io/guides/smallrye-health#context-propagation-into-the-health-check-invocations. We should add a warning if the user attempts to use CDI without this flag or maybe we should change the default to recreate Unis and just allow to set caching explicitly. WDYT @cescoffier I think that you originally proposed Uni caching in Health.

[cescoffier]

Caching is fine if you handle the duplicated context correctly - See #37077

[olivierbeltrandocintoo]

Hello @xstefank , I have tried adding the property quarkus.smallrye-health.context-propagation=true in the application.properties but the issue persists -> Ie. the @PreDestroy is not called.
You can test on the provided reproducer :)

• add the line in the properties file
• mvn clean quarkus:dev
• call http://localhost:8080/q/health/ 3 times, the 4th time it is going to get DOWN -> see logs to check that the message endOfRequest called is missing

[gsmet]

This is caused by manual Uni caching done in SmallRye Health for performance reasons.

What exactly are you caching?

[olivierbeltrandocintoo]

Hi ! I run the reproducer against Quarkus 3.7.4 that contains the fix #38888 and you are totally right, the @PreDestroy related to the RequestContext is invoked now :) !
And you were also right: the quarkus.smallrye-health.context-propagation=true needs to be set.
Thanks a lot.
Cheers,
Olivier