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 deadlock in windows profiler #34454

Merged
merged 1 commit into from
Jan 28, 2020
Merged

Fix deadlock in windows profiler #34454

merged 1 commit into from
Jan 28, 2020

Conversation

Keno
Copy link
Member

@Keno Keno commented Jan 21, 2020

The deadlock here happens when the main thread is trying to register
a new JIT object while the profiler is triggered. The main thread
will acquire the objectmap lock and then get suspended. Then the profiler
thread will attempt to stackwalk, but in order to do that effectively,
it needs to look at the object map to find the unwind info for the function
on the stack. We don't have the same problem on linux because the main
thread runs the backtracing. On linux and mac the registration process
works differently and on linux at least the backtracing happens on a
different thread so we may not have this problem. However, #13294 says
that it is also a problem on OS X. We should keep an eye out for it.
For the moment, just try to fix this by terminating the stack unwind
when we fail to acquire the lock. That's not ideal, because it reduces
the quality of the profiler info, but only in situations where we would
previous deadlock. This entire code needs a rewrite, but for now, I'm
just hoping to get CI to stopping deadlocking on us.

The deadlock here happens when the main thread is trying to register
a new JIT object while the profiler is triggered. The main thread
will acquire the objectmap lock and then get suspended. Then the profiler
thread will attempt to stackwalk, but in order to do that effectively,
it needs to look at the object map to find the unwind info for the function
on the stack. We don't have the same problem on linux because the main
thread runs the backtracing. On linux and mac the registration process
works differently and on linux at least the backtracing happens on a
different thread so we may not have this problem. However, #13294 says
that it is also a problem on OS X. We should keep an eye out for it.
For the moment, just try to fix this by terminating the stack unwind
when we fail to acquire the lock. That's not ideal, because it reduces
the quality of the profiler info, but only in situations where we would
previous deadlock. This entire code needs a rewrite, but for now, I'm
just hoping to get CI to stopping deadlocking on us.
@Keno
Copy link
Member Author

Keno commented Jan 21, 2020

How many of these do we have?

@JeffBezanson JeffBezanson merged commit b37d094 into master Jan 28, 2020
@JeffBezanson JeffBezanson deleted the kf/win32profdeadlock branch January 28, 2020 18:17
KristofferC pushed a commit that referenced this pull request Apr 11, 2020
The deadlock here happens when the main thread is trying to register
a new JIT object while the profiler is triggered. The main thread
will acquire the objectmap lock and then get suspended. Then the profiler
thread will attempt to stackwalk, but in order to do that effectively,
it needs to look at the object map to find the unwind info for the function
on the stack. We don't have the same problem on linux because the main
thread runs the backtracing. On linux and mac the registration process
works differently and on linux at least the backtracing happens on a
different thread so we may not have this problem. However, #13294 says
that it is also a problem on OS X. We should keep an eye out for it.
For the moment, just try to fix this by terminating the stack unwind
when we fail to acquire the lock. That's not ideal, because it reduces
the quality of the profiler info, but only in situations where we would
previous deadlock. This entire code needs a rewrite, but for now, I'm
just hoping to get CI to stopping deadlocking on us.
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.

None yet

2 participants