807297 - WebRTC data race with mozilla::MediaPipeline::PipelineTransport::SendRtcpPacket vs. mozilla::MediaPipeline::MediaPipeline

Status: NEW

(Core :: WebRTC: Networking, defect, P5)

Description

[Christoph Diehl [:posidron]]

Attachment: callstack

media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp:488
  if (!pipeline_)
    return NS_OK;  // Detached

media/webrtc/signaling//./src/mediapipeline/MediaPipeline.h:67
  MediaPipeline(Direction direction,
                nsCOMPtr<nsIEventTarget> main_thread,
                nsCOMPtr<nsIEventTarget> sts_thread,
                nsDOMMediaStream* stream,
                RefPtr<MediaSessionConduit> conduit,
                RefPtr<TransportFlow> rtp_transport,
                RefPtr<TransportFlow> rtcp_transport)
      : direction_(direction),
        stream_(stream),
        conduit_(conduit),
        rtp_transport_(rtp_transport),
        rtp_state_(MP_CONNECTING),
        rtcp_transport_(rtcp_transport),
        rtcp_state_(MP_CONNECTING),
        main_thread_(main_thread),
        sts_thread_(sts_thread),
        transport_(new PipelineTransport(this)),
        rtp_send_srtp_(),
        rtcp_send_srtp_(),
        rtp_recv_srtp_(),
        rtcp_recv_srtp_(),
        rtp_packets_sent_(0),
        rtcp_packets_sent_(0),
        rtp_packets_received_(0),
        rtcp_packets_received_(0),
        muxed_((rtcp_transport_ == NULL) || (rtp_transport_ == rtcp_transport_)) {
    Init();
  }

Tested with m-c changeset: 111684:e19e170d2f6d

Comment 1

[Eric Rescorla (:ekr)]

Maybe I'm not thinking about this the right way, but I'm having trouble seeing how this
can be a race condition.

The relevant code in MediaPipeline.h (the first access) initializes rtp_transport_ which
then initialized rtp_transport_->pipeline_ as part of the class initialization. Both the upward
and downward connections happen in Init(), which is in the body of the ctor
thus runs after the initializers. Packets shouldn't be sent until after that point.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

I believe this is a false positive.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.