les/vflux/client, p2p/nodestate: fix data races #24058
Conversation
les/vflux/client/serverpool_test.go
Outdated
| @@ -54,6 +54,8 @@ type ServerPoolTest struct { | |||
| db ethdb.KeyValueStore | |||
| clock *mclock.Simulated | |||
| quit chan chan struct{} | |||
| stopping bool | |||
| queryWg *sync.WaitGroup | |||
There was a problem hiding this comment.
Why not just use a sync.WaitGroup instead of a pointer?
There was a problem hiding this comment.
Changed it back and added comments to explain why it needs to be recreated each time and how the waitgroup is used safely :)
|
I think the changes in les/vflux/client tests violate WaitGroup semantics, and will lead to occasional crashes in tests. We could ignore this and merge anyway, but then the tests will subtly wrong in a different and less-detectable way. Also, the fix applied here is super ugly. To make a meaningful improvement, it should at least be documented what the scope of the lock is. Does it apply to everything in |
zsfelfoldi
force-pushed
the
fix-test-datarace
branch
from
cf698a2 to
4289f1b
Compare
zsfelfoldi
force-pushed
the
fix-test-datarace
branch
from
4289f1b to
d45309b
Compare
|
Now I think it's mergeable |
This PR fixes some data races in the server pool tests and also one in the NodeStateMachine.
Fixes #23848