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

SseEmitter is not thread safe [SPR-13224] #17815

Closed
spring-projects-issues opened this issue Jul 11, 2015 · 3 comments
Closed

SseEmitter is not thread safe [SPR-13224] #17815

spring-projects-issues opened this issue Jul 11, 2015 · 3 comments
Assignees
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jul 11, 2015

Tomasz Nurkiewicz opened SPR-13224 and commented

SseEmitter misbehaves when multiple threads call send() at the same time. Sometimes container fails, but most of the time different messages get interleaved, e.g.:

emitter.send("foo");  //thread A
emitter.send("bar");  //thread B

sometimes results in:

data:data:AB

It happens because emitter.send("A") actually sends three independent messages: data:, A and \n\n. These three messages can be interleaved with messages from different threads. Please either synchronize properly or document clearly that SseEmitter is not thread safe, so that if clients want to use from multiple threads (I think it's quite useful), it must be synchronized manually:

synchronized(sseEmitter) {
  sseEmitter.send("foo");
}

Affects: 4.2 RC2

Issue Links:

  • #17814 ResponseBodyEmitter skips same messages during initialization

Referenced from: commits bdb6348

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 13, 2015

Juergen Hoeller commented

Implemented together with #17814, using a common struct for a value with media type both in ResponseBodyEmitter and in SseEventBuilder, and using common synchronization for all affected operations - including the option to manually synchronize on the emitter, for aggregating multiple values just like SseEmitter does by default now.

Juergen

@martinandersson
Copy link

@martinandersson martinandersson commented May 4, 2021

Can you please re-open this issue? The thread-safety of relevant types are still not documented, leading to 1) time waste for people concerned about thread-safety having to google this - and that should be most of us, 2) developers programming against a contract must manually synchronize regardless of implementation details they might have come across online - and that should be most of us.

@snicoll
Copy link
Member

@snicoll snicoll commented May 4, 2021

@martinandersson we're not going to reopen an issue that was closed in a released version of the framework. Please create a separate issue with additional details against the current GA version.

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

Successfully merging a pull request may close this issue.

None yet
4 participants