827291 - WebRTC crash [@sipcc::PeerConnectionObserverDispatch::PeerConnectionObserverDispatch]

Status: RESOLVED INCOMPLETE

(Core :: WebRTC, defect, P1)

Description

[Christoph Diehl [:posidron]]

I am getting this crash right now only on m-i with changeset: 117855:965e97b75bfa

class PeerConnectionObserverDispatch : public nsRunnable {

public:
  PeerConnectionObserverDispatch(CSF::CC_CallInfoPtr aInfo,
                                 nsRefPtr<PeerConnectionImpl> aPC,
                                 IPeerConnectionObserver* aObserver)
      : mPC(aPC),
        mObserver(aObserver),
        mCode(static_cast<StatusCode>(aInfo->getStatusCode())),
        mSdpStr(),
        mCallState(aInfo->getCallState()),
        mStateStr(aInfo->callStateToString(mCallState)) {
    if (mCallState == REMOTESTREAMADD) {
      MediaStreamTable *streams = NULL;
      streams = aInfo->getMediaStreams();
      mRemoteStream = mPC->media()->GetRemoteStream(streams->media_stream_id);
      MOZ_ASSERT(mRemoteStream);
    }
    if ((mCallState == CREATEOFFER) || (mCallState == CREATEANSWER)) {
        mSdpStr = aInfo->getSDP();  // <<==
    }
  }

Comment 1

[Christoph Diehl [:posidron]]

Attachment: callstack

Comment 2

[Eric Rescorla (:ekr)]

What process will reproduce this?

Comment 3

[Christoph Diehl [:posidron]]

Happens during fuzzing the SDP - reloading the HTML template.

Comment 4

[Eric Rescorla (:ekr)]

Can you provide a reproduction case?

Comment 5

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

This is an odd one, since there's only one caller of this, and that function does an MOZ_ASSERT(aInfo.get()).  Is this a debug build?  If it isn't then the MOZ_ASSERT wouldn't do anything, which would make more sense.  If it is a debug build, then I can't see how this could happen except by stack-trashing.

It really is too bad ASAN doesn't give core files (which would likely be awfully large to ship around) or at least dump local vars, but that's probably not an option.

If this is not debug (and it probably isn't), then the null pointer would be coming from somewhere inside sipcc.  <- ethan

Comment 6

[Eric Rescorla (:ekr)]

I typically run ASan under the debugger and break in __asan_report_error(?)

(In reply to Randell Jesup [:jesup] from comment #5)
> This is an odd one, since there's only one caller of this, and that function
> does an MOZ_ASSERT(aInfo.get()).  Is this a debug build?  If it isn't then
> the MOZ_ASSERT wouldn't do anything, which would make more sense.  If it is
> a debug build, then I can't see how this could happen except by
> stack-trashing.
> 
> It really is too bad ASAN doesn't give core files (which would likely be
> awfully large to ship around) or at least dump local vars, but that's
> probably not an option.
> 
> If this is not debug (and it probably isn't), then the null pointer would be
> coming from somewhere inside sipcc.  <- ethan

Comment 7

[Christoph Diehl [:posidron]]

(In reply to Eric Rescorla (:ekr) from comment #4)
> Can you provide a reproduction case?
Sorry no, it happens quite often but only during fuzzing / reloading of the template.

(In reply to Randell Jesup [:jesup] from comment #5)
> Is this a debug build? 
Yup, it is a debug build with -O1

Comment 8

[Maire Reavy [:mreavy]]

Ethan -- Can you investigate what's going on here?

Comment 9

[Ethan Hugg [:ehugg]]

>Happens during fuzzing the SDP - reloading the HTML template.

I have an ASan build of this rev of M-I, but have not seen this yet.  Could you post the HTML template that you were using when this happened?

Comment 10

[Christoph Diehl [:posidron]]

Used the same template as in bug https://bugzilla.mozilla.org/show_bug.cgi?id=828147, except that line 26 is no commented out.

Comment 11

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

Can we re-verify this?  And verify the line number/source if it does happen?

Comment 12

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

blocking- for now; if it's reproduced again we can look at blocking+

Comment 13

[Christoph Diehl [:posidron]]

Haven't seen this crash lately.

m-i changeset: 122171:5e137a87e84f

Comment 14

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

Closing given current knowledge; if we see again please reopen.  Thanks!