340242 - "WARNING: not an nsIRDFRemoteDataSource" during restart due to switching builds

Status: RESOLVED WORKSFORME

(Core :: XUL, defect)

Description

[Jesse Ruderman]

This happened during the "the last build that ran wasn't this one, so I have to restart myself" restart.

++WEBSHELL 0x230e5100 == 1
WARNING: NS_ENSURE_TRUE(cv) failed: file /Users/admin/trunk/mozilla/xpfe/appshell/src/nsXULWindow.cpp, line 1544
++DOMWINDOW == 1
++DOMWINDOW == 2
WARNING: nsExceptionService ignoring thread destruction after shutdown: file /Users/admin/trunk/mozilla/xpcom/base/nsExceptionService.cpp, line 191
###!!! ASSERTION: not an nsIRDFRemoteDataSource: 'remote != nsnull', file /Users/admin/trunk/mozilla/rdf/datasource/src/nsLocalStore.cpp, line 345

Program received signal SIGTRAP, Trace/breakpoint trap.
0x90047e4c in kill ()
(gdb) bt
#0  0x90047e4c in kill ()
#1  0x013a00f0 in Break (aMsg=0xbfffe0b0 "###!!! ASSERTION: not an nsIRDFRemoteDataSource: 'remote != nsnull', file /Users/admin/trunk/mozilla/rdf/datasource/src/nsLocalStore.cpp, line 345") at /Users/admin/trunk/mozilla/xpcom/base/nsDebugImpl.cpp:465
#2  0x013a043c in NS_DebugBreak_P (aSeverity=1, aStr=0x1ef37ab8 "not an nsIRDFRemoteDataSource", aExpr=0x1ef37ad8 "remote != nsnull", aFile=0x1ef37a10 "/Users/admin/trunk/mozilla/rdf/datasource/src/nsLocalStore.cpp", aLine=345) at /Users/admin/trunk/mozilla/xpcom/base/nsDebugImpl.cpp:350
#3  0x1ef22ce4 in LocalStoreImpl::Flush (this=0x297b360) at /Users/admin/trunk/mozilla/rdf/datasource/src/nsLocalStore.cpp:345
#4  0x06f9260c in nsXULDocument::~nsXULDocument (this=0x21cdc00) at /Users/admin/trunk/mozilla/content/xul/document/src/nsXULDocument.cpp:231
#5  0x06d5d488 in nsDocument::Release (this=0x21cdc00) at /Users/admin/trunk/mozilla/content/base/src/nsDocument.cpp:827
#6  0x06ed91dc in nsXMLDocument::Release (this=0x21cdc00) at /Users/admin/trunk/mozilla/content/xml/document/src/nsXMLDocument.cpp:210
#7  0x06f8ab44 in nsXULDocument::Release (this=0x21cdc00) at /Users/admin/trunk/mozilla/content/xul/document/src/nsXULDocument.cpp:296
#8  0x06f9b2e0 in nsXULDocument::ParserObserver::OnStopRequest (this=0x26457d10, request=0x26458ad0, aContext=0x0, aStatus=2152398850) at /Users/admin/trunk/mozilla/content/xul/document/src/nsXULDocument.cpp:4107
#9  0x1ad86dd8 in nsParser::OnStopRequest (this=0x26458620, request=0x26458ad0, aContext=0x0, status=2152398850) at /Users/admin/trunk/mozilla/parser/htmlparser/src/nsParser.cpp:2299
#10 0x1bbc6a00 in nsJARChannel::OnStopRequest (this=0x26458ad0, req=0x26458dc0, ctx=0x0, status=2152398850) at /Users/admin/trunk/mozilla/modules/libjar/nsJARChannel.cpp:749
#11 0x1bf61080 in nsInputStreamPump::OnStateStop (this=0x26458dc0) at /Users/admin/trunk/mozilla/netwerk/base/src/nsInputStreamPump.cpp:566
#12 0x1bf61220 in nsInputStreamPump::OnInputStreamReady (this=0x26458dc0, stream=0x26458e5c) at /Users/admin/trunk/mozilla/netwerk/base/src/nsInputStreamPump.cpp:391
#13 0x01429870 in nsInputStreamReadyEvent::Run (this=0x26459100) at /Users/admin/trunk/mozilla/xpcom/io/nsStreamUtils.cpp:111
#14 0x0138f6d8 in nsThread::ProcessNextEvent (this=0x29130d0, mayWait=0, result=0xbfffe9a0) at /Users/admin/trunk/mozilla/xpcom/threads/nsThread.cpp:482
#15 0x0130d218 in NS_ProcessPendingEvents_P (thread=0x29130d0, timeout=4294967295) at nsThreadUtils.cpp:179
#16 0x01313780 in NS_ShutdownXPCOM_P (servMgr=0x29151d4) at /Users/admin/trunk/mozilla/xpcom/build/nsXPComInit.cpp:710
#17 0x00207290 in ScopedXPCOMStartup::~ScopedXPCOMStartup (this=0xbfffebec) at /Users/admin/trunk/mozilla/toolkit/xre/nsAppRunner.cpp:553
#18 0x0020ff88 in XRE_main (argc=1, argv=0xbfffeef8, aAppData=0x3070) at /Users/admin/trunk/mozilla/toolkit/xre/nsAppRunner.cpp:2378
#19 0x00002c78 in main (argc=1, argv=0xbfffeef8) at /Users/admin/trunk/mozilla/browser/app/nsBrowserApp.cpp:61

