MonitorFdHup: replace pthread_cancel trick with a notification pipe #12714
Conversation
Mic92
force-pushed
the
pic/monitorhup-fix-pthread-cancellation
branch
from
9e495a0 to
e35b2c9
Compare
On NixOS#8946, we faced a surprising behaviour wrt. exception when using pthread_cancel. In a nutshell when a thread is inside a catch block and it's getting pthread_cancel by another one, then the original exception is bubbled up and crashes the process. We now poll on the notification pipe from the thread and exit when the main thread closes its end. This solution does not exhibit surprising behaviour wrt. exceptions. Co-authored-by: Mic92 <joerg@thalheim.io> fixes NixOS#8946
Mic92
force-pushed
the
pic/monitorhup-fix-pthread-cancellation
branch
from
e35b2c9 to
0d8a35f
Compare
Mic92
added
infra
|
on macOS: Will check if I can reproduce this. |
Syscalls can fail for many reasons and we don't want to loose the errno and error context.
Mic92
force-pushed
the
pic/monitorhup-fix-pthread-cancellation
branch
from
0d8a35f to
b39ad3a
Compare
|
Can reproduce this locally. Normal macOS tests works but this is a |
|
fyi this is https://gerrit.lix.systems/c/lix/+/1605 |
|
Similar fix also this commit predates the commit in Lix by a year. However the race condition in macOS uncovered here seems to be not tested in Lix as there is no equivalent to nix-daemon-compat-tests, so unsure if it also happens there. I can reproduce it the issue without the commits here by running tests in isolation on macOS and they go away if I add sleeps before the garbage collection calls. The issue is that there are still temproots shown as hold by the nix-daemon preventing the garbage collection temporarily. |
Motivation
On #8946, we faced a surprising
behaviour wrt. exception when using pthread_cancel. In a nutshell when
a thread is inside a catch block and it's getting pthread_cancel by
another one, then the original exception is bubbled up and crashes the
process.
We now poll on the notification pipe from the thread and exit when the
main thread closes its end. This solution does not exhibit surprising
behaviour wrt. exceptions.
Context
#8946
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.