search

paullouisageneau / libdatachannel

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

Why does track default to !isClosed? #1257

Closed singpolyma closed 2 months ago

singpolyma commented 2 months ago

https://github.com/paullouisageneau/libdatachannel/blob/master/src/impl/track.hpp#L75

This leads to a race condition being possible when checking if !track.isClosed() while it is initially false, but then gets set to true during connection, before being back to false again after connected.

paullouisageneau commented 2 months ago

I'm not sure I understand what you mean. isClosed() is not set to true during connection, it stays false until the track is closed.

singpolyma commented 2 months ago

Hmm. I'm definitely experiencing something where if I call send too soon, I get an exception about how the track is closed, which from the source seems to happen because mTrackIsClosed is set true.

paullouisageneau commented 2 months ago

The track is not open immediately on creation, like for data channels you have to wait for onOpen() to be triggered or isOpen() to return true. This is unrelated to isClosed(), which only reflects if close() has been called.

singpolyma commented 2 months ago

Hmm, ok, I can check isOpen instead of !isClosed but it still seems odd that isClosed will be false, then true, then false again, instead of starting out as true then moving to false.

paullouisageneau commented 2 months ago

isClosed() is false on creation, and change to true when you close the track.

singpolyma commented 2 months ago

Ok, but if that's true then how can I check if(track->isClosed()) ... then call outgoing and get https://github.com/paullouisageneau/libdatachannel/blob/master/src/impl/track.cpp#L165 if mIsClosed will never be true until after the track has been closed?

paullouisageneau commented 2 months ago

I think you actually get this one, because the track is not open yet: https://github.com/paullouisageneau/libdatachannel/blob/4bf08fd9e5d9236d948537c791c4b67116302163/src/impl/track.cpp#L205

It could be changed to "Track is not open" if it's misleading.

singpolyma commented 2 months ago

Oh! That makes way more sense. I didn't even notice that second place with an identical-looking exception. My bad.