Skip to content

Commit 035b0e2

Browse files
jartjart
committed
Attempt to fix MODE=dbg Windows execve() flake
1 parent 7b67b20 commit 035b0e2

File tree

5 files changed

+69
-85
lines changed

5 files changed

+69
-85
lines changed

‎libc/intrin/sig.c

Lines changed: 57 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ __msabi extern typeof(VirtualProtectEx) *const __imp_VirtualProtectEx;
8787
__msabi extern typeof(VirtualQuery) *const __imp_VirtualQuery;
8888
__msabi extern typeof(WriteFile) *const __imp_WriteFile;
8989

90-
extern pthread_mutex_t __sig_worker_lock;
90+
atomic_int __sig_worker_state;
9191

9292
textwindows static bool __sig_ignored_by_default(int sig) {
9393
return sig == SIGURG || //
@@ -742,74 +742,77 @@ HAIRY static uint32_t __sig_worker(void *arg) {
742742
STKSZ, PROT_READ | PROT_WRITE,
743743
MAP_PRIVATE | MAP_ANONYMOUS | MAP_NOFORK);
744744
for (;;) {
745-
_pthread_mutex_lock(&__sig_worker_lock);
746-
747-
// dequeue all pending signals and fire them off. if there's no
748-
// thread that can handle them then __sig_generate will requeue
749-
// those signals back to __sig.process; hence the need for xchg
750-
unsigned long sigs =
751-
atomic_exchange_explicit(__sig.process, 0, memory_order_acq_rel);
752-
while (sigs) {
753-
int sig = bsfl(sigs) + 1;
754-
sigs &= ~(1ull << (sig - 1));
755-
__sig_generate(sig, SI_KERNEL);
756-
}
757-
758-
// unblock stalled i/o signals in threads
759-
_pthread_lock();
760-
for (struct Dll *e = dll_first(_pthread_list); e;
761-
e = dll_next(_pthread_list, e)) {
762-
struct PosixThread *pt = POSIXTHREAD_CONTAINER(e);
763-
if (atomic_load_explicit(&pt->pt_status, memory_order_acquire) >=
764-
kPosixThreadTerminated)
765-
break;
766-
if (atomic_load_explicit(&pt->pt_blocker, memory_order_acquire) &&
767-
(atomic_load_explicit(&pt->tib->tib_sigpending,
768-
memory_order_acquire) &
769-
~atomic_load_explicit(&pt->pt_blkmask, memory_order_acquire)))
770-
__sig_wake(pt, 0);
771-
}
772-
_pthread_unlock();
745+
// ok sys_execve_nt() might disable this worker
746+
if (~__sig_worker_state & 2) {
747+
748+
// dequeue all pending signals and fire them off. if there's no
749+
// thread that can handle them then __sig_generate will requeue
750+
// those signals back to __sig.process; hence the need for xchg
751+
unsigned long sigs =
752+
atomic_exchange_explicit(__sig.process, 0, memory_order_acq_rel);
753+
while (sigs) {
754+
int sig = bsfl(sigs) + 1;
755+
sigs &= ~(1ull << (sig - 1));
756+
__sig_generate(sig, SI_KERNEL);
757+
}
773758

774-
// unblock stalled asynchronous signals in threads
775-
for (;;) {
776-
sigset_t pending, mask;
777-
struct PosixThread *mark = 0;
759+
// unblock stalled i/o signals in threads
778760
_pthread_lock();
779761
for (struct Dll *e = dll_first(_pthread_list); e;
780762
e = dll_next(_pthread_list, e)) {
781763
struct PosixThread *pt = POSIXTHREAD_CONTAINER(e);
782764
if (atomic_load_explicit(&pt->pt_status, memory_order_acquire) >=
783765
kPosixThreadTerminated)
784766
break;
785-
pending = atomic_load_explicit(&pt->tib->tib_sigpending,
786-
memory_order_acquire);
787-
mask =
788-
atomic_load_explicit(&pt->tib->tib_sigmask, memory_order_acquire);
789-
if (pending & ~mask) {
790-
_pthread_ref(pt);
791-
mark = pt;
792-
break;
793-
}
767+
if (atomic_load_explicit(&pt->pt_blocker, memory_order_acquire) &&
768+
(atomic_load_explicit(&pt->tib->tib_sigpending,
769+
memory_order_acquire) &
770+
~atomic_load_explicit(&pt->pt_blkmask, memory_order_acquire)))
771+
__sig_wake(pt, 0);
794772
}
795773
_pthread_unlock();
796-
if (!mark)
797-
break;
798-
while (!atomic_compare_exchange_weak_explicit(
799-
&mark->tib->tib_sigpending, &pending, pending & ~mask,
800-
memory_order_acq_rel, memory_order_relaxed)) {
801-
}
802-
while ((pending = pending & ~mask)) {
803-
int sig = bsfl(pending) + 1;
804-
pending &= ~(1ull << (sig - 1));
805-
__sig_killer(mark, sig, SI_KERNEL);
774+
775+
// unblock stalled asynchronous signals in threads
776+
for (;;) {
777+
sigset_t pending, mask;
778+
struct PosixThread *mark = 0;
779+
_pthread_lock();
780+
for (struct Dll *e = dll_first(_pthread_list); e;
781+
e = dll_next(_pthread_list, e)) {
782+
struct PosixThread *pt = POSIXTHREAD_CONTAINER(e);
783+
if (atomic_load_explicit(&pt->pt_status, memory_order_acquire) >=
784+
kPosixThreadTerminated)
785+
break;
786+
pending = atomic_load_explicit(&pt->tib->tib_sigpending,
787+
memory_order_acquire);
788+
mask =
789+
atomic_load_explicit(&pt->tib->tib_sigmask, memory_order_acquire);
790+
if (pending & ~mask) {
791+
_pthread_ref(pt);
792+
mark = pt;
793+
break;
794+
}
795+
}
796+
_pthread_unlock();
797+
if (!mark)
798+
break;
799+
while (!atomic_compare_exchange_weak_explicit(
800+
&mark->tib->tib_sigpending, &pending, pending & ~mask,
801+
memory_order_acq_rel, memory_order_relaxed)) {
802+
}
803+
while ((pending = pending & ~mask)) {
804+
int sig = bsfl(pending) + 1;
805+
pending &= ~(1ull << (sig - 1));
806+
__sig_killer(mark, sig, SI_KERNEL);
807+
}
808+
_pthread_unref(mark);
806809
}
807-
_pthread_unref(mark);
808810
}
809811

810812
// wait until next scheduler quantum
811-
_pthread_mutex_unlock(&__sig_worker_lock);
813+
__sig_worker_state |= 1;
812814
Sleep(POLL_INTERVAL_MS);
815+
__sig_worker_state &= ~1;
813816
}
814817
__builtin_unreachable();
815818
}

‎libc/intrin/siglock.c

Lines changed: 0 additions & 22 deletions
This file was deleted.

‎libc/proc/execve-nt.greg.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
│ PERFORMANCE OF THIS SOFTWARE. │
1818
╚─────────────────────────────────────────────────────────────────────────────*/
1919
#include "libc/assert.h"
20+
#include "libc/atomic.h"
2021
#include "libc/calls/calls.h"
2122
#include "libc/calls/internal.h"
2223
#include "libc/calls/sig.internal.h"
@@ -57,11 +58,10 @@
5758
__msabi extern typeof(CloseHandle) *const __imp_CloseHandle;
5859
__msabi extern typeof(TerminateProcess) *const __imp_TerminateProcess;
5960

60-
extern pthread_mutex_t __sig_worker_lock;
61+
extern atomic_int __sig_worker_state;
6162

6263
static void sys_execve_nt_abort(sigset_t sigmask) {
63-
_pthread_unlock();
64-
_pthread_mutex_unlock(&__sig_worker_lock);
64+
__sig_worker_state &= ~2;
6565
__sig_unblock(sigmask);
6666
}
6767

@@ -70,8 +70,10 @@ textwindows int sys_execve_nt(const char *program, char *const argv[],
7070

7171
// execve() needs to be @asyncsignalsafe
7272
sigset_t sigmask = __sig_block();
73-
_pthread_mutex_lock(&__sig_worker_lock); // order matters
74-
_pthread_lock(); // order matters
73+
__sig_worker_state |= 2;
74+
for (;;)
75+
if (__sig_worker_state & 1)
76+
break;
7577

7678
// new process should be a child of our parent
7779
int64_t hParentProcess =
@@ -176,6 +178,7 @@ textwindows int sys_execve_nt(const char *program, char *const argv[],
176178
STRACE("warning: execve() lingering due to non-cosmo parent process");
177179

178180
// terminate other threads
181+
_pthread_lock();
179182
struct Dll *e;
180183
struct PosixThread *me = _pthread_self();
181184
for (e = dll_first(_pthread_list); e; e = dll_next(_pthread_list, e)) {

‎libc/proc/fork.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
│ TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR │
1717
│ PERFORMANCE OF THIS SOFTWARE. │
1818
╚─────────────────────────────────────────────────────────────────────────────*/
19+
#include "libc/atomic.h"
1920
#include "libc/calls/calls.h"
2021
#include "libc/calls/internal.h"
2122
#include "libc/calls/sig.internal.h"
@@ -54,9 +55,9 @@
5455

5556
__msabi extern typeof(GetCurrentProcessId) *const __imp_GetCurrentProcessId;
5657

58+
extern atomic_int __sig_worker_state;
5759
extern pthread_mutex_t __cxa_lock_obj;
5860
extern pthread_mutex_t __pthread_lock_obj;
59-
extern pthread_mutex_t __sig_worker_lock;
6061

6162
void __rand64_lock(void);
6263
void __rand64_unlock(void);
@@ -191,7 +192,7 @@ static void fork_child(int ppid_win32, int ppid_cosmo) {
191192
sys_read_nt_wipe_keystrokes();
192193
__proc_wipe_and_reset();
193194
__itimer_wipe_and_reset();
194-
_pthread_mutex_wipe_np(&__sig_worker_lock);
195+
atomic_init(&__sig_worker_state, 0);
195196
if (_weaken(__sig_init))
196197
_weaken(__sig_init)();
197198
if (_weaken(sys_getppid_nt_wipe))

‎test/libc/proc/execve_test.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ TEST(execve, testArgPassing) {
5858
FormatInt32(ibuf, i);
5959
GenBuf(buf, i);
6060
SPAWN(vfork);
61-
execve(prog, (char *const[]){(char *)prog, "-", ibuf, buf, 0},
62-
(char *const[]){0});
61+
execl(prog, prog, "-", ibuf, buf, NULL);
6362
kprintf("execve failed: %m\n");
6463
EXITS(0);
6564
}

0 commit comments

Comments
 (0)