Closed Bug 1152364 Opened 5 years ago Closed 5 years ago

crash in Address Book via nsAbBSDirectory::GetChildNodes nsCOMArrayEnumerator::operator new(unsigned int, nsCOMArray_base const&)

Categories

(MailNews Core :: Address Book, defect, critical)

defect
Not set
critical

Tracking

(thunderbird38+ fixed, thunderbird39 fixed, thunderbird40 fixed)

RESOLVED FIXED
Thunderbird 40.0
Tracking Status
thunderbird38 + fixed
thunderbird39 --- fixed
thunderbird40 --- fixed

People

(Reporter: rkent, Assigned: rkent)

References

Details

(Keywords: crash, regression, topcrash-thunderbird, Whiteboard: [regression:TB38?])

Crash Data

Attachments

(1 file)

0 	xul.dll 	nsCOMArrayEnumerator::operator new(unsigned int, nsCOMArray_base const&) 	xpcom/glue/nsArrayEnumerator.cpp
1 	xul.dll 	NS_NewArrayEnumerator(nsISimpleEnumerator**, nsCOMArray_base const&) 	xpcom/glue/nsArrayEnumerator.cpp
2 	xul.dll 	nsAbBSDirectory::GetChildNodes(nsISimpleEnumerator**) 	c:/builds/moz2_slave/tb-rel-c-beta-w32_bld-00000000/build/mailnews/addrbook/src/nsAbBSDirectory.cpp:98
3 	xul.dll 	nsAbManager::GetDirectories(nsISimpleEnumerator**) 	c:/builds/moz2_slave/tb-rel-c-beta-w32_bld-00000000/build/mailnews/addrbook/src/nsAbManager.cpp:202
#6 crash for 38beta
crash does not exist prior to version 38

Mac signature NS_NewArrayEnumerator(nsISimpleEnumerator**, nsCOMArray_base const&)
Crash Signature: [@ NS_NewArrayEnumerator(nsISimpleEnumerator**, nsCOMArray_base const&)] [@ nsCOMArrayEnumerator::operator new(unsigned int, nsCOMArray_base const&)]
Summary: crash in Address Book nsCOMArrayEnumerator::operator new(unsigned int, nsCOMArray_base const&) → crash in Address Book via nsAbBSDirectory::GetChildNodes nsCOMArrayEnumerator::operator new(unsigned int, nsCOMArray_base const&)
Whiteboard: [regression:TB38?]
NS_NewArrayEnumerator assigns *aResult and we have no aResult != nullptr check. But the calling GetDirectories() does have the check.
The crashing address of 0x5a5a5a5e seems to be some magic value (although in SOME Tb39 crashes it is different), like the pattern put into freed memory.

I'd suspect this could be triggered by something in bug 170270 (I think there were not many other patches to AB C++ code). It adds some of the GetDirectories() calls.

What kind of directory does 'BS' (in nsAbBSDirectory) represent?
Is it fine that is it the "root directory" ?
Interesting. Will have a look at it later today then.

Thanks for letting me know.
So, I am unable to reproduce the crash. So, if there are any specific STRs that
result in the crash, please let me know.

