Skip to content

Commit 557d286

Browse files
shai-almogclaude
andcommitted
ParparVM GC: never name a thread that may have exited
A collection that cannot stop a thread records a root miss and skips the sweep, then names each missed thread with pthread_getname_np. The thread it could not stop is usually one that is exiting: markDeadThread deregisters it and the pthread ends, so the recorded handle can name a thread that no longer exists, and glibc reads its freed descriptor. The bulk conformance run, whose pool checks start and retire many threads, crashed with SIGSEGV in this diagnostic right after the skip line (vm-tests, StringBuilderGrowthApp, exit 139). The name is now read only for a handle still registered in allThreads, under the critical section markDeadThread takes, and blocked threads are released before that lock is taken. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 49bddf5 commit 557d286

1 file changed

Lines changed: 24 additions & 3 deletions

File tree

‎vm/ByteCodeTranslator/src/cn1_globals.m‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7333,20 +7333,41 @@ void codenameOneGCSweep() {
73337333
// 4 and 6 a virtual thread's resumer outside its host stack, 5 the thread could
73347334
// not be stopped (stopErr says why), 7 stack pointer outside the stack, 8 no
73357335
// registers, 9-10 the collector's own stack.
7336+
//
7337+
// Released FIRST: the naming below takes the critical section, and a thread
7338+
// parked on the collector while holding it would otherwise never let go.
7339+
cn1GcReleaseBlockedThreads();
73367340
for(int __m = 0; __m < cn1GcRootMissCount && __m < CN1_GC_ROOT_MISS_MAX; __m++) {
73377341
char __name[64];
73387342
__name[0] = 0;
73397343
#if defined(__APPLE__) || defined(__linux__)
7340-
// Safe here: every thread was released before the sweep decided to skip.
7344+
// Named only while the thread is provably alive. A thread the stop could
7345+
// not reach is very often one that is EXITING: markDeadThread takes it out
7346+
// of allThreads under the critical section and the pthread then ends, so
7347+
// by now the recorded handle may name a thread that no longer exists, and
7348+
// glibc's pthread_getname_np reads that thread's freed descriptor -- a
7349+
// SIGSEGV in the diagnostic itself, after the skip line and before any
7350+
// site line. Holding the critical section keeps a still-registered thread
7351+
// from finishing markDeadThread, so a handle found in allThreads stays
7352+
// valid for the call. Every thread was released before the sweep decided
7353+
// to skip, so none is frozen holding the lock pthread_getname_np takes.
73417354
if(cn1GcRootMiss[__m].hasPthread) {
7342-
pthread_getname_np(cn1GcRootMiss[__m].pthread, __name, sizeof(__name));
7355+
lockCriticalSection();
7356+
for(int __k = 0; __k < NUMBER_OF_SUPPORTED_THREADS; __k++) {
7357+
struct ThreadLocalData* __t = allThreads[__k];
7358+
if(__t != 0 && __t->gcPthreadValid
7359+
&& pthread_equal(__t->gcPthread, cn1GcRootMiss[__m].pthread)) {
7360+
pthread_getname_np(cn1GcRootMiss[__m].pthread, __name, sizeof(__name));
7361+
break;
7362+
}
7363+
}
7364+
unlockCriticalSection();
73437365
}
73447366
#endif
73457367
fprintf(stderr, "CN1 GC: site=%d thread=%ld lightweight=%d stopErr=%d name=%s\n",
73467368
cn1GcRootMiss[__m].site, cn1GcRootMiss[__m].threadId,
73477369
cn1GcRootMiss[__m].lightweight, cn1GcRootMiss[__m].stopError, __name);
73487370
}
7349-
cn1GcReleaseBlockedThreads();
73507371
return;
73517372
}
73527373
if(cn1GcPageIndexStale) {

0 commit comments

Comments
 (0)