versatica / mediasoup

Cutting Edge WebRTC Video Conferencing
https://mediasoup.org
ISC License
6.27k stars 1.13k forks source link

worker: Do not use references for async callbacks #1274

Closed jmillan closed 11 months ago

jmillan commented 11 months ago

If the callback is not executed in the same uv_loop iteration, the access to such values upon executing the callback is undefined.

jmillan commented 11 months ago

TODO: CHANGELOG

ibc commented 11 months ago

What is this fixing?

jmillan commented 11 months ago

What is this fixing?

If the packet is not sent in the current loop iteration, ie: if uv_udp_try_send() cannot send the packet immediately and we need to use uv_udp_send() which will send the packet when possible (not in this iteration) and notify us when that is done, we will run the send callback in the future. In such future, the variables passed by reference to the callback will NOT exist, because they were in the stack, back in such previous iteration when uv_udp_send() was called.

Accessing references to stack variables that do not exist anymore has undefined behaviour, so we may be probably calling the transport congestion PacketSent() methods with garbage.

Also, this reminded me that we were having a crash with jemalloc at work, and this may well be related.

ibc commented 11 months ago

I see. Good. Approved, like my txuleton. Look:

image

jmillan commented 11 months ago

I see. Good. Approved, like my txuleton

0xdeadbeef

ibc commented 11 months ago

Plus good dessert and 3 wines and patxaran