pebbe / zmq4

A Go interface to ZeroMQ version 4
BSD 2-Clause "Simplified" License
1.17k stars 166 forks source link

Concurrent map write panic in zmq.Reactor #195

Open glenvan opened 1 year ago

glenvan commented 1 year ago

I found out the hard way that zmq.Reactor has issues with concurrent use of Reactor.AddChannel* and Reactor.AddSocket. The Reactor.sockets and Reactor.channel maps (and state in general) are not protected by mutexes, so accessing them while Reactor.Run is executing may result in concurrent writes to the maps, causing a panic. This is particularly bad for Reactor.channels which is pruned at the start of every Reactor.Run top-level loop.

For now I've worked around this by leveraging the reactor itself to monitor channels containing Reactor.AddChannel parameters as a message, and calling Reactor.AddChannel inside the handler itself, since adding to a map while ranging over the map seems to be safe with some undefined behaviour around iteration order for the new item that's not a big deal in this nested loop.

However it would seem to be worth a mention at least in the godocs that adding concurrently to a running Reactor is not safe.

It's a tricky one to fix since any app that makes heavy use of channels might spend a lot of run-time iterating over the channels map, so that's an expensive critical section to lock. There's sync.Map or the possibility of using a thread-safe "pending adds" slice or map that gets consumed in Reactor.Run around the prune operation, I suppose.