sync: hand the RWMutex over instead of a re-test of the reader count - #5630
Open
yohimik wants to merge 1 commit into
Open
sync: hand the RWMutex over instead of a re-test of the reader count#5630yohimik wants to merge 1 commit into
yohimik wants to merge 1 commit into
Conversation
This was referenced Aug 30, 2026
shibukawa
added a commit
to shibukawa/tinygodriver
that referenced
this pull request
Sep 2, 2026
…on the TinyGo path TinyGo's sync.RWMutex (through at least 0.42) deadlocks whenever a reader arrives while a writer is waiting for the existing readers to drain. Lock() subtracts rwMutexMaxReaders and waits for the last RUnlock to bring the count back to exactly that; RLock() adds 1 and then waits for the count to turn positive, keeping its +1 while it waits. A reader that arrives mid-wait is therefore counted as a holder that never leaves: the writer is never woken and the reader waits for the writer's Unlock. Standard Go snapshots the readers a writer must wait for in a separate counter. Upstream fix: tinygo-org/tinygo#5630, open at time of writing. This is what `tinygo test ./websocket` had been hanging on, roughly one run in three: a stack sample showed 15 threads in RWMutex.RLock and one in RWMutex.Lock, all reached through netdev.Device.mu, which every Send and Recv read-locks while Socket, Accept and Close write-lock it. A four-step interleaving reproduces it deterministically; 16 readers and a writer around a map hung 13 of 13 runs; the same netdev echo probe went from 11 of 40 hung to 0 of 40 with a plain mutex. internal/syncx.RWMutex is sync.RWMutex on standard Go and a plain sync.Mutex behind the same method set on tinygo and force_tinygo_logic. Every RWMutex a tinygo build could reach now uses it: netdev's socket table and the darwin and windows TLS session tables, httpmux, the dynamodb and datastore field caches, the s3 region, fasthttp's HostClient map (as vendor.py patches, PATCHES.md section 8) and the five registries in the mysql fork (PETITWEB comments, README). Each critical section is a map lookup, so serializing readers costs nothing measurable. Three tests pin it down. TestRWMutexHandsOverToWaitingWriter runs the four-step interleaving against the shim on every build. TestUpstreamRWMutexStillDeadlocks (tinygo only) runs it against sync.RWMutex and expects the deadlock, so it fails the day a TinyGo release ships the fix, which is the signal to retire the shim. TestNoStdRWMutexOnTinyGoPath asks `go list -tags tinygo` for the files of every package on darwin, linux and windows and parses them for the selector, so neither first-party code nor a re-vendored fork can drift back. Verified: go test and go test -tags force_tinygo_logic across the module; tinygo test for syncx, netdev, httpmux, httprevproxy, fasthttp and fasthttpwebsocket; `tinygo test ./websocket` 20 times under a watchdog with 0 hangs, against 5 of 16 before; the examples that touch the swapped packages build, as does a linux/arm64 cross-link; mingw vets the windows session table. TestLargeMessage's occasional EPIPE, seen on the untouched tree as well, is a separate flake and is not changed by this. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
shibukawa
added a commit
to shibukawa/tinygodriver
that referenced
this pull request
Sep 2, 2026
TinyGo's sync.RWMutex deadlocks whenever a reader arrives while a writer is waiting; netdev.Device.mu is read on every Send and Recv and written on every Socket, Accept and Close, which is how tinygo test ./websocket hung one run in three. internal/syncx.RWMutex is the standard type on standard Go and a plain mutex on TinyGo, every reachable RWMutex now uses it, a policy test keeps it that way, and a tinygo-only test fails the day a release ships the upstream fix (tinygo-org/tinygo#5630). 0 hangs in 20 runs, against 5 in 16. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
RWMutex counts the readers that hold the lock and the readers that queue behind a waiting writer in one number, and both sides wait on a predicate over that number. Two interleavings stop the program permanently. A writer waits until the count shows no readers at all. A reader that arrives during that wait joins the same count, so the last holder of the lock no longer sees the condition that wakes the writer. A reader that Unlock releases reads the count again instead of an acquire. A writer that arrives in between changes the base of the count, so the reader goes back to sleep after its wakeup is spent, while that writer waits for it. Use the split that the standard library uses. A writer records how many readers it finds and waits only for those, so later readers cannot starve it. Counting semaphores hand the lock over, so a released waiter holds the lock and does not test a value again that a third party can change back. task.Semaphore cannot do this, because one Post does nothing when there are several waiters, so the file gets a small futex semaphore that can. Ordinary code reaches this. syscall.ForkLock is an RWMutex, os.Pipe read-locks it and os.StartProcess write-locks it, so a program that starts processes and makes pipes at the same time can stop. The two new tests fail on the current code and pass with this change.
yohimik
force-pushed
the
upstream-pr/sync-rwmutex
branch
from
September 2, 2026 08:49
7001593 to
6965ab2
Compare
Author
|
Rebased on dev after the 0.42.0 release. The problem is present in v0.42.0 as |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sync: hand the RWMutex over instead of a re-test of the reader count
What this does
sync.RWMutexcounts the readers that hold the lock and the readers that queuebehind a waiting writer in one number, and both sides wait on a predicate over
that number. Two interleavings stop the program permanently.
during that wait joins the same count, so the last holder of the lock no
longer sees the condition that wakes the writer.
Unlockreleases reads the count again instead of an acquire.A writer that arrives in between changes the base of the count, so the reader
goes back to sleep after its wakeup is spent, while that writer waits for it.
Both cases end with every party parked and nobody left to wake anyone.
The change adopts the split that the Go standard library uses. A writer records
how many readers it finds and waits only for those, so later readers cannot
starve it. Counting semaphores hand the lock over, so a released waiter holds
the lock and does not test a value again that a third party can change back.
internal/task.Semaphorecannot do this, because onePostdoes nothing whenthere are several waiters, so
src/sync/mutex.gogets a small futex semaphorethat can.
Why it matters
Ordinary code reaches this.
syscall.ForkLockis anRWMutex,os.Piperead-locks it and
os.StartProcesswrite-locks it, so a program that startsprocesses and makes pipes at the same time can stop.
Evidence
Two regression tests are added to
src/sync/mutex_test.go.syncis already inTEST_PACKAGES_FAST, so they run on every platform that the standard librarytest matrix covers, with no makefile change.
Measured on macOS 26.6 arm64 with
tinygo test sync.TestRWMutexWriterNotStarvedByLateReadersTestRWMutexHandoffToQueuedReadersThe same shape was seen with real work. A probe that starts sixteen processes
at the same time and gives each one a pipe stopped within twelve seconds on
every run before this change, and completed on twenty runs out of twenty after
it, on linux and on macOS.
A downstream product also ships binaries built with a fork that carries this
change, in the published release dispat v1.4.0.
https://github.com/yohimik/dispat/releases/tag/services%2Fdispat%2Fv1.4.0
Scope
internal/task.Futex,so no path is special-cased.
Notes for review
rwSemis small on purpose. If the maintainers would rather have a countingsemaphore in
internal/tasknext toSemaphore, it can move there.Unlocknow reports "sync: Unlock of unlocked RWMutex" throughruntimeFatal, which the old code did not do. Say if that check is unwanted.Related pull requests
This change is part of one body of work. Together the changes make programs that use the network and child processes work on hosted linux and macOS. A full CLI was tested end to end with all of them and ships binaries built this way, see dispat v1.4.0 in the evidence section.
In this repository
In tinygo-org/net
A merge order that works. The three bug fixes are independent. #5633 goes before #5635. HTTPS on linux needs only #5633 and #5635. Full darwin support also needs #5636, the net changes and a new src/net submodule pin.