(gdb) c
Continuing.
--WEBSHELL 0x230e5100 == 0
--DOMWINDOW == 1
--DOMWINDOW == 0

Comment 1

[(not reading, please use [e-mail] instead)]

I'm seeing this on windows on regular old shutdown.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070323 Minefield/3.0a3pre

###!!! ASSERTION: Profile change cancellation.: 'Error', file c:/builds/trunk-no
-places/mozilla/toolkit/xre/nsXREDirProvider.cpp, line 721
WARNING: nsExceptionService ignoring thread destruction after shutdown: file c:/
builds/trunk-no-places/mozilla/xpcom/base/nsExceptionService.cpp, line 191
###!!! ASSERTION: not an nsIRDFRemoteDataSource: 'remote != nsnull', file c:/bui
lds/trunk-no-places/mozilla/rdf/datasource/src/nsLocalStore.cpp, line 347
--DOMWINDOW == 7
--DOMWINDOW == 6
--DOMWINDOW == 5
--DOMWINDOW == 4
###!!! ASSERTION: not an nsIRDFRemoteDataSource: 'remote != nsnull', file c:/bui
lds/trunk-no-places/mozilla/rdf/datasource/src/nsLocalStore.cpp, line 347

Comment 2

[Nickolay_Ponomarev]

(In reply to comment #1)
> I'm seeing this on windows on regular old shutdown.
> 
Same here, trunk, win32. Lately (no specific moment that I can remember) started getting lots of these on shutdown instead of a single assertion.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think hooking RDF up to cycle collection is what made this start happening more.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So the basic problem here is that when we leak a XUL document past profile-before-change and then free it, we get these assertions.  nsLocalStore::Observe observes profile-before-change, and flushes out its datasource and replaces it with an in-memory data source.  When we then call flush while the datasource is this in-memory data source, we get this assertion.

It seems like XUL documents ought to flush, and perhaps disconnect somehow, their localstores in profile-before-change observers too -- we shouldn't depend on destructors to do flushing since we should never put user-visible behavior in destructors (since you don't want things to break when objects leak or their collection is delayed).

I care because this is the last assertion stopping us from making assertions fatal on tinderbox again.  (I think it was actually a regression from the cycle collector landing itself.)

Comment 5

[[e-mail]]

I assume the destructor flushes the local store to reduce the chances of data loss. Some other system, e.g. flushing on a timer, could be used instead. Or alternatively simply rely on the profile change notification that flushes.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

But what about the data loss resulting from things being written to the local store after we switch to an in-memory data source?

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: turn the assertion into a warning for now so we can make asserts fatal on tinderbox [Checkin: Comment 12]

