Commit 852ba3a8 authored by cznic's avatar cznic
Browse files

libc_musl: fix lost-wakeup deadlock in ___lock/___unlock 

___lock/___unlock emulated a C mutex using an atomic lock-word fast path
plus a throwaway hand-off object stored in a map. When an unlocker took
locksMu before a contending locker had registered its hand-off, ___unlock
synthesized one and immediately discarded it; the waiter then blocked on a
fresh, never-unlocked mutex forever, and the lock word was left non-zero, so
every subsequent ___lock on that address also blocked. The process wedged
permanently at zero CPU.

The window is reachable through the exported API alone: localtime, localtime_r
and mktime all take the single process-global timezone lock (_lock4) via
__secs_to_zone on every call, so goroutines calling them concurrently can hit
it. Reported against modernc.org/quickjs evaluating JS Date local-time getters.

Replace the scheme with a per-address sync.Mutex, created lazily and
reference-counted for cleanup. The opaque C lock word is no longer touched
(nothing reads it outside these two functions); routing all mutual exclusion
through the per-address mutex makes the hand-off race-free, so a release
delivered before the waiter blocks is no longer lost.

Add TestIssue51: a mutual-exclusion stress on ___lock/___unlock plus a
concurrent localtime_r stress; both clean under -race.

Fixes: #51

Co-Authored-By: default avatarClaude Opus 4.8 (1M context) <noreply@anthropic.com>
parent 7cdf9f2b

issue51_musl_test.go

0 → 100644
+115 −0
Original line number Diff line number Diff line 
// Copyright 2026 The Libc Authors. All rights reserved.

// Use of this source code is governed by a BSD-style

// license that can be found in the LICENSE file.



//go:build linux && (amd64 || arm64 || loong64 || ppc64le || s390x || riscv64 || 386 || arm)



package libc



import (

	"sync"

	"sync/atomic"

	"testing"

	"time"

	"unsafe"

)



// TestIssue51 guards against the lost-wakeup deadlock in ___lock/___unlock

// (https://gitlab.com/cznic/libc/-/work_items/51). The previous implementation

// kept an atomic fast path on the C lock word plus a throwaway hand-off object;

// when an unlocker reached locksMu before a contending locker had registered in

// the locks map, the hand-off was created and immediately discarded, leaving the

// waiter blocked on a fresh, never-unlocked mutex forever (process wedged at zero

// CPU). The trigger is reachable through the exported API alone, e.g. concurrent

// localtime_r contending on the single process-global timezone lock.

//

// These are stress checks: they drive the real ___lock/___unlock and the exported

// localtime_r path under heavy contention, asserting mutual exclusion holds and

// nothing deadlocks. Run under -race to additionally verify the hand-off is a

// properly synchronized, data-race-free critical section.

func TestIssue51(t *testing.T) {

	t.Run("lock_mutual_exclusion", testIssue51LockMutualExclusion)

	t.Run("localtime_r_concurrent", testIssue51LocaltimeConcurrent)

}



// testIssue51LockMutualExclusion hammers a single lock address from many

// goroutines. Each critical section increments a non-atomic counter; if mutual

// exclusion holds the final value is exact, and a lost update (or a -race report)

// signals a broken lock. A deadlock is caught by the timeout.

func testIssue51LockMutualExclusion(t *testing.T) {

	const goroutines = 20

	const iters = 10000



	var word int32 // the C lock word; its address is the lock identity

	p := uintptr(unsafe.Pointer(&word))

	var guarded int64 // deliberately non-atomic; protected by ___lock(p)



	var wg sync.WaitGroup

	wg.Add(goroutines)

	for g := 0; g < goroutines; g++ {

		go func() {

			defer wg.Done()

			tls := NewTLS()

			defer tls.Close()

			for i := 0; i < iters; i++ {

				___lock(tls, p)

				guarded++

				___unlock(tls, p)

			}

		}()

	}



	if !issue51WaitTimeout(&wg, time.Minute) {

		t.Fatal("deadlock: ___lock/___unlock contention did not complete (issue #51)")

	}

	if got, want := guarded, int64(goroutines)*iters; got != want {

		t.Fatalf("mutual exclusion violated: guarded=%d want=%d", got, want)

	}

}



// testIssue51LocaltimeConcurrent exercises the exact exported path from the bug

