-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[#15346] Fix deserialization filtering for Externalizables and Deadlock in Map index #15358
Conversation
…nd Deadlock in Map index
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice job, some minor comments
|
||
sendJoinDatagram(new TestDeserialized()); | ||
Thread.sleep(500L); | ||
assertTrueEventually(() -> assertTrue("Object was not deserializaed", TestDeserialized.isDeserialized)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: typo in deserializaed
Hazelcast.newHazelcastInstance(config); | ||
|
||
sendJoinDatagram(new TestDeserialized()); | ||
Thread.sleep(500L); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to sleep here since we use assertTrueEventually
next
Object object = serializationService.toObject(valueBeforeCas); | ||
Object object = null; | ||
try { | ||
object = serializationService.toObject(valueBeforeCas); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch!
Thanks for review, @vbekiaris. The findings are fixed in a new commit. |
@kwart , @vbekiaris @mmedenjak : is it possible to pick up a build of the 3.12.2 jar containing this fix? If so, how/where? |
@chefhoobajoob Latest 3.12.z build is the 3.12.12.
|
@kwart : great! thank you, the visa tag was explicitly mentioned in security scans as the version to update to, so pretty confusing |
Fixes #15346 and also leaking record lock in the
Records
class.