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

Fix write conflicts in parallel output #1068

Merged
merged 2 commits into from
Apr 14, 2022
Merged

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Apr 14, 2022

Replace instance lock with static lock, because there can be multiple different ThreadSafeStringWriters writing into the same async context. Reduce the locking only to state initialization and dictionary lookups and edits, but don't hold the lock over StringBuilder operations.

Add a wrapper for StringBuilder that locks on every method on the instance level, so when two threads get the same instance of StringBuilder then will sync together, but they won't sync with other threads that have different string builder.

Fix #1063

Haplois
Haplois previously approved these changes Apr 14, 2022
@nohwnd
Copy link
Member Author

nohwnd commented Apr 14, 2022

I've used the project from #1063 (comment) that @JustArchi so helpfully provided, and run it 100 times in a loop without error. And without any regression in execution time.

This might be slightly improved by locking only when we grab or initialize the State, and then making the edits on concurrent dictionary, but it adds more logic and I guess the improvements would be marginal.

…13ThreadSafeStringWriter.cs

Co-authored-by: Amaury Levé <amaury.leve@gmail.com>
@nohwnd nohwnd dismissed stale reviews from Evangelink and Haplois via f9f089c April 14, 2022 10:48
@nohwnd nohwnd enabled auto-merge (squash) April 14, 2022 10:48
@nohwnd nohwnd merged commit 464b117 into microsoft:main Apr 14, 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.2.9 Concurrency does not seem to work properly
3 participants