crash in janus_videoroom_hangup_media on v0.10.3

[lm123]

Janus crash in function janus_videoroom_hangup_media, backtrace is liking as following:

#0  0x00007f0e8758d438 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007f0e8758f03a in __GI_abort () at abort.c:89
#2  0x00007f0e88e1e28d in ?? ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007f0e7c44b487 in janus_videoroom_hangup_media (
    handle=<optimized out>) at plugins/janus_videoroom.c:5441
#4  0x00000000004494b4 in janus_ice_outgoing_traffic_handle (
    handle=0x7f0e68014520, pkt=<optimized out>) at ice.c:4117
#5  0x000000000044ba24 in janus_ice_outgoing_traffic_dispatch (
    source=0x7f0e6800faa0, callback=<optimized out>,
    user_data=<optimized out>) at ice.c:376
#6  0x00007f0e88dda197 in g_main_context_dispatch ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#7  0x00007f0e88dda3f0 in ?? ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#8  0x00007f0e88dda712 in g_main_loop_run ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#9  0x000000000043de19 in janus_ice_handle_thread (
    data=0x7f0e68014520) at ice.c:1165
#10 0x00007f0e88e00c55 in ?? ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#11 0x00007f0e879296ba in start_thread (arg=0x7f0e0cff9700)

EDIT by atoppi: @lm123 I have edited your message to format the stack trace in a code block.
Please read carefully the guidelines next time.

[MvEerd]

To help meetecho fix this you should test on master and provide a libasan log
For instructions on how to get Address sanitizer output see; https://janus.conf.meetecho.com/docs/debug.html

3. Make sure the issue wasn't already solved in master: we will ignore the issue otherwise.

5. If you're encountering crashes, make sure you provide a useful trace of what happened: depending on the issue, we may need output from AddressSanitizer or GDB stacktraces (more details available here).

[lm123]

After doing changing the following lines (5439 and 5442) in janus_videoroom.c:
//janus_refcount_increase(&session->ref);
janus_mutex_unlock(&sessions_mutex);
janus_videoroom_hangup_media_internal(session);
//janus_refcount_decrease(&session->ref);

The coredump can be resolved. Can you evaluate such change is correct?

[groupboard]

You should test on latest master. I'm not sure if your "fix" makes sense...just because it fixes the crash doesn't mean it is the proper fix. I would guess it might be fixed by the latest version of the janus_videoroom.c code, which has a fix for a similar crash. Try reverting your changes and test on latest master to see if it is resolved.

[lm123]

Yes. I test against the latest master v0.10.4. Please investigating the following codes:

void janus_videoroom_hangup_media(janus_plugin_session *handle) {
...
janus_mutex_unlock(&sessions_mutex);
janus_videoroom_hangup_media_internal(session);
}

static void janus_videoroom_hangup_media_internal(gpointer session_data) {
janus_videoroom_session *session = (janus_videoroom_session *)session_data;
g_atomic_int_set(&session->started, 0);
if(!g_atomic_int_compare_and_exchange(&session->hangingup, 0, 1)) {
janus_mutex_unlock(&sessions_mutex);
return;
}
...
}

The session_mutex might be unlocked twice.

[groupboard]

Yes, it looks like line line 5485 might need to be removed in janus_videoroom_hangup_media_internal:

	janus_mutex_unlock(&sessions_mutex);

It looks like that piece of code was missed in this refactor: 9dff2d8#diff-f302a3ccc39e037fccf44e207332553c

[atoppi]

@lm123 @groupboard indeed, it seems we overlooked that mutex unlock in the mentioned refactoring.
I'm pushing a fix right now.
Thanks to all for spotting this!