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

Make use compareAndSet instead lock #2862

Closed

Conversation

laststem
Copy link
Contributor

Changes

  1. Use compareAndSwap operation: CAS operation is more efficient than lock + unlock operation.
  2. Set the parameters((superStreamRouting, producerCustomizer, ...) required to create a producer in advance : The parameters used to create a producer have no effect after the producer has been created, so i changed them to only have an effect when the producer is not initialized.
  3. Rename streamConverter to streamMessageConverter.

New found

  • If send() and close() are executed at the same time, there is a race condition where producer becomes null but trying to call a method on producer

}
return this.producer;
Copy link
Member

Choose a reason for hiding this comment

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

This is exactly the problem we are trying to cover with a Lock.
Imaging the situation when several threads concurrently call this method.
So, the first one calls compareAndSet() and enters into a block to create a producer.
The second one got false for the compareAndSet() and goes to the end of the method.
Since the first one has not finished its work for producer initialization, the second one ends up with a null.

Do I miss anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. my mistake.

@artembilan
Copy link
Member

I think we need to revert all your changes, and just add extra if (this.producer == null) { before calling lock().
This way only first calls to the createOrGetProducer() would suffer from Lock delay.

@artembilan artembilan mentioned this pull request Oct 19, 2024
@ngocnhan-tran1996
Copy link
Contributor

@laststem

As @artembilan said
I have a PR #2874 for adding condition this.producer. You can check out and continue to contributing your idea

@artembilan
Copy link
Member

Well, I even would say that this is a duplicate of that #2874. So, closed respectively .
@laststem , feel free to come back to us with any other contribution. Thank you for your effort!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants