From: Michael Anthony Knyszek Date: Thu, 9 May 2019 19:55:39 +0000 (+0000) Subject: runtime: fix js/wasm lock implementation X-Git-Tag: go1.13beta1~354 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=cd03664f82a53dbe20d0b828189158ba3863039c;p=gostls13.git runtime: fix js/wasm lock implementation Trybots started failing on js/wasm after golang.org/cl/175797 landed, but it seemed completely unrelated. It would fail very consistently on the heapsampling.go test. Digging deeper it was very difficult to ascertain what was going wrong, but clearly m.locks for some m was non-zero when calling into the scheduler. The failure comes from the fact that lock calls into gosched, but it's unclear how exactly we got there in the first place; there should be no preemption in this single-threaded context. Furthermore, lock shouldn't be calling gosched_m at all because in a single-threaded context, the thread shouldn't be preempted until it actually unlocks. But, digging further it turns out the implementation in lock_js.go never incremented or decremented m.locks. This is definitely wrong because many parts of the runtime depend on that value being set correctly. So, this change removes the loop which calls into gosched_m (which should be unnecessary) and increments and decrements m.locks appropriately. This appears to fix the aforementioned failure. Change-Id: Id214c0762c3fb2b405ff55543d7e2a78c17443c4 Reviewed-on: https://go-review.googlesource.com/c/go/+/176297 Run-TryBot: Michael Knyszek Reviewed-by: Russ Cox Reviewed-by: Cherry Zhang TryBot-Result: Gobot Gobot --- diff --git a/src/runtime/lock_js.go b/src/runtime/lock_js.go index f58c915b63..c038499f2a 100644 --- a/src/runtime/lock_js.go +++ b/src/runtime/lock_js.go @@ -11,8 +11,6 @@ import ( ) // js/wasm has no support for threads yet. There is no preemption. -// Waiting for a mutex is implemented by allowing other goroutines -// to run until the mutex gets unlocked. const ( mutex_unlocked = 0 @@ -28,9 +26,16 @@ const ( ) func lock(l *mutex) { - for l.key == mutex_locked { - mcall(gosched_m) + if l.key == mutex_locked { + // js/wasm is single-threaded so we should never + // observe this. + throw("self deadlock") } + gp := getg() + if gp.m.locks < 0 { + throw("lock count") + } + gp.m.locks++ l.key = mutex_locked } @@ -38,6 +43,11 @@ func unlock(l *mutex) { if l.key == mutex_unlocked { throw("unlock of unlocked lock") } + gp := getg() + gp.m.locks-- + if gp.m.locks < 0 { + throw("lock count") + } l.key = mutex_unlocked }