423539 - Enormous amount of "morkObject not thread-safe" assertions when importing mail

Status: RESOLVED FIXED

(MailNews Core :: Import, defect)

Description

[Håkan Waara]

Attachment: Patch

When importing mail from any app in Thunderbird, you get lots of these:

###!!! ASSERTION: morkObject not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /Users/hakan/Programmering/mozilla/hg/applemail/mozilla/db/mork/src/morkObject.cpp, line 67

The problem is simply that we commit to the DB from another thread, without using a proxy object.

Beckley, feel free to give this a spin with your Eudora importer to see if it fixes everything for you (and doesn't regress anything).

Comment 1

[Jeff Beckley]

Yes, that gets rid of quite a few assertions.  Good job!

There seems to be one type left, though.  I get a number of these still:

###!!! ASSERTION: nsSupportsArrayEnumerator not thread-safe: '_mOwningThread.GetThread() ==
PR_GetCurrentThread()', file
/Users/Beckley/mozilla/xpcom/ds/nsSupportsArrayEnumerator.cpp,
line 153

Comment 2

[Håkan Waara]

(In reply to comment #1)
> Yes, that gets rid of quite a few assertions.  Good job!
> 
> There seems to be one type left, though.  I get a number of these still:

Yeah, I still get that and a bunch of other ones. Please file new bugs on those if you find a fix. 

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 310111 [details] [diff] [review]
Patch

I wasn't quite able to reproduce this myself (not got a mac for one), but given the fact the rest of the ImportMailThread folder interfaces use proxy to main, then this is appears to be the correct fix.

two nits:

+        rv = proxyObjMgr->GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
+                                              NS_GET_IID(nsIMsgFolder),
+                                              newFolder,
+                                              NS_PROXY_SYNC | NS_PROXY_ALWAYS,
+                                              getter_AddRefs(newFolderProxy));

The newlines should align with the start of the NS_PROXY_TO_MAIN_THREAD

+        else {
+          // probably a fatal error if you can't commit the mailbox, right?
+          fatalError = PR_TRUE;
+        }

No need for the { } here.

r=me with those fixed.

Comment 4

[Mark Banner (:standard8)]

(In reply to comment #1)
> Yes, that gets rid of quite a few assertions.  Good job!
> 
> There seems to be one type left, though.  I get a number of these still:
> 
> ###!!! ASSERTION: nsSupportsArrayEnumerator not thread-safe:
> '_mOwningThread.GetThread() ==
> PR_GetCurrentThread()', file
> /Users/Beckley/mozilla/xpcom/ds/nsSupportsArrayEnumerator.cpp,
> line 153

Note that nsSupportsArray should really die as its obsolete. Not saying that it'll fix the problem, but it may help.

Håkan, bonus points if you can fix bug 180843 as well ;-)

Comment 5

[Håkan Waara]

Checked in, with sr=Neil over IRC.

Comment 6

[timeless]

this isn't very correct. afaict the object you're proxying isn't threadsafe and you created a proxy for it from some random thread.