paullouisageneau / libdatachannel

C/C++ WebRTC network library featuring Data Channels, Media Transport, and WebSockets
https://libdatachannel.org/
Mozilla Public License 2.0
1.82k stars 366 forks source link

PeerConnection::close() appears to block for 1-2seconds #422

Closed dbotha closed 3 years ago

dbotha commented 3 years ago

Hi Paul-Louis,

I've recently updated to the latest libdatachannel (previously I was using a commit from back in January). I've noticed that on both Windows & Mac calling PeerConnection::close() now appears to block for almost exactly 1 second or 2 seconds. This wasn't happening for me previously. It seems that the blocking is occuring here for me:

https://github.com/paullouisageneau/libdatachannel/blob/30703741e503af64bc12c8befd7081d7a383d7ce/src/impl/datachannel.cpp#L99

Is this expected behaviour or should I investigate further?

Cheers,

Deon

paullouisageneau commented 3 years ago

Hi Deon,

It's not expected behavior as close() should not wait for network operations. SctpTransport::closeStream() translates to a SctpTransport::send(), which should not block. Is the DataChannel under heavy traffic when you close it?

The possible culprits could be usrsctp or a mitigation for usrsctp. Could you please try removing the mutex locking there (it's only to prevent an apparent data race inside usrsctp found via thread sanitizer): https://github.com/paullouisageneau/libdatachannel/blob/30703741e503af64bc12c8befd7081d7a383d7ce/src/impl/sctptransport.cpp#L421

dbotha commented 3 years ago

Spot on - if I remove that mutex then things are resolved. DataChannel has < 1KiB traffic flowing over it at the point of close().

paullouisageneau commented 3 years ago

Great, I'm removing the mitigation then! I'll investigate if the data race is really something to worry about and maybe push a proper fix to usrsctp.

dbotha commented 3 years ago

Amazing, thanks for the quick turnaround!