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

Fixes 841: Incorrect reference used in compareAndSet in CTrie.cleanTomb. #842

Merged

Conversation

hylkevds
Copy link
Collaborator

cleanTomb used compareAndSet to update a reference, but incorrectly re-fetched the 'original' instead of using the version that was used to make the copy. The result was that in case of a conflict, the changes of one thread would be overwritten by another thread.

If cleanTomb failes when called from the remove method, the node is already replaced by a TNode, and the subsequent call will
re-try to clean the TNode. The node removed by cleanTomb may already have been replaced with a live node by another thread, so
cleanTomb checks if the removed node actually was the intended node before committing the results.

  • My code follows the style guidelines of this project

  • I have commented my code, particularly in hard-to-understand areas

  • I have made corresponding changes to the documentation

  • I have made corresponding change to the default configuration files (and/or docker env variables)

  • I have added tests that prove my fix is effective or that my feature works

  • I have updated the Changelog if it's a feature or a fix that has to be reported

  • bug

  • Closes High CPU usage when run Ctrie.insert function #841

hylkevds added 2 commits July 29, 2024 16:13
incorrectly re-fetched the 'original' instead of using the version that
was used to make the copy.
@hylkevds hylkevds force-pushed the fix_841_cleanTomb_reference_loss branch from efff085 to 4af3483 Compare July 29, 2024 14:13
Copy link
Collaborator

@andsel andsel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Great catch @hylkevds a BIG 👍 thank's for the fix

@andsel andsel merged commit 9e1fed1 into moquette-io:main Aug 17, 2024
1 of 4 checks passed
@hylkevds hylkevds deleted the fix_841_cleanTomb_reference_loss branch August 25, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

High CPU usage when run Ctrie.insert function
2 participants