[multistream] Segmentation fault, re-negotation/DTLS alert race condition

[cameronlucas3]

There appears to be a race condition that can occur when a client sends a new SDP, and then immediately closes it's peer connection (hang-up due to DTLS alert).

This causes a segmentation fault, due to a null pointer access of handle->pc at

janus-gateway/janus.c

Line 1630 in e741411

for(mi=0; mi<g_hash_table_size(handle->pc->media); mi++) {

I am able to reliably reproduce it, by adding a sleep(1) immediately before that loop. And then forcing our own web client to re-negotiate and close it's PeerConnection ~100ms later.

Full stack trace from AddressSanitizer

[atoppi]

Thanks for the detailed report @cameronlucas3
Lines do not match, can you please share a stack trace from a recent commit ?

[atoppi]

Oops, I was inspecting the wrong file! My bad, sorry for the noise.

[lminiero]

I guess the obvious, and simplest, choice would be wrapping that simulcast traversal check in handle->mutex, since that's what janus_ice_webrtc_free (that sets handle->pc to NULL) is using internally. This should ensure no race condition occurs for that specific variable in that circumstance.

It might be better to extend the usage of that mutex to more parts of that request management, though, as just fixing it there may still result in some weird state in case conflict occurs (we start handling a new SDP, an alert wipes everything away, we then keep on handling the request anyway after the mutex is released). I'll have a look at the code to see if/where this can be done.

[lminiero]

@cameronlucas3 apologies if this took a while, but I had a stab at a few multistream issues so I addressed this too. Could you let me know if you can still replicate this, or if we can consider this fixed?

[lminiero]

I'll assume it's fixed. Please let us know if that's not the case.