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

win32: fix in_stackwalk mutex #37002

Merged
merged 1 commit into from
Aug 14, 2020
Merged

win32: fix in_stackwalk mutex #37002

merged 1 commit into from
Aug 14, 2020

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Aug 11, 2020

This should make this previously non-atomic mutex threadsafe.

Fixes #35117

This should make this previously non-atomic mutex threadsafe.

Fixes #35117
@vtjnash vtjnash requested a review from Keno August 11, 2020 15:43
@Keno
Copy link
Member

Keno commented Aug 11, 2020

This is supposed to lock out doing a backtrace while adding new symbols? What happens when the profiler fires while we're in that code? Shouldn't there be a trylock somewhere that bails if we're trying to stackwalk while in symbol registration?

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Aug 11, 2020

I intended to have the profiler get the lock before stopping any threads. I considered alternatively using trylock, but hoped it shouldn't add too much noise (beyond the problems already seen with profiling thread scaling in the presence of locks #9224). I think I may have forgotten to finish with doing that part of the PR however (cf. your past work in #34454 and open issue #13294)

@Keno
Copy link
Member

Keno commented Aug 11, 2020

Hmm, I see, I guess that works on win/mac since the profiler starts on a separate thread. This LGTM then.

@JeffBezanson JeffBezanson added kind:bugfix This change fixes an existing bug domain:multithreading Base.Threads and related functionality labels Aug 12, 2020
@JeffBezanson JeffBezanson merged commit 8b3cb2f into master Aug 14, 2020
@JeffBezanson JeffBezanson deleted the jn/in_stackwalk branch August 14, 2020 20:14
simeonschaub pushed a commit to simeonschaub/julia that referenced this pull request Aug 29, 2020
This should make this previously non-atomic mutex threadsafe.

Fixes JuliaLang#35117
@Sacha0 Sacha0 mentioned this pull request Oct 23, 2020
15 tasks
Sacha0 pushed a commit that referenced this pull request Oct 23, 2020
This should make this previously non-atomic mutex threadsafe.

Fixes #35117
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:multithreading Base.Threads and related functionality kind:bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make stacktrace lookup and unwinding more threadsafe
3 participants