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

Avoid potential race condition in DeferredLogstashMarker#getSuppliedV… #386

Conversation

PascalSchumacher
Copy link
Contributor

…alue by not publishing a null LogstashMarker.

By the way: Thank you very much for developing logstash-logback-encoder and a happy new year!

…alue by not publishing a null LogstashMarker.
@PascalSchumacher PascalSchumacher force-pushed the DeferredLogstashMarker_avoid_potential_race_condition branch from 1391c08 to a0f4b04 Compare January 1, 2020 18:24
@philsttr
Copy link
Collaborator

philsttr commented Jan 5, 2020

I don't think this actually fixes any race conditions, since the changed code is within a synchronized block, but I'll merge it anyway, since it removes a volatile read/write.

@philsttr philsttr merged commit 687b575 into logfellow:master Jan 5, 2020
@philsttr philsttr added this to the 6.4 milestone Jan 5, 2020
@PascalSchumacher
Copy link
Contributor Author

PascalSchumacher commented Jan 6, 2020

On further thinking I belive you are correct.

It is not a race condition, because only null can be visible to other threads before the end of the synchronized block. And if the value is null they will block waiting for the lock.

Sorry for the missleading comment and pull request title. :-(

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