race on detach with new threads on Windows #2611
Comments
|
The new test api.detach_spawn sometines fails on appveyor and I believe this race is the reason why. |
derekbruening
added a commit
that referenced
this issue
Oct 27, 2017
Marks api.detach_spawn as flaky as a race exists that has not yet been fixed. Issue: #2611
derekbruening
added a commit
that referenced
this issue
Oct 27, 2017
Marks api.detach_spawn as flaky as a race exists that has not yet been fixed. Issue: #2611
derekbruening
added a commit
that referenced
this issue
Nov 6, 2017
To handle a thread exiting on attach, adds a timeout to wait_event() and its corresponding implementations: os_wait_event() on Windows and ksynch_wait() on UNIX. Uses the timeout to check whether a thread exited, and if so, to move on. Augments the test from #2600 to test this race. Abandon the api.detach_spawn test on Windows:i#2611 covers fixing the tricky problems on Windows. Leaves in place some fixes toward #2611: + Fixes a race where we put interception_code hooks in place before marking them +x + Increases MAX_THREADS_WAITING_FOR_DR_INIT Fixes clang 32-bit missing __moddi3 by adding it to third_party/libgcc and linking that into x86 and arm. To enable adding race checks, moves doing_detach inside the synchall and adds started_detach for the few checks that need pre-synch querying. Removes the dynamo_thread_init_during_process_exit flag that was added in 45dd931 for #2075, as the UNIX uninit_thread_count solution from #2600 solves that problem on its own. Fixes #2601
fhahn
pushed a commit
that referenced
this issue
Dec 4, 2017
Marks api.detach_spawn as flaky as a race exists that has not yet been fixed. Issue: #2611
fhahn
pushed a commit
that referenced
this issue
Dec 4, 2017
To handle a thread exiting on attach, adds a timeout to wait_event() and its corresponding implementations: os_wait_event() on Windows and ksynch_wait() on UNIX. Uses the timeout to check whether a thread exited, and if so, to move on. Augments the test from #2600 to test this race. Abandon the api.detach_spawn test on Windows:i#2611 covers fixing the tricky problems on Windows. Leaves in place some fixes toward #2611: + Fixes a race where we put interception_code hooks in place before marking them +x + Increases MAX_THREADS_WAITING_FOR_DR_INIT Fixes clang 32-bit missing __moddi3 by adding it to third_party/libgcc and linking that into x86 and arm. To enable adding race checks, moves doing_detach inside the synchall and adds started_detach for the few checks that need pre-synch querying. Removes the dynamo_thread_init_during_process_exit flag that was added in 45dd931 for #2075, as the UNIX uninit_thread_count solution from #2600 solves that problem on its own. Fixes #2601
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
#2600 fixed this race on UNIX. On Windows, however, we cannot use the same approach of a counter because we cannot see all thread creations due to externally injected threads. Windows does have init_apc_go_native, which helps, but it still leaves a race window (one which shows up on the new api.detach_spawn test).
I tried an approach of catching threads in thread init post-detach and sending them native (via a return through the APC hook), which won't work for a shared library detach but I was hoping would work for a static lib. However, the api.detach_spawn test showed a problem: if we've attached to a thread but it hasn't been scheduled yet, it's still pointing at thread_attach_takeover_callee. We can't just return there b/c it's not a hook: it's just a gencode routine. We also can't use the existing AFTER_INTERCEPT_LET_GO_ALT_DYN because it requires a static target and we need a dynamic target for each thread's attach point.
The text was updated successfully, but these errors were encountered: