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

Race condition in the unw_address_is_valid pipe initialization #715

Open
janvorli opened this issue Feb 1, 2024 · 4 comments
Open

Race condition in the unw_address_is_valid pipe initialization #715

janvorli opened this issue Feb 1, 2024 · 4 comments

Comments

@janvorli
Copy link
Contributor

janvorli commented Feb 1, 2024

There is a race condition in how the helper pipe is initialized in the unw_address_is_valid that was introduced by ec03043.
Before that commit, the function used pthread_once to ensure that only one thread has initialized the pipe. The above mentioned commit changed that to

if (unlikely (!atomic_flag_test_and_set(&_unw_address_validator_initialized)))
    {
      _open_pipe ();
    }

which in case of multiple thread racing here can get it into a situation when the first thread would get into the _open_pipe and another thread would use the pipe before the first thread completed the pipe opening.
In subsequent changes this pipe opening got moved to the _write_validate method, but the race is still the same.

I've observed this as random rare unwind failures in one of the .NET runtime tests where 10 threads are started at the same time and they all do the same exception handling where the libunwind is used to unwind a piece of native code. Reverting this change has fixed the issue.

@janvorli
Copy link
Contributor Author

janvorli commented Feb 1, 2024

cc: @am11

I assume moving away from pthread_once was done to make the code async signal safe.

@janvorli
Copy link
Contributor Author

janvorli commented Feb 1, 2024

@bregma I was thinking about a proper fix for the problem. I wonder if the case where we need to recreate the pipe is really possible. If not, then we could move the initialization of the pipe into the Ginit.c and thus fix the problem in a simple way.

@sec
Copy link

sec commented Mar 22, 2024

@janvorli Can't the hotfix you made for dotnet be used to fix this also here?

@janvorli
Copy link
Contributor Author

@sec, the hotfix essentially removed a commit and follow up changes from the related file. I would prefer fixing the issue and keeping all the other changes instead.

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

No branches or pull requests

2 participants