142123 - thread safety assertions when doing import into address books (ASSERTION: morkObject not thread-safe)

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect)

Description

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

thread safety assertions when doing ldif import

thanks to bienvenu for pointing this out

NTDLL! 77f9f9df()
nsDebug::Assertion(const char * 0x03c58974, const char * 0x1012c4e0, const char 
* 0x1012c4b0, int 528) line 291 + 13 bytes
NS_CheckThreadSafe(void * 0x00302ca0, const char * 0x03c58974) line 528 + 34 
bytes
morkObject::AddRef(morkObject * const 0x03db9ac8) line 68 + 62 bytes
morkRowObject::AddRef(morkRowObject * const 0x03db9ac8) line 121 + 12 bytes
morkRowObject::QueryInterface(morkRowObject * const 0x03db9ac8, const nsID & 
{...}, void * * 0x0517f94c) line 121 + 158 bytes
nsQueryInterface::operator()(const nsID & {...}, void * * 0x0517f94c) line 47 + 
25 bytes
nsCOMPtr<nsIMdbRow>::assign_from_helper(const nsCOMPtr_helper & {...}, const 
nsID & {...}) line 922 + 18 bytes
nsCOMPtr<nsIMdbRow>::nsCOMPtr<nsIMdbRow>(const nsQueryInterface & {...}) line 
566
nsCOMPtr<nsIMdbRow>::Assert_NoQueryNeeded() line 501
nsGetterAddRefs<nsIMdbRow>::~nsGetterAddRefs<nsIMdbRow>() line 1006
nsTextAddress::AddLdifRowToDatabase(int 0) line 971
nsTextAddress::ParseLdifFile(nsIFileSpec * 0x035093d8, unsigned int * 
0x0350fa94) line 909
nsTextAddress::ImportLDIF(int * 0x0517fea4, const unsigned short * 0x03ef41a8, 
nsIFileSpec * 0x035093d8, nsIAddrDatabase * 0x03f0c658, nsString & {...}, 
unsigned int * 0x0350fa94) line 86 + 16 bytes
ImportAddressImpl::ImportAddressBook(ImportAddressImpl * const 0x0350fa50, 
nsIImportABDescriptor * 0x03509370, nsIAddrDatabase * 0x03f0c658, 
nsIImportFieldMap * 0x00000000, int 0, unsigned short * * 0x0517ff0c, unsigned 
short * * 0x0517ff08, int * 0x0517ff14) line 514 + 44 bytes
ImportAddressThread(void * 0x0350da88) line 948 + 62 bytes
_PR_NativeRunThread(void * 0x0350daf0) line 433 + 13 bytes
_threadstartex(void * 0x0350d600) line 212 + 13 bytes
KERNEL32! 77e92ca8()

Comment 1

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

I wonder if the fix for this (and the other addressbook import assertion bugs) 
is to fix our code to create the card, and proxy that over, instead of having 
the import code know about mork.

Comment 2

[David :Bienvenu]

yes, that's the only way I can think of fixing this - if the import thread does
addrefs or releases of the row, we'll get the assertions. Simply proxing over a
pointer to the row does an addref and release in the proxy code itself on the
import thread...

Comment 3

[Mike Fedyk]

Has this been fixed?  I haven't seen this on mozilla 1.1 or 1.2.

Comment 4

[Scott MacGregor]

> Has this been fixed?  I haven't seen this on mozilla 1.1 or 1.2

it hasn't been fixed.  I just saw it, but you need a debug build to see it.

Comment 5

[David :Bienvenu]

I'm not planning on working on this any time soon.

Comment 6

[Mark Banner (:standard8)]

This happens on any address book import, not just ldif. Taking bug and duping bug 180843 to this as that's the older bug.

Comment 8

[Mark Banner (:standard8)]

Attachment: Possible fix for nsIAbLDIFService assertions

Current WIP, I expect it'll need a little work on the Windows compilation side. This fixes the nsIAbLDIFService assertion that we currently get, that would seem to let the test case on bug 439819 pass.

The morkObject not thread-safe assertion still needs looking at.

Comment 9

[Mark Banner (:standard8)]

Attachment: [checked in] Fix nsIAbLDIFService assertions

Fixes the nsIAbLDIFService assertions. David, could you test compile this on Windows please, I think it'll be ok, but just to make sure...

Comment 10

[David :Bienvenu]

ack, sorry,  I don't have access to a windows box at the moment 

Comment 11

[David :Bienvenu]

Comment on attachment 331595 [details] [diff] [review]
[checked in] Fix nsIAbLDIFService assertions

but the patch looks OK otherwise.

Comment 12

[Josh Geenen (:pi)]

The patch doesn't compile due to an error in line 157 of nsOutlookImport.cpp.  There is a missing comma after nsISupports *aSupportService:

+  NS_IMETHOD ImportAddressBook(nsIImportABDescriptor *source,
+                               nsIAddrDatabase *destination,
+                               nsIImportFieldMap *fieldMap,
+                               nsISupports *aSupportService <---
+                               PRBool isAddrLocHome,
+                               PRUnichar **errorLog,
+                               PRUnichar **successLog,
+                               PRBool *fatalError);

Thunderbird compiled with a comma added to that line and the two import tests passed.

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 331595 [details] [diff] [review]
[checked in] Fix nsIAbLDIFService assertions

Checked in: changeset id 42:46d3979475c9

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

Given our new stricter definition of wanted; marking this as wanted-.

Comment 15

[Mark Banner (:standard8)]

This was fully fixed by bug 675407 that actually removed the import thread for address books (as well as a few other changes).