// report: many goroutines, each with its own TLS, contend on the process-global

// timezone lock via localtime_r. It must not deadlock.

func testIssue51LocaltimeConcurrent(t *testing.T) {

	const goroutines = 16

	const iters = 50000



	var wg sync.WaitGroup

	wg.Add(goroutines)

	var calls int64

	for g := 0; g < goroutines; g++ {

		go func() {

			defer wg.Done()

			tls := NewTLS()

			defer tls.Close()

			tp := Xmalloc(tls, 8)    // time_t

			tm := Xmalloc(tls, 128)  // struct tm (over-allocated)

			if tp == 0 || tm == 0 {

				return

			}

			defer Xfree(tls, tp)

			defer Xfree(tls, tm)

			*(*int64)(unsafe.Pointer(tp)) = 1600000000

			for i := 0; i < iters; i++ {

				Xlocaltime_r(tls, tp, tm)

				atomic.AddInt64(&calls, 1)

			}

		}()

	}



	if !issue51WaitTimeout(&wg, time.Minute) {

		t.Fatal("deadlock: concurrent Xlocaltime_r did not complete (issue #51)")

	}

	t.Logf("completed %d concurrent Xlocaltime_r calls without deadlock", atomic.LoadInt64(&calls))

}



func issue51WaitTimeout(wg *sync.WaitGroup, d time.Duration) bool {

	done := make(chan struct{})

	go func() { wg.Wait(); close(done) }()

	select {

	case <-done:

		return true

	case <-time.After(d):

		return false

	}

}

libc_musl.go

+19 −35
Original line number Diff line number Diff line
@@ -507,7 +507,7 @@ func Xabort(tls *TLS) { 


type lock struct {

	sync.Mutex

	waiters int

	refs int // holders + waiters currently inside ___lock/___unlock for this address

}



var (
@@ -515,57 +515,41 @@ var ( 
	locks   = map[uintptr]*lock{}

)



/*



	T1		T2



	lock(&foo)			// foo: 0 -> 1



			lock(&foo)	// foo: 1 -> 2



	unlock(&foo)			// foo: 2 -> 1, non zero means waiter(s) active



			unlock(&foo)	// foo: 1 -> 0



*/

// ___lock/___unlock emulate musl's __lock/__unlock, a mutual-exclusion lock over

// an opaque C lock word *p, by keying a Go sync.Mutex on the lock word's address.

// The per-address lock is created lazily and reference-counted (refs) so its map

// entry can be reclaimed once no goroutine holds or waits on it. The C lock word

// *p is intentionally left untouched: it is opaque to every caller (nothing reads

// it outside these two functions), and routing all mutual exclusion through the

// per-address sync.Mutex is what makes the hand-off race-free.

//

// A previous implementation kept an atomic fast path on *p plus a throwaway

// hand-off object. It lost wakeups when an unlocker reached locksMu before a

// contending locker had registered in locks: the unlock created and immediately

// discarded a hand-off, then the locker blocked on a fresh, never-unlocked mutex

// forever, wedging the process at zero CPU (cznic/libc#51).



func ___lock(tls *TLS, p uintptr) {

	if atomic.AddInt32((*int32)(unsafe.Pointer(p)), 1) == 1 {

		return

	}



	// foo was already acquired by some other C thread.

	locksMu.Lock()

	l := locks[p]

	if l == nil {

		l = &lock{}

		locks[p] = l

		l.Lock()

	}

	l.waiters++

	l.refs++

	locksMu.Unlock()

	l.Lock() // Wait for T1 to release foo. (X below)

	l.Lock() // Block until the current holder of p releases it.

}



func ___unlock(tls *TLS, p uintptr) {

	if atomic.AddInt32((*int32)(unsafe.Pointer(p)), -1) == 0 {

		return

	}



	// Some other C thread is waiting for foo.

	locksMu.Lock()

	l := locks[p]

	if l == nil {

		// We are T1 and we got the locksMu locked before T2.

		l = &lock{waiters: 1}

		l.Lock()

	}

	l.Unlock() // Release foo, T2 may now lock it. (X above)

	l.waiters--

	if l.waiters == 0 { // we are T2

	l.refs--

	if l.refs == 0 {

		delete(locks, p)

	}

	locksMu.Unlock()

	l.Unlock() // Hand p to the next waiter, if any.

}



type lockedFile struct {