944854 - "ASSERTION: You can't dereference a NULL nsRefPtr with operator*()." with RTC, GC/CC

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Jesse Ruderman]

When I follow the steps in bug 928221, I get this assertion, but it is NOT followed by a crash. Maybe it's a bogus assertion!?

I can only reproduce with a Tinderbox debug build, not with a local debug build. I'm not sure why. (Both are from mozilla-central.)

###!!! ASSERTION: You can't dereference a NULL nsRefPtr with operator*().: 'mRawPtr != 0', file ../../../../../media/webrtc/signaling/../../../xpcom/base/nsAutoPtr.h, line 1072

sipcc::PeerConnectionImpl::IceGatheringStateChange_m(mozilla::dom::PCImplIceGatheringState) [media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:1777]

Comment 1

[Jesse Ruderman]

See bug 933447 comment 39 to 42.

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Avoid triggering harmless assertion on PeerConnectionObserver weakref

I don't think this needs to be a sec bug, since the assert appears harmless.  The code takes the address of a deference (&* operators used in conjunction), i.e. it takes the nullptr out of an nsRefPtr for casting purposes but doesn't use it for anything. Furthermore, the NS_PRECONDITION() macro used here to generate the assertion message doesn't appear to throw in debug builds AFAICT.

So the remaining issue is the misleading log junk, which this patch fixes.

Comment 3

[Adam Roach [:abr]]

Comment on attachment 8341705 [details] [diff] [review]
Avoid triggering harmless assertion on PeerConnectionObserver weakref

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

Looks reasonable to me.

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

This is ready to land as soon as we agree it is not a security bug.

Comment 5

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

I agree this is not a sec issue (and even if it were an opt-build crash, it would be a null-deref).  Let's land it.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[Carsten Book [:Tomcat]]

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