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

Serious, hard to trace bug in core DirtyCheckingSupport class #1462

Closed
lkuchars opened this issue May 31, 2021 · 0 comments · Fixed by #1490
Closed

Serious, hard to trace bug in core DirtyCheckingSupport class #1462

lkuchars opened this issue May 31, 2021 · 0 comments · Fixed by #1490

Comments

@lkuchars
Copy link

lkuchars commented May 31, 2021

Grails 4.0.3
grails-datastore-core:7.0.6-RELEASE
grails-datastore-gorm-mongodb:7.0.1

I Am sorry I will not be able to provide you with unit test for this. This error is too intricate. I'm chasing this problem for days on 2 production systems where mongo mapped Domain entities are not updated in database because dirty properties are not detected properly.

Let me explain why this happens and hopefully you can follow me with the reasoning as this, I believe is sth that is almost impossible to debug.

It all boils down to this fragment and how the marker in question is initialized.

Screenshot_20210531_172324

Because ^^^^^ it is mutable and assigned around to other references in the code the marker can and eventually is modified. In specific circumstances it comes to the point where my system in question lands in following state:

Screenshot_20210531_172823

And event which make my system goes into this state is pretty common:

Screenshot_20210531_172949
It turns out that in my case when OptimisticLockingException is thrown ^^^^^^ it causes DirtyCheckingSupport.DIRTY_CLASS_MARKER left non empty. Well i guess it should not be modifiable in the first place. Let me mention that I DO NOT modify or use those references anywhere in my code.
I guess that it is inevitable that when mutable collection is used as marker, for example in DirtyCheckable.groovy like this:

Screenshot_20210531_173252
enetually $changedProperties will be modified by someone and marker will be modified too.

My suggestion is to initialized this marker like this:
Map DIRTY_CLASS_MARKER = [:].asImmutable()

Then at least we will know where and when another library tries to modify it and we will fail fast

Thanks

puneetbehl added a commit that referenced this issue Jul 27, 2021
Initialize DIRTY_CLASS_MARKER as Immutable collection, as marker should not be modified
Fixes #1462

Signed-off-by: Puneet Behl <[email protected]>
puneetbehl added a commit that referenced this issue Jul 27, 2021
Initialize DIRTY_CLASS_MARKER as Immutable collection, as marker should not be modified
Fixes #1462

Signed-off-by: Puneet Behl <[email protected]>
@puneetbehl puneetbehl linked a pull request Jul 27, 2021 that will close this issue
@puneetbehl puneetbehl added this to the 7.0.9 milestone Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants