526483 - Crash in [@ WrappedNativeMarker]

Status: RESOLVED INVALID

(Core :: XPConnect, defect)

Description

[Oleg Romashin (:romaxa)]

Not sure is it XPC bug or CC.
But problem is visible on MicroB engine based on 1.9.2 branch:

Open some page, keep it alive, then switch couple of times between offline/non-offline modes:


#0  0x40a52f94 in XPCNativeSet::IsMarked (this=0x0) at js/src/xpconnect/src/xpcprivate.h:1803
#1  0x40a53f18 in XPCNativeSet::Mark (this=0x0) at js/src/xpconnect/src/xpcinlines.h:590
#2  0x40a53fb0 in XPCWrappedNativeProto::Mark (this=0x1e7368)
    at js/src/xpconnect/src/xpcprivate.h:2166
#3  0x40aaa008 in XPCWrappedNative::Mark (this=0x1eb0f0)
    at js/src/xpconnect/src/xpcprivate.h:2550
#4  0x40ab71fc in WrappedNativeMarker (table=0x46b50, hdr=0x1a17cc, number=23, arg=0x0)
    at js/src/xpconnect/src/xpcwrappednativescope.cpp:521
#5  0x41b68b64 in JS_DHashTableEnumerate (table=0x0, etor=0x40ab71cc <WrappedNativeMarker>, arg=0x0)
    at js/src/jsdhash.cpp:743
#6  0x40a573fc in Native2WrappedNativeMap::Enumerate (this=0x1a1510, f=0x40ab71cc <WrappedNativeMarker>, arg=0x0)
    at js/src/xpconnect/src/xpcmaps.h:165
#7  0x40ab76c0 in XPCWrappedNativeScope::MarkAllWrappedNativesAndProtos ()
    at js/src/xpconnect/src/xpcwrappednativescope.cpp:541
#8  0x40a86d90 in XPCJSRuntime::GCCallback (cx=0xd63a0, status=JSGC_FINALIZE_END)
    at js/src/xpconnect/src/xpcjsruntime.cpp:574
#9  0x41220ac4 in DOMGCCallback (cx=0xd63a0, status=JSGC_FINALIZE_END)
    at dom/base/nsJSEnvironment.cpp:3739
#10 0x40a507fc in XPCCycleCollectGCCallback (cx=0xd63a0, status=JSGC_FINALIZE_END)
    at js/src/xpconnect/src/nsXPConnect.cpp:411
#11 0x41b80d7c in js_GC (cx=0xd63a0, gckind=GC_NORMAL) at js/src/jsgc.cpp:3705
#12 0x41b51434 in JS_GC (cx=0xd63a0) at js/src/jsapi.cpp:2484
#13 0x40a4f8e8 in nsXPConnect::Collect (this=0xae860)
    at js/src/xpconnect/src/nsXPConnect.cpp:477
---Type <return> to continue, or q <return> to quit---
#14 0x4185a618 in nsCycleCollector::Collect (this=0x7b0f8, aTryCollections=1)
    at xpcom/base/nsCycleCollector.cpp:2434
#15 0x4185a770 in nsCycleCollector_collect () at xpcom/base/nsCycleCollector.cpp:3129
#16 0x41223894 in nsJSContext::CC () at dom/base/nsJSEnvironment.cpp:3553
#17 0x4122398c in nsJSContext::IntervalCC () at dom/base/nsJSEnvironment.cpp:3641
#18 0x41223c1c in nsJSContext::CCIfUserInactive () at dom/base/nsJSEnvironment.cpp:3631
#19 0x41223fd4 in GCTimerFired (aTimer=0x5640f0, aClosure=0x0)
    at dom/base/nsJSEnvironment.cpp:3667
#20 0x4184ede8 in nsTimerImpl::Fire (this=0x5640f0) at xpcom/threads/nsTimerImpl.cpp:427
#21 0x4184ee9c in nsTimerEvent::Run (this=<value optimized out>)
    at xpcom/threads/nsTimerImpl.cpp:519
#22 0x4184b20c in nsThread::ProcessNextEvent (this=0x59e60, mayWait=1, result=0xbeb287fc)
    at xpcom/threads/nsThread.cpp:527
#23 0x41812248 in NS_ProcessNextEvent_P (thread=0x0, mayWait=1) at nsThreadUtils.cpp:239
#24 0x4184b50c in nsThread::Shutdown (this=0x14c8d8) at xpcom/threads/nsThread.cpp:468
#25 0x40b5048c in nsSocketTransportService::Shutdown (this=0x1964e0)
    at netwerk/base/src/nsSocketTransportService2.cpp:457
#26 0x40b3bf1c in nsIOService::SetOffline (this=0xfa8a0, offline=<value optimized out>)
    at netwerk/base/src/nsIOService.cpp:661
#27 0x40b3ab44 in nsIOService::Observe (this=0xfa8a0, subject=0x0, topic=0x40895d2c "profile-change-net-teardown", data=0x0)
    at netwerk/base/src/nsIOService.cpp:829


(gdb) frame 4
#4  0x40ab71fc in WrappedNativeMarker (table=0x46b50, hdr=0x1a17cc, number=23, arg=0x0)
    at js/src/xpconnect/src/xpcwrappednativescope.cpp:521
521	    val->Mark();
(gdb) l
516	static JSDHashOperator
517	WrappedNativeMarker(JSDHashTable *table, JSDHashEntryHdr *hdr,
518	                    uint32 number, void *arg)
519	{
520	    XPCWrappedNative *val = ((Native2WrappedNativeMap::Entry*)hdr)->value;
521	    val->Mark();
522	    return JS_DHASH_NEXT;
523	}
524	
525	// We need to explicitly mark all the protos too because some protos may be
(gdb) p *val
$5 = {<nsIXPConnectWrappedNative> = {<nsIXPConnectJSObjectHolder> = {<nsISupports> = {_vptr.nsISupports = 0x41a1ac18}, <No data fields>}, 
    mIdentity = 0x1a83e8}, mRefCnt = {mValue = 1}, 
  static _cycleCollectorGlobal = {<nsXPCOMCycleCollectionParticipant> = {<nsScriptObjectTracer> = {<nsCycleCollectionParticipant> = {
          _vptr.nsCycleCollectionParticipant = 0x41a1ac58}, <No data fields>}, <No data fields>}, <No data fields>}, {mMaybeScope = 0x1e7368, 
    mMaybeProto = 0x1e7368}, mSet = 0x1ad600, mFlatJSObject = 0x1ba600, mScriptableInfo = 0x0, mFirstChunk = {mTearOffs = {{mInterface = 0x0, mNative = 0x0, 
        mJSObject = 0x0}}, mNextChunk = 0x0}, mWrapperWord = 0}
(gdb) 

#3  0x40aaa008 in XPCWrappedNative::Mark (this=0x1eb0f0)
    at mozilla/js/src/xpconnect/src/xpcprivate.h:2550
2550	        if(HasProto()) GetProto()->Mark();
(gdb) p mSet
$6 = (XPCNativeSet *) 0x1ad600
(gdb) p *mSet
$7 = {mMemberCount = 8, mInterfaceCount = 32770, mInterfaces = {0xdc8a0}}

(gdb) frame 2
#2  0x40a53fb0 in XPCWrappedNativeProto::Mark (this=0x1e7368)
    at js/src/xpconnect/src/xpcprivate.h:2166
2166	        {mSet->Mark(); 
(gdb) l
2161	    // NOP. This is just here to make the AutoMarkingPtr code compile.
2162	    inline void AutoTrace(JSTracer* trc) {}
2163	
2164	    // Yes, we *do* need to mark the mScriptableInfo in both cases.
2165	    void Mark() const
2166	        {mSet->Mark(); 
2167	         if(mScriptableInfo) mScriptableInfo->Mark();}
2168	
2169	#ifdef DEBUG
2170	    void ASSERT_SetNotMarked() const {mSet->ASSERT_NotMarked();}
(gdb) p mSet
$9 = (XPCNativeSet *) 0x0




Can somebody explain what is the real problem here? and should we check mSet for null here

Comment 1

[Oleg Romashin (:romaxa)]

Ah, and this is ARM version

Comment 2

[Oleg Romashin (:romaxa)]

More debug info
(gdb) p *val->mMaybeProto        
$16 = {mScope = 0x1a14c8, mJSProtoObject = 0x1ba5e0, mClassInfo = {<nsCOMPtr_base> = {mRawPtr = 0x7b77c}, <No data fields>}, mClassInfoFlags = 0, 
  mSet = 0x0, mSecurityInfo = 0x0, mScriptableInfo = 0x0, mOffsets = 0x0}

mSet == 0

Comment 3

[Peter Van der Beken [:peterv]]

There's only one place where a XPCWrappedNativeProto is constructed, we null-check the set there and I don't think anything changes mSet apart from the constructor, so mSet should never be null.

Comment 4

[Johnny Stenback (:jst)]

Seems related to top crash bug 520554 (currently top crash nr 88 for 3.5.4). If this is reproducible and debuggable on ARM, we should probably push hard to figure out what's going on here.

Comment 5

[Oleg Romashin (:romaxa)]

It is reproducible for me very easily.
But I cannot understand how it is possible.

I setup printf's into XPCWrappedNativeProto constructor, Destructor, and Mark() functions.

And it create one object and "aSet" is correct there.
Then it call 2 time Mark function, and mSet is correct there.
Third time it call Mark and mSet magically = null...

Comment 6

[Oleg Romashin (:romaxa)]

Any idea what can I do to debug and catch problematic place?

Comment 7

[Oleg Romashin (:romaxa)]

I have created new mSet2 variable in XPCWrappedNativeProto, and initialized it with the same "Set"...
When crash happens, mSet == 0x0, but mSet2 == still valid and the same as mSet in constructor...

Comment 8

[Oleg Romashin (:romaxa)]

Attachment: Debug state when "good" Mark called,

Here is more debug info at moment when Mark called for XPCWrappedNativeProto 0x1eecf8 and it does not  crash.

Comment 9

[Oleg Romashin (:romaxa)]

Attachment: Debug state when "bad" Mark called, before crash

Here is more debug info at moment when Mark called for XPCWrappedNativeProto
0x1eecf8 and it does crash.

mSet == 0x0, but mSet2 still valid.

Comment 10

[Oleg Romashin (:romaxa)]

I have made this change:
if (mSet)
 mSet->Mark();
else if (mSet2)
 mSet2->Mark();

And it works without crashes.

Comment 11

[Oleg Romashin (:romaxa)]

Attachment: Testcase to reproduce this crash

1) Open attachment from local file system,
2) As soon maps are loaded, Press power button and switch to "Offline mode".
After that bunch of ->Mark() calls happen and crash.

Comment 12

[Oleg Romashin (:romaxa)]

I found also that problem is caused by wrong access to mSecurityInfo variable, which is right before mSet field.
Also I found that we are calling WNProtoSecPolicyClearer which call 
*(proto->GetSecurityInfoAddr()) = nsnull;

And it looks like this is overriding mSet variable.

Comment 14

[Peter Van der Beken [:peterv]]

Is mSecurityInfo pointing to mSet?

Comment 15

[Oleg Romashin (:romaxa)]

I found problem, and that was memory corruption in our code.
I think we should mark this bug as invalid, and reopen bug 520554.