Jesse Ruderman Reporter
	Description • 15 years ago

This assertion was added in bug 490695.  I've hit it a few times, but I haven't managed to reproduce.  Here's a stack trace captured using XPCOM_DEBUG_BREAK=stack and fix-macosx-stack.pl.

###!!! ASSERTION: wrong entry: 'e->mObject == owner', file ../../dist/include/xpcom/nsISupportsImpl.h, line 145

nsCycleCollectingAutoRefCnt::incr(nsISupports*)
nsXBLDocumentInfo::AddRef()
nsXBLDocumentInfo::QueryInterface(nsID const&, void**)
nsXPCOMCycleCollectionParticipant::CheckForRightISupports(nsISupports*)
nsXBLDocumentInfo::cycleCollection::Trace(void*, void (*)(unsigned int, void*, void*), void*)
TraceJSHolder(JSDHashTable*, JSDHashEntryHdr*, unsigned int, void*)
JS_DHashTableEnumerate
XPCJSRuntime::TraceXPConnectRoots(JSTracer*, int)
XPCJSRuntime::TraceJS(JSTracer*, void*)
js_TraceRuntime
js_GC
js_DestroyContext
JS_DestroyContext
mozJSComponentLoader::UnloadModules()
mozJSComponentLoader::Observe(nsISupports*, char const*, unsigned short const*)
NS_ShutdownXPCOM_P
ScopedXPCOMStartup::~ScopedXPCOMStartup()
XRE_main
main
_start
start

	David Baron :dbaron:
	Comment 1 • 15 years ago

This isn't the kind of thing I would expect to happen randomly unless it were a sign of memory corruption, although I could be missing something.

	Andrew Sutherland [:asuth] (he/him)
	Comment 2 • 15 years ago

nsCycleCollector_shutdown gets called on line 819 of nsXPComInit.cpp in NS_ShutdownXPCOM.  This cleans up the purple buffer.

On line 839 of the same function, the module loader shutdown process triggers mozJSComponentLoader::UnloadModules who triggers a GC that wants to use the purple buffer entries.

Presumably when shutdown the purple buffer should either un-purple the purple objects, or shutdown should be moved to take place later.

	Peter Van der Beken [:peterv] Assignee
	Comment 3 • 15 years ago

(In reply to comment #2)
> On line 839 of the same function, the module loader shutdown process triggers
> mozJSComponentLoader::UnloadModules who triggers a GC that wants to use the
> purple buffer entries.

Why would the GC want to use purple buffer entries?

	Peter Van der Beken [:peterv] Assignee
	Comment 4 • 15 years ago

Ah, hmm. Refcounting of any objects that were purple before we call nsCycleCollector_shutdown would be problematic, it's not really related to the GC.

	Peter Van der Beken [:peterv] Assignee
	Comment 5 • 15 years ago

(In reply to comment #1)
> This isn't the kind of thing I would expect to happen randomly unless it were a
> sign of memory corruption, although I could be missing something.

Could be sporadic leaks keeping some objects alive past the last cycle collection? I don't think anything unmarks objects purple after the last cycle collection, so if they are purple at that point we'll end up with a stale pointer to the nsPurpleBufferEntry?

	Peter Van der Beken [:peterv] Assignee
	Comment 6 • 15 years ago

I think this should work (haven't been able to test yet). Other ideas don't seem possible. We could try to block Suspect2/Forget2 from just after scanning for the last cycle collection, but at that point we don't know whether this is the last collection. We could make the purple buffer refcounted after CC shutdown (with mCount the refcount), but nsCycleCollectingAutoRefCnt::incr and nsCycleCollectingAutoRefCnt::decr won't be able to use Suspect2/Forget2 anymore.

I've seen this assertion once, but didn't manage to attach gdb :-(.

	Peter Van der Beken [:peterv] Assignee
	Comment 7 • 15 years ago

Wrong patch, need to not break DEBUG_CC.

	Andrew Sutherland [:asuth] (he/him)
	Comment 8 • 15 years ago

The things that were still alive were nsJSEventListener instances (the only possibility allowed by the backtrace).  I had an impressive reliable 472 of these messages last night, but apparently I made mozmill stop misbehaving or un-broke my own code or something.

	David Baron :dbaron:
	Comment 9 • 15 years ago

Comment on attachment 380650 [details] [diff] [review]
v1

r=dbaron, but maybe condition the whole for loop inside an if (mCount == 0) so we can avoid it most of the time?

	Peter Van der Beken [:peterv] Assignee
	Comment 10 • 15 years ago

I think we should take this on branch, we could end up dereferencing dangling pointers. It's probably rare, but still pretty bad.

	Johnny Stenback (:jst)
	Comment 11 • 15 years ago

Comment on attachment 380701 [details] [diff] [review]
v1.1

sr=jst, but per discussion with beltzner and shaver, we'll hold off on taking this for 1.9.1. We should get this into a dot release once it's been baked on the trunk.

	Andrew Sutherland [:asuth] (he/him)
	Comment 12 • 15 years ago

v1.1 doesn't actually fix my problem.  Probably because it is ignoring mFirstBlock.

	Peter Van der Beken [:peterv] Assignee
	Comment 13 • 15 years ago

Good point, but I'd rather do this then. Try this and please give us steps to reproduce, since you seem to be able to.

	Andrew Sutherland [:asuth] (he/him)
	Comment 14 • 15 years ago

Comment on attachment 380882 [details] [diff] [review]
v1.3
[Checkin: Comment 17 & 20]

Yep, fixes the problem.

STR:
1) Build a thunderbird debug build with http://hg.mozilla.org/users/bugmail_asutherland.org/comm-central-patches/raw-file/221b3dc8bb0b/folder-display-layer.patch applied (patch is against e2d3efb4e7df).
2) Install mozmill and its dependencies (jsbridge, mozrunner).
3) In comm-central/mail/test/mozmill, run python runtest.py --showall -t folderdisplay/test_real_inbox.js.

A more minimal solution would be to write something that observes "xpcom-shutdown" and causes a new nsJSEventListener to come into existence.  I am unclear on how easy this would be to accomplish using xpcshell.

	David Baron :dbaron:
	Comment 15 • 15 years ago

Comment on attachment 380882 [details] [diff] [review]
v1.3
[Checkin: Comment 17 & 20]

sure, although I actually might prefer 1.2 over 1.3, although I don't particularly care.

	Peter Van der Beken [:peterv] Assignee
	Comment 17 • 15 years ago

http://hg.mozilla.org/mozilla-central/rev/65d66232d0b4

	Serge Gautherie (:sgautherie)
	Comment 18 • 15 years ago

Comment on attachment 380882 [details] [diff] [review]
v1.3
[Checkin: Comment 17 & 20]


Per comment 11.

	Daniel Veditz [:dveditz]
	Comment 19 • 15 years ago

Comment on attachment 380882 [details] [diff] [review]
v1.3
[Checkin: Comment 17 & 20]

Approved for 1.9.1.4, a=dveditz for release-drivers

	Serge Gautherie (:sgautherie)
	Comment 20 • 15 years ago

Comment on attachment 380882 [details] [diff] [review]
v1.3
[Checkin: Comment 17 & 20]


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a787dec4c28a

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 21 • 15 years ago

Verified fixed based on the duped bug 497257 and with Firebug installed. No assertion can be seen for a couple of shutdowns. Marking as verified fixed on 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.5pre) Gecko/20091008 Shiretoko/3.5.5pre ID:20091008213747