DynamoRIO / DynamoRIO/dynamorio
Signals received by threads during the synch process can deadlock
- Dominant language
- C
- Stars
- 3.2k
- Forks
- 629
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 30
Description
In PR #3249 we moved the removal of itimer handlers to earlier in the detach sequence because it was causing a deadlock we noticed internally. The race went something like:
0. Thread A has executed `dr_app_setup()` and `dr_app_start()`; Thread B is executing from the code-cache
1. Thread A initiates detach via `dr_app_stop_and_cleanup()` and attempts to synch with thread B; it increments thread B's `pending_synch_count`
2. On its next trip through dispatch, thread B blocks in `check_wait_at_safe_spot()` after setting its synch permission to `THREAD_SYNCH_VALID_MCONTEXT`
3. Thread B receives a signal (`SIGPROF`, or really any arbitrary signal sent to this thread which calls through `record_pending_signal`)
4. In `record_pending_signal`, thread B obtains the `fcache_unit_areas` readlock
5. Thread A then sends the `SUSPEND_SIGNAL` to thread B
6. Thread B receives the `SUSPEND_SIGNAL` (nested in the previous `SIGPROF`, while thread B still holds the `fcache_unit_areas` readlock)
7. Thread A synchs with thread B, finds that thread B has a safe synch perm from `check_wait_at_safe_spot`, and sends thread B native
8. At some point in the future, thread A attempts to obtain the `fcache_unit_areas` writelock and blocks forever
We somewhat mitigated this in PR #3249 in that the SIGPROF in question could not be received after a thread is blocked in `check_wait_at_safe_spot` for detach, but there are still races which are possible today:
* Thread B could have been recently blocking in `check_wait_at_safe_spot` from a previous synch when a signal comes and causes the previous set of events to occur.
* Many signals cause the `fcache_unit_areas` to be accessed, and any signal can be sent to any thread at any time by some other process not under DR control. Disabling just timer handlers only protects us from just a subset of signals, though in practice it's unlikely for this to affect a large number of use cases.
Contributor guide
Research direction
Start at dr_app_stop_and_cleanup(), check_wait_at_safe_spot(), and record_pending_signal(), then compare the detach changes in PR #3249. Done means the described signal and detach interleavings no longer leave fcache_unit_areas blocked indefinitely; the issue does not name a file or test, so locate the relevant synchronization coverage first.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100