Thanks.
(In reply to Suyash Agarwal (:sshagarwal) from comment #4)
> So, I am unable to reproduce the crash. So, if there are any specific STRs
> that
> result in the crash, please let me know.
> 
> Thanks.

Matt can you comment futher on your crash? " was deleting addresses from the contact pane in the compose windows using Shilf select groups " bp-c7505cd3-18cf-48f9-9d0b-fab272150329
Flags: needinfo?(unicorn.consulting)
Like many, my address book was damaged.  I am not sure what happened in the first place,  but it first showed up in Google synced address books. (gcontactsync)

Massive duplication is what I saw, with some addressed/Contests repeated over a thousand times.  I started composing a message, got sidetracked and started deleting these duplicates (I did not know that worked until then) in the contacts pane of the compose window.  Using Shift to select large numbers at a time.

Pressed delete for perhaps the third or fourth time and got the crash.  I have no duplicates today to try and reproduce the crash with.
Flags: needinfo?(unicorn.consulting)
Perhaps https://crash-stats.mozilla.com/report/index/e37c03fa-9fcf-465c-acad-9942a2150408 is a variation. User reports " Somehow e-mail address books were combined resulting in duplicate addresses. Upon deleting duplicates Thunderbird keeps crashing. "
(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #7)
> Perhaps
> https://crash-stats.mozilla.com/report/index/e37c03fa-9fcf-465c-acad-
> 9942a2150408 is a variation. User reports " Somehow e-mail address books
> were combined resulting in duplicate addresses. Upon deleting duplicates
> Thunderbird keeps crashing. "

Well, the idea of All ABs is to have combined ABs showing duplicate entries. This should've been a part of the release notes so that the users aren't surprised.

TB crashing on an attempt to remove duplication is concerning. I tested it (deletion across multiple ABs at once). I will try to reproduce this usecase once again later today.

Thanks.
If All ABs include also Collected addresses then there may be many dupes shown. Even if user has all other ABs clean. Users must be careful to delete the proper entry - from collected addresses.
(In reply to :aceman from comment #9)
> If All ABs include also Collected addresses then there may be many dupes
> shown. Even if user has all other ABs clean. Users must be careful to delete
> the proper entry - from collected addresses.

So, if user has an address in one of the ABs, 1) will it still be added to Collected Addresses?, 2) If somehow the entry is in both (an AB and Collected Addresses), will deletion result in any dataloss? Maybe because of other fields that may be filled up in other ABs but not in Collected ABs (maybe).
(In reply to Suyash Agarwal (:sshagarwal) from comment #10)
> (In reply to :aceman from comment #9)
> > If All ABs include also Collected addresses then there may be many dupes
> > shown. Even if user has all other ABs clean. Users must be careful to delete
> > the proper entry - from collected addresses.
> 
> So, if user has an address in one of the ABs, 1) will it still be added to
> Collected Addresses?
Even if not, the address may first get into Collected, and then user puts it into Personal AB.

> 2) If somehow the entry is in both (an AB and
> Collected Addresses), will deletion result in any dataloss? Maybe because of
> other fields that may be filled up in other ABs but not in Collected ABs
> (maybe).
Yes, that is the assumption that if the user put the address into Personal AB, then it is manually crafted and with better data. It would be bad to loose that version of the card.
I can reliably reproduce this deleting about 8 cards, so I'll sort it out. The main abook.mab directory for some reason has an extra release, so when the js reference is deleted we crash.
Assignee: nobody → rkent
Status: NEW → ASSIGNED
Attachment #8596336 - Flags: review?(neil)
Attachment #8596336 - Flags: review?(neil) → review+
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
OS: Windows 8.1 → All
Hardware: x86_64 → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 40.0
(In reply to Kent James (:rkent) from comment #12)
> I can reliably reproduce this deleting about 8 cards, so I'll sort it out.
> The main abook.mab directory for some reason has an extra release, so when
> the js reference is deleted we crash.

What precisely are the steps to reproduce?
And, is there concensus this is caused by bug 170270?
Flags: needinfo?(rkent)
(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #15)
> And, is there concensus this is caused by bug 170270?

Bug 170270 seems to introduce the whole getDirectoryFromId() function in nsAbManager.cpp. And in that function lies the bug.
Blocks: 170270
For me, to reproduce all I had to do was to create a directory with around 10 cards in it, then delete them. Crash occurs either immediately or after a few seconds in garbage collect. Normal default address book is sufficient. I was probably deleting using the all AB search view. Both the compose sidebar or the normal address book would show the delete.

The "extra release" I mentioned in comment 12 turned out to be a missing AddRef (which is equivalent). This is a pretty clear fix I think, which is why I just went ahead and pushed it everywhere.
Flags: needinfo?(rkent)
You need to log in before you can comment on or make changes to this bug.