Skip to content

Conversation

@vtjnash
Copy link
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
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 bugfix This change fixes an existing bug 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

bugfix This change fixes an existing bug multithreading Base.Threads and related functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

make stacktrace lookup and unwinding more threadsafe

4 participants