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

stacktrace_libunwind-inl.h: reentrancy check should use a thread-local instead of a global variable #160

Closed
mpercy opened this issue Jan 23, 2017 · 0 comments · Fixed by #161

Comments

@mpercy
Copy link
Contributor

mpercy commented Jan 23, 2017

At the time of writing, in the implementation of google::GetStackTrace() in src/stacktrace_libunwind-inl.h there is a global variable that enforces that only one thread may invoke libunwind at a time. However, libunwind is thread-safe. The comment above the variable declaration indicates that this is to protect against reentrancy issues:

// Sometimes, we can try to get a stack trace from within a stack
// trace, because libunwind can call mmap (maybe indirectly via an
// internal mmap based memory allocator), and that mmap gets trapped
// and causes a stack-trace request.  If were to try to honor that
// recursive request, we'd end up with infinite recursion or deadlock.
// Luckily, it's safe to ignore those subsequent traces.  In such
// cases, we return 0 to indicate the situation.
static bool g_now_entering = false;

Instead of using a global variable, it would be much better to use a thread-local variable to protect against these reentrancy issues. That should provide the needed reentrancy protection while allowing multiple threads to get stack traces at the same time. It would also allow for the removal of the atomic CAS operations on the g_now_entering variable.

mpercy added a commit to mpercy/glog that referenced this issue Jan 28, 2017
Previously, the implementation of google::GetStackTrace() that uses
libunwind uses a global variable that enforces that only one thread may
invoke libunwind at a time. However, libunwind is thread-safe. The
comment above the variable indicates that it is to protect against
reentrancy issues.

Instead of using a global variable, it would be much better to use a
thread-local variable to protect against these reentrancy issues. That
should provide the needed reentrancy protection while allowing multiple
threads to get stack traces at the same time.

It also allows for the removal of the atomic CAS operations on the
variable.

Resolves google#160.
@sergiud sergiud mentioned this issue May 6, 2021
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 a pull request may close this issue.

1 participant