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

ServerSerialisers static init should change Serialisers primitive lookup strategy #37778

Closed
wants to merge 1 commit into from

Conversation

franz1981
Copy link
Contributor

Fixes #37776

@franz1981 franz1981 force-pushed the lookup_primitive_wrapper branch from 84fe353 to 39d3475 Compare December 15, 2023 15:19
@franz1981
Copy link
Contributor Author

franz1981 commented Dec 15, 2023

@geoand this is weird (meaning that I wasn't expecting this dependency, but reading the code twice was clear!): this is really the expected behaviour?

eg

  1. ClientSerialisers is used and no primitive wrappers lookup happen
  2. ServerSerialisers is now used as well
  3. both ServerSerialisers and ClientSerialisers start to lookup primitive types

Maybe this is just a non possible situation, and everything always happen in a single thread, and in a given order.
In that case I can remove the volatile keyword or we can move this "setup" in who will make these classes to be loaded
and can "configure" them in order that when ServerSerialisers is configured, all the serializers behave the same.

@geoand
Copy link
Contributor

geoand commented Dec 15, 2023

I haven't debugged yet, but my guess is that ClientSerializers are now wrong, as opposed to the case before the change

@franz1981
Copy link
Contributor Author

@geoand

as opposed to the case before the change

I've restored the "original" behaviour (before #37655) with this one
ie static init of SerialSerialisers influence the behaviour of all Serialisers's son.

If the original behaviour need to be changed for the case of #37778 (comment) let me know and i'll put it in draft, awaiting a different fix on monday - I don't think we're in hurry (it's nearly weekend :P)

@geoand
Copy link
Contributor

geoand commented Dec 15, 2023

To be honest, we always need the primitive lookup.

You could have only the client part without the server but that still needs to work with primitives

Copy link

quarkus-bot bot commented Dec 15, 2023

Failing Jobs - Building 39d3475

Status Name Step Failures Logs Raw logs Build scan
Native Tests - Amazon Update Docker Client User Agent ⚠️ Check → Logs Raw logs 🚧
Native Tests - Data2 Update Docker Client User Agent ⚠️ Check → Logs Raw logs 🚧
Native Tests - Data3 Update Docker Client User Agent ⚠️ Check → Logs Raw logs 🚧

@geoand
Copy link
Contributor

geoand commented Dec 16, 2023

@franz1981 thanks for this, but I actually prefer #37788 since its is more correct and also contains a test.

@geoand geoand closed this Dec 16, 2023
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/invalid This doesn't seem right
Projects
None yet
2 participants