GetSnapshot copied the snapshot struct but returned the state's own four maps. The dashboard and /api/v1/status range over them after the read lock is released while the watcher writes them, which is a data race and can abort the process with concurrent map iteration and map write. GetSnapshot now copies Domains, Hostnames, Ports and Certificates under the read lock. Of the two fixes the issue allows, this is the smaller: the handlers are unchanged.
The entries in the maps are still shared with the state. The issue's claim holds: the watcher changes an entry only by saving a new one with a Set...State call, and no caller of GetSnapshot or of the Get...State getters changes an entry it got back (Load sorts records in place, but in a snapshot that is not yet the state's). The comment on GetSnapshot states the rule for whoever adds a writer.
TestGetSnapshot_WhileWritten marshals snapshots to JSON, which ranges over every map, while a goroutine writes all four maps; -race reports a race on each map when the copies are removed. With the copies removed, the race detector may also name another parallel state test as failed, since it blames whichever test is running. TestGetSnapshot_ReturnsCopy now adds an entry to each map of a snapshot and checks that the state does not have it.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/dnswatcher/issues/261.
`GetSnapshot` copied the snapshot struct but returned the state's own four maps. The dashboard and `/api/v1/status` range over them after the read lock is released while the watcher writes them, which is a data race and can abort the process with `concurrent map iteration and map write`. `GetSnapshot` now copies `Domains`, `Hostnames`, `Ports` and `Certificates` under the read lock. Of the two fixes the issue allows, this is the smaller: the handlers are unchanged.
The entries in the maps are still shared with the state. The issue's claim holds: the watcher changes an entry only by saving a new one with a `Set...State` call, and no caller of `GetSnapshot` or of the `Get...State` getters changes an entry it got back (`Load` sorts records in place, but in a snapshot that is not yet the state's). The comment on `GetSnapshot` states the rule for whoever adds a writer.
`TestGetSnapshot_WhileWritten` marshals snapshots to JSON, which ranges over every map, while a goroutine writes all four maps; `-race` reports a race on each map when the copies are removed. With the copies removed, the race detector may also name another parallel `state` test as failed, since it blames whichever test is running. `TestGetSnapshot_ReturnsCopy` now adds an entry to each map of a snapshot and checks that the state does not have it.
Model: opus-5-5
The dashboard and /api/v1/status range over the maps in the snapshot
GetSnapshot returns after the read lock is released, while the watcher
writes the same maps under the lock. Go aborts the process when a write
lands during such a range. GetSnapshot now returns copies of the four
maps.
The entries in the maps are still shared with the state. That is safe
because the watcher replaces an entry with a Set call and never changes
one in place; the comment on GetSnapshot says so.
TestGetSnapshot_WhileWritten ranges over snapshots while another
goroutine writes the state and fails under -race without the copies.
TestGetSnapshot_ReturnsCopy now checks that each map is not the state's
own.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #261.
GetSnapshotcopied the snapshot struct but returned the state's own four maps. The dashboard and/api/v1/statusrange over them after the read lock is released while the watcher writes them, which is a data race and can abort the process withconcurrent map iteration and map write.GetSnapshotnow copiesDomains,Hostnames,PortsandCertificatesunder the read lock. Of the two fixes the issue allows, this is the smaller: the handlers are unchanged.The entries in the maps are still shared with the state. The issue's claim holds: the watcher changes an entry only by saving a new one with a
Set...Statecall, and no caller ofGetSnapshotor of theGet...Stategetters changes an entry it got back (Loadsorts records in place, but in a snapshot that is not yet the state's). The comment onGetSnapshotstates the rule for whoever adds a writer.TestGetSnapshot_WhileWrittenmarshals snapshots to JSON, which ranges over every map, while a goroutine writes all four maps;-racereports a race on each map when the copies are removed. With the copies removed, the race detector may also name another parallelstatetest as failed, since it blames whichever test is running.TestGetSnapshot_ReturnsCopynow adds an entry to each map of a snapshot and checks that the state does not have it.Model: opus-5-5
Review: PASS
Model: opus-5-5