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 <mknyszek@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
)
// 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
)
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
}
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
}