Thank you for this project, it has been incredibly helpful!
I have found multiple concurrency issues which can be easily reproduced and usually lead to a crash. It's easy to figure out the offending codepaths by running RTSPToWeb with the Go race detector (go run -race .). I have spotted at least three issues and I intend to submit patches for two of them:
[x] StreamsList(). This method correctly locks StorageST.mutexand performs a copy of StorageST.Streams, but the copy is not enough since it's just a shallow copy, so the ChannelST map pointer stays the same and can lead to crashes when other parts of the program attempt to write to the channels map.
This can easily reproduced by invoking any API method that calls Storage.StreamChannelExist, since it mutates the map by changing the value of the ack field.
The fix is to either copy ChannelST as well or marshal it beforehand (which removes the mutable fields). Fixed in #295.
[x] StreamChannelCodecs(). This is more or less the same fallacy as the previous. The loop body locks the mutex and retrieves an instance to the streams:
obj.mutex.RLock()
tmp, ok := obj.Streams[streamID]
obj.mutex.RUnlock()
However, the channels map is accessed immediately after when the mutex is not held, which can easily result in crashes. This seems to be the root cause of #69, and I also intend to submit a fix for this soon. Fixed in #296.
Thank you for this project, it has been incredibly helpful!
I have found multiple concurrency issues which can be easily reproduced and usually lead to a crash. It's easy to figure out the offending codepaths by running RTSPToWeb with the Go race detector (
go run -race .). I have spotted at least three issues and I intend to submit patches for two of them:StreamsList(). This method correctly locksStorageST.mutexand performs a copy ofStorageST.Streams, but the copy is not enough since it's just a shallow copy, so theChannelSTmap pointer stays the same and can lead to crashes when other parts of the program attempt to write to the channels map. This can easily reproduced by invoking any API method that callsStorage.StreamChannelExist, since it mutates the map by changing the value of theackfield. The fix is to either copyChannelSTas well or marshal it beforehand (which removes the mutable fields). Fixed in #295.StreamChannelCodecs(). This is more or less the same fallacy as the previous. The loop body locks the mutex and retrieves an instance to the streams:However, the channels map is accessed immediately after when the mutex is not held, which can easily result in crashes. This seems to be the root cause of #69, and I also intend to submit a fix for this soon. Fixed in #296.
*av.Packet. One such happens whenStreamServerRunStreamupdates the packet duration: https://github.com/deepch/RTSPtoWeb/blob/82a88e1c20b64c9d72e5337de9108831780de14c/streamCore.go#L178, whilst the WebRTC servers writes it: https://github.com/deepch/RTSPtoWeb/blob/82a88e1c20b64c9d72e5337de9108831780de14c/apiHTTPWebRTC.go#L89. This is done without any lock and thus triggers the race detector.This can reliably reproduce the first two concurrency issues (just substitute
<streamid>and<channel>with appropriate values in your config):