Comment 8

[[e-mail]]

(In reply to comment #6)
>But what about the data loss resulting from things being written to the local
>store after we switch to an in-memory data source?
Well nobody should be writing to the local store after a profile change (nsXULDocument calls Flush unconditionally but it may not need flushing).

Comment 9

[Brendan Eich [:brendan]]

(In reply to comment #4)
> It seems like XUL documents ought to flush, and perhaps disconnect somehow,
> their localstores in profile-before-change observers too -- we shouldn't depend
> on destructors to do flushing since we should never put user-visible behavior
> in destructors (since you don't want things to break when objects leak or their
> collection is delayed).

This is spot-on. Is there a bug to track this, if this bug is not the bug for that purpose?

/be

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 268955 [details] [diff] [review]
turn the assertion into a warning for now so we can make asserts fatal on tinderbox
[Checkin: Comment 12]

>Temporarily change assertion to warning so we can make assertions fatal on tinderbox.  b=340242
>
>diff --git a/rdf/datasource/src/nsLocalStore.cpp b/rdf/datasource/src/nsLocalStore.cpp
>--- a/rdf/datasource/src/nsLocalStore.cpp
>+++ b/rdf/datasource/src/nsLocalStore.cpp
>@@ -1,4 +1,5 @@
> /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
>+/* vim: set cindent tabstop=4 expandtab shiftwidth=4: */
> /* ***** BEGIN LICENSE BLOCK *****
>  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
>  *
>@@ -332,7 +333,11 @@ LocalStoreImpl::Flush()
> LocalStoreImpl::Flush()
> {
> 	nsCOMPtr<nsIRDFRemoteDataSource> remote = do_QueryInterface(mInner);
>-    NS_ASSERTION(remote != nsnull, "not an nsIRDFRemoteDataSource");
>+    // Temporarily make this a warning rather than an assertion until we
>+    // sort out the ordering of how we write everything to the
>+    // localstore, flush it, and disconnect it when we're getting
>+    // profile-change notifications.
>+    NS_WARN_IF_FALSE(remote != nsnull, "not an nsIRDFRemoteDataSource");
> 	if (! remote)
>         return NS_ERROR_UNEXPECTED;

(Horrors, hard tabs in the file, in defiance of the modelines.)

A useful convention Nat Friedman championed in Ximian code: add a FIXME comment (in preference to XXX) citing a bug by URL or number, in a comment such as this one.

sr=me contingent on FIXME and r=peterv.

/be

Comment 11

[Peter Van der Beken [:peterv]]

Comment on attachment 268955 [details] [diff] [review]
turn the assertion into a warning for now so we can make asserts fatal on tinderbox
[Checkin: Comment 12]

Hopefully someone will step up soon to fix this the right way.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Change from assertion to warning checked in; leaving bug open for fixing it the right way.

Comment 13

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

A clean Mac trunk Thunderbird debug build outputs the following on shutdown:

WARNING: not an nsIRDFRemoteDataSource: 'remote != nsnull', file /Users/skywalker/Desktop/Mozilla/cvs/mozilla/rdf/datasource/src/nsLocalStore.cpp, line 312

Comment 14

[Serge Gautherie (:sgautherie)]

<http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1217484420.1217485284.24505.gz&fulltext=1>
{{
Linux nye Depend bloat on 2008/07/30 23:07:00

WARNING: not an nsIRDFRemoteDataSource: 'remote != nsnull', file /home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/src/mozilla/rdf/datasource/src/nsLocalStore.cpp, line 312
}}

Comment 15

[Jesse Ruderman]

I get two of these on every shutdown now.  Is it ok to kill the warning until the shutdown-ordering bug is fixed?

mrbkap says |mInner| is null, even before being QIed to nsIRDFRemoteDataSource as |remote|.  So maybe it should be silenced just for that case (and maybe even turned back into an assertion at the same time).

Comment 16

[Andrew McCreight [:mccr8]]

nsIRDFRemoteDataSource doesn't exist any more.