1625650 - Crash in [@ nsAbAddressCollector::SetUpAbFromPrefs]

Status: VERIFIED FIXED

(Thunderbird :: Address Book, defect, P1)

Description

[Wayne Mery (:wsmwk)]

Crashes start with 75.0b1 buildid 20200309225314
#7 crash for beta. No crashes seen in daily builds.

This bug is for crash report bp-10b7fccd-12d7-489b-b579-d6ec50200326.

Top 10 frames of crashing thread:

0 xul.dll nsAbAddressCollector::SetUpAbFromPrefs comm/mailnews/addrbook/src/nsAbAddressCollector.cpp:293
1 xul.dll nsAbAddressCollector::Init comm/mailnews/addrbook/src/nsAbAddressCollector.cpp:268
2 xul.dll nsAbAddressCollectorConstructor comm/mailnews/build/nsMailModule.cpp:436
3 xul.dll nsComponentManagerImpl::GetServiceLocked xpcom/components/nsComponentManager.cpp:1372
4 xul.dll nsComponentManagerImpl::GetServiceByContractID xpcom/components/nsComponentManager.cpp:1559
5 xul.dll nsGetServiceByContractID::operator const xpcom/components/nsComponentManagerUtils.cpp:243
6 xul.dll nsCOMPtr_base::assign_from_gs_contractid xpcom/base/nsCOMPtr.cpp:82
7 xul.dll nsMsgComposeAndSend::DeliverFileAsMail comm/mailnews/compose/src/nsMsgSend.cpp:2930
8 xul.dll nsMsgComposeAndSend::DeliverMessage comm/mailnews/compose/src/nsMsgSend.cpp:2901
9 xul.dll nsMsgComposeAndSend::GatherMimeAttachments comm/mailnews/compose/src/nsMsgSend.cpp:883

Comment 1

[Magnus Melin [:mkmelin]]

The cause looks pretty clear: mDirectory doesn't get set here: https://searchfox.org/comm-central/rev/1e97b3c1f2224fbbf7c5479e2ec9f906d1949132/mailnews/addrbook/src/nsAbAddressCollector.cpp#293

Not entirely clear what the right solution is. For XPCOM the convention is that success means you didn't return null... This contract is not really followed for the new js implementation: https://searchfox.org/comm-central/rev/1e97b3c1f2224fbbf7c5479e2ec9f906d1949132/mailnews/addrbook/jsaddrbook/AddrBookManager.jsm#179

Comment 2

[Geoff Lankow (:darktrojan)]

Yes, getDirectory should throw. I recall at one point deciding this, then changing my mind back again. Fortunately I can push all of the UI code that now expects null for the root address book through a wrapper function.

Side note: for this crash to happen the user would have to have an invalid value for mail.collect_addressbook, probably an address book that's been deleted. Should we reset that pref if the directory it points to is deleted?

Comment 3

[Geoff Lankow (:darktrojan)]

Attachment: 1625650-getdirectory-throw-1.diff

Comment 4

[Magnus Melin [:mkmelin]]

(In reply to Geoff Lankow (:darktrojan) from comment #2)

Side note: for this crash to happen the user would have to have an invalid value for mail.collect_addressbook, probably an address book that's been deleted. Should we reset that pref if the directory it points to is deleted?

I think we should.

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9136638 [details] [diff] [review]
1625650-getdirectory-throw-1.diff

Review of attachment 9136638 [details] [diff] [review]:
-----------------------------------------------------------------

r=mkmelin

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/bdbc2ecaead2
AddrBookManager.getDirectory should throw instead of returning null when no directory is found. r=mkmelin

Comment 7

[Wayne Mery (:wsmwk)]

Confirmed - gone from crash-stats