Skip to content

rabbitmq-java-client #120: unsynchronized access to recordedBindings gives undesired results #121

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

Merged
merged 1 commit into from
Dec 30, 2015
Merged

Conversation

spatula75
Copy link

Fix is to simply make removeBindingsWithDestination a synchronized method (like the other methods which access recordedBindings).

@michaelklishin
Copy link
Contributor

Other similar methods synchronise on the specific fields they mutate. Can we make this synchronise on recordedBindings?

@michaelklishin michaelklishin self-assigned this Dec 30, 2015
@spatula75
Copy link
Author

I'm sure it's possible; however, all of the other methods which are currently synchronized would need to change to the field-synchronized strategy as well. There's sort of a mix of different synchronization styles in play currently.

@michaelklishin
Copy link
Contributor

Ah, yes, there are synchronized methods in AutorecoveringConnection, too. Alright then.

michaelklishin added a commit that referenced this pull request Dec 30, 2015
rabbitmq-java-client #120: unsynchronized access to recordedBindings gives undesired results
@michaelklishin michaelklishin merged commit 500120b into rabbitmq:stable Dec 30, 2015
@michaelklishin michaelklishin added this to the n/a milestone Dec 30, 2015
@spatula75
Copy link
Author

Yeah, it might be slightly less-than-ideal, but probably a bigger project to revise how concurrency works in that particular class.

@michaelklishin
Copy link
Contributor

@spatula75 agreed. Thank you!

@spatula75 spatula75 deleted the rabbitmq-java-client-120 branch January 4, 2016 22:28
stream-iori pushed a commit to stream-iori/rabbitmq-java-client that referenced this pull request Mar 20, 2022
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.

2 participants