Maintain libc.threads_minus_1 correctly. #1273
Conversation
crt/exit.c
Outdated
| // Decrement MUSL's number of threads. Typically this is done by | ||
| // MUSL's pthread_exit which is not called for child process's | ||
| // main thread. | ||
| __atomic_sub_fetch( |
There was a problem hiding this comment.
crt protects this with locks. Therefore this is not thread safe. You will need to patch musl to make the variable atomic.
There was a problem hiding this comment.
Good point. We need to use the same lock that MUSL uses or patch musl to make this atomic.
There was a problem hiding this comment.
Used locks to be consistent with MUSL. I considered using atomics, but felt it was simpler to just follow what MUSL is doing.
Great point. We need to keep the counter in sync when the thread becomes owned by the child process. If the child process ever hands the ownership back to the parent process, then at that point the count ought to be incremented. But I'm not sure where exactly that happens in code... |
anakrish
force-pushed
the
anakrish-libc-num-threads
branch
from
53354fd
to
014468d
Compare
src/process/execve.c under musl seems like a good place. We could either modify musl or rewrite execve under |
|
Started a nightly pipeline for the PR with broad impact. https://oe-jenkins-dev.westeurope.cloudapp.azure.com/job/Mystikos/job/Nightly-Pipeline-Manual/34/ Please wait for the outcome. |
|
Looks like the PR caused a reproducible failure on samples/go. @anakrish Please investigate. |
|
Unfortunately, the samples/go failure doesn't easily reproduce in GDB even after running for hours. I'm trying to see what other approach we can use to understand the SEGV. When run in a loop outside GDB, there is eventually a SEGV: fatal error: unexpected signal during runtime execution goroutine 3 [running]: goroutine 1 [chan receive, locked to thread]: |
|
@anakrish The go sample failures are root caused to the invalid instruction RDTSCP in Coffee Lake SGX environment. Note the above failures happen on coffee lake only. Do you think we can merge the PR now? |
@jxyang There is a SEGV failure in go samples in IceLake machines. This failure is unrelated to the RDTSCP failure in Coffee Lake machines. However, the SEGV failures can be observed in Mystikos' main branch itself. Based on that, this PR no longer has to be blocked due to go sample failures. |
Sure. Let me see if that can be addressed easily. |
The thread created for the child of a fork is not created via pthread_create. Such a thread also does not call pthread_exit. Typically, libc.threads_minus_1 is maintained by pthread_create and pthread_exit. Since these functions are not invoked for a forked child, maintain the counter manually by doing increment just before cloning to create the thread and doing decrement just before invoking SYS_group_exit for the thread. Additionally, just before execve syscall is dispatched, the thread count of the current CRT is decremented. execve will create a new process and the current thread is owned by that process. Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
anakrish
force-pushed
the
anakrish-libc-num-threads
branch
from
014468d
to
5b57be2
Compare
|
@vtikoo I have handled SYS_execve and SYS_execveat in myst_syscall. This is consistent with how fork is handled. We will get control whether execve function is called or whether SYS_execve is called via the syscall function. Note, the current fork implementation has the drawback that if fork is called via the syscall instruction, then myst_fork won't be called. Similarly, the custom thread count management won't be invoked if a syscall instruction is used for execve. |
The thread created for the child of a fork is not created via pthread_create.
Such a thread also does not call pthread_exit.
Typically, libc.threads_minus_1 is maintained by pthread_create and pthread_exit.
Since these functions are not invoked for a forked child, maintain the counter
manually by doing increment just before cloning to create the thread and
doing decrement just before invoking SYS_group_exit for the thread.
Signed-off-by: Anand Krishnamoorthi anakrish@microsoft.com