Fix race condition in Packet.MarshalTo() #234
Conversation
This was already fixed by pion#168 but got lost in pion#227. in SFUs, in order to distribute a packet to all clients, MarshalTo() is called in parallel by multiple routines, causing a race condition because the padding flag is dynamically set inside MarshalTo(). This is particular annoying when running automated tests. This PR fixes the issue by removing this write operation as discussed in pion#168.
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #234 +/- ##
==========================================
- Coverage 87.39% 87.38% -0.01%
==========================================
Files 20 20
Lines 1888 1887 -1
==========================================
- Hits 1650 1649 -1
Misses 207 207
Partials 31 31
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
Hi @aler9 , Upgrade from 1.8.0 to 1.8.1, my h264 stream is broken. I have set |
|
@0x5e on the contrary, it is required to set Header.Padding to true before calling MarshalTo when there's padding. in my opinion, MarshalTo() must be a read-only function exactly like it is in all other objects and in the standard library. Otherwise it's impossible to safely pass packets between routines. |
|
Eventually we could add a consistency check in MarshalTo that returns an error when Header.Padding is true and PaddingSize is zero. |
|
Sorry that was my mistake, actually I need is Padding=false, so Header.Padding=true and PaddingSize=0 make me broken :) |
Description
This was already fixed by #168 but got lost in #227. in SFUs, in order to distribute a packet to all clients, MarshalTo() is called in parallel by multiple routines, causing a race condition because the padding flag is dynamically set inside MarshalTo(). This is particular annoying when running automated tests. This PR fixes the issue by removing this write operation as discussed in #168.