806828 - WebRTC possible data race with updateVidCapTbl vs gsmsdp_get_media_capability

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P2)

Description

[Christoph Diehl [:posidron]]

Attachment: callstack

media/webrtc/signaling/src/sipcc/core/gsm/media_cap_tbl.c:72
                // we can do native now enable cap
                g_media_table.cap[CC_VIDEO_1].enabled = TRUE;

media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c:125
    *(dcb_p->media_cap_tbl) = g_media_table;

Tested with m-c changeset: 111684:e19e170d2f6d

Comment 1

[Christoph Diehl [:posidron]]

No locks involved; the TSan mode was pure-happens-before, false positive and/or the data race between 'g_media_table' is unimportant.

Comment 2

[Suhas]

How can i reproduce this issue ....

Comment 3

[Christoph Diehl [:posidron]]

Run https://developer.mozilla.org/en-US/docs/Thread_Sanitizer
against http://mozilla.github.com/webrtc-landing/

Comment 4

[Ethan Hugg [:ehugg]]

This is indeed a data race.  Couldn't repro with TSan, but I can see it with the debugger.  However those values set in media_cap_tbl.c:72 don't matter because they're reset 8 lines after the line listed in gsm_sdp.c since "if (sdpmode)" is always true.  For example, I set the cap[CC_VIDEO_1].enabled = FALSE instead of TRUE and the video still works because it's being set again.  So I don't think there's a danger here, but it does point to some cleanup needed.

Comment 5

[Ethan Hugg [:ehugg]]

Attachment: Signaling - copy selectively from g_media_table on init

Comment 6

[Ethan Hugg [:ehugg]]

Comment on attachment 709909 [details] [diff] [review]
Signaling - copy selectively from g_media_table on init

Review of attachment 709909 [details] [diff] [review]:
-----------------------------------------------------------------

This patch avoids the struct copy of g_media_table altogether when sdpmode is on which is always true in Firefox.

Comment 7

[Ethan Hugg [:ehugg]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e7c3386d9310

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e7c3386d9310