Jackson should use HybridJacksonPool

[franz1981]

• Fixes Quarkus should use HybridJacksonPool with Loom #39738

[franz1981]

@mariofusco the problem here seems related https://github.com/FasterXML/jackson-core/blob/b8a132eefe3ccdcccfe4af444c1c3d237211fd1b/src/main/java/com/fasterxml/jackson/core/util/JsonRecyclerPools.java#L29 which create a whole new pool, meaning that the condition to verify if the user have configured the default "type" is always false given that the instances won't matches.

@geoand wdyt?

[geoand]

Seems reasonable to me

[franz1981]

@mariofusco @geoand I have found this -> FasterXML/jackson-module-scala#672 (comment)

and

FasterXML/jackson-module-scala#672 (comment)

[mariofusco]

That's ok for me as a temporary workaround, but in reality the problem should be fixed in jackson-core that when returning the defaultPool() should actually return the shared one instead of creating a new pool each and every time.

I will send a pull request to jackson-core to fix this problem, but when the change will be included in a stable release, and we will upgrade to it, I believe we should also revert this fix otherwise if jackson will decide to change the default pool with a different implementation we will break our integration again.

[franz1981]

in reality the problem should be fixed in jackson-core that when returning the defaultPool() should actually return the shared one instead of creating a new pool each and every time.

Agree, although, if users set a different instance of still that specific pool I would feel safer by NOT using it, given that is dangerous even for the non virtual thread use case ie after 3 attempts to compare and set a recycler, it will create a whole new one (!), which is not what we want regardless.
Related this specific point I believe jackson should not use that pool as default and/or having an infinite loop-like there, instead.

[mariofusco]

As anticipated I sent this pull request to jackson that should fix the problem.

[franz1981]

after speaking with @mariofusco I agree on the fact that we cannot save users which want to shoot themselves in the foot ie this PR is just a workaround, which prevent users which use the "wrong" pool type to do it, so, in theory it would be better to wait for the actual fix or merge this in meantime, depending by the time required by jackson to keep up (and us to use it). @geoand

[geoand]

👌

[mariofusco]

@geoand it seems that Jackson's maintainers don't agree with the change I proposed to avoid creating a new pool at each and every incocation of the defaultPool() method, so we necessarily need to merge this PR asap. \cc @franz1981

[geoand]

Understood, thanks

[geoand]

Mind rebasing onto main please?

[franz1981]

@geoand @gsmet I don't remember this by heart, but if the existing code path is taken regardless the JDK version in use or if we use RunOnVirtualThread, it means it is impactful for our existing users perf-wise, because the lock free pool, default on Jackson, give up on reusing the recycler (which is ~half meg) after 3 failed concurrent attempts; something that can happen, in real world. - real world ie users which dedicate few resources to containers and get contention issues NOT because too fast, but because too much sharing vs real cores :P

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 982fdb8.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

---

Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 integration-tests/virtual-threads/grpc-virtual-threads

✖ io.quarkus.grpc.example.streaming.VertxVirtualThreadTest.testUnary - History

• INTERNAL: Half-closed without a request - io.grpc.StatusRuntimeException

io.grpc.StatusRuntimeException: INTERNAL: Half-closed without a request
	at io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:268)
	at io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:249)
	at io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:167)
	at io.grpc.testing.integration.TestServiceGrpc$TestServiceBlockingStub.unaryCall(TestServiceGrpc.java:220)
	at io.quarkus.grpc.example.streaming.VirtualThreadTestBase.testUnary(VirtualThreadTestBase.java:33)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1018)