1. Load the testcase. 2. Enable accessibility, e.g. by pasting the following into the js console: Components .classes["@mozilla.org/accessibilityService;1"] .getService(Components.interfaces.nsIAccessibleRetrieval); 3. Initiate a print-to-file. (A small Firefox window should appear with a progress bar.) 4. Quit Firefox before the printing progress bar reaches 100%. Result: ###!!! ASSERTION: op == PL_DHASH_LOOKUP || RECURSION_LEVEL(table) == 0: 'op == PL_DHASH_LOOKUP || RECURSION_LEVEL(table) == 0', file pldhash.c, line 612 ###!!! ASSERTION: PL_DHASH_ENTRY_IS_LIVE(entry): 'PL_DHASH_ENTRY_IS_LIVE(entry)', file pldhash.c, line 721 Crash [@ nsRefPtr<nsDocAccessible>::~nsRefPtr]

I can't reproduce the crash in opt.

David, can you investigate this?

I didn't recreate this on Solaris with GNOME a11y on. Instead, I got an assertion at cairo-hash.c:195 hash_table->live_entries == 0. For step 2, I pasted the line to web console, and I got permission denied. I guess I don't need this step since I've GNOME a11y on.

Checked log, I saw ###!!! ASSERTION: op == PL_DHASH_LOOKUP || RECURSION_LEVEL(table) == 0: 'op == PL_DHASH_LOOKUP || RECURSION_LEVEL(table) == 0', file pldhash.c, line 611 ###!!! ASSERTION: PL_DHASH_ENTRY_IS_LIVE(entry): 'PL_DHASH_ENTRY_IS_LIVE(entry)', file pldhash.c, line 720 So, I think I did reproduce the bug.

At http://hg.mozilla.org/mozilla-central/file/824f8a023254/accessible/src/base/nsAccDocManager.cpp#l502 We called aDocAccessible->Shutdown(); It will do NotifyOfDocumentShutdown(), and remove the doc accessible from cache. So when we return PL_DHASH_REMOVE for ClearDocCacheEntry(), the entry was already removed, crash. I think return PL_DHASH_NEXT will solve the problem. Normally we shutdown all the documents in other places and ClearDocCacheEntry() is never called.

Er, is it safe to do remove while enumerator is running?

(In reply to comment #8) > Er, is it safe to do remove while enumerator is running? It should be, we may run out of hash but it looks like this should happen in the case of PL_DHASH_REMOVE (http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/pldhash.c#735). We need somebody who knows xpcom hash code to check.

(In reply to comment #9) > (In reply to comment #8) > > Er, is it safe to do remove while enumerator is running? > > It should be, we may run out of hash but it looks like this should happen in > the case of PL_DHASH_REMOVE > (http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/pldhash.c#735). > > We need somebody who knows xpcom hash code to check. According to the comment in nsTHashtable.h, RawRemoveEntry() is safe to use during enumeration, but RemoveEntry() is not. NotifyOfDocumentShutdown() uses RemoveEntry().

Other ideas?

other idea would be ignore NotifyOfDocumentShutdown() when we are shut down the accessibility service. For that we should move gIsShutdown = PR_TRUE before nsAccDocManager::Shutdown(), it should be safe but I didn't check. Your approach is fine I think, though that while makes me a little bit worry because what if document didn't removed itself from document cache, then we get infinite loop. That shouldn't ever happen though. Please let me know what you prefer.

I'd prefer my approach, I think it is easier to maintain.

Comment on attachment 494328 [details] [diff] [review] patch >+ *(nsDocAccessible**)aUserArg = aDocAccessible; use reinterpret_cast >+void nsAccDocManager::ClearDocCache() void on own line

(In reply to comment #3) > David, can you investigate this? Apparently no need when I'm surrounded by awesome :) (Thanks Ginn!)

Comment on attachment 494328 [details] [diff] [review] patch Approving patch to land on trunk (please fix Alexander's nits).

Landed on Ginn's behalf: http://hg.mozilla.org/mozilla-central/rev/4661521b737c (Try server ran green)