1617530 - Crash in [@ RtlAcquireSRWLockExclusive | nsWabAddressBook::Initialize]

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[Geoff Lankow (:darktrojan)]

This bug is for crash report bp-825dd8ad-f6db-4f29-87c2-0624d0200224.

Top 10 frames of crashing thread:

0 ntdll.dll RtlAcquireSRWLockExclusive 
1 xul.dll nsWabAddressBook::Initialize comm/mailnews/addrbook/src/nsWabAddressBook.cpp:113
2 xul.dll nsWabAddressBook::nsWabAddressBook comm/mailnews/addrbook/src/nsWabAddressBook.cpp:97
3 xul.dll nsAbWinHelperGuard::nsAbWinHelperGuard comm/mailnews/addrbook/src/nsAbWinHelper.cpp:915
4 xul.dll nsAbOutlookInterface::GetFolderURIs comm/mailnews/addrbook/src/nsAbOutlookInterface.cpp:32
5 xul.dll XPTC__InvokebyIndex 
6  @0xce1d9f9dd7 
7 xul.dll nsXPTType::IsDependent const xpcom/reflect/xptinfo/xptinfo.h:258
8 xul.dll static XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:1145
9 xul.dll XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:947

Comment 1

[Geoff Lankow (:darktrojan)]

This crash is at startup when the Windows address book is enabled. To reproduce it set these prefs:

• ldap_2.servers.oe.uri: moz-aboutlookdirectory://oe/
• ldap_2.servers.oe.dirType: 3

I know it's not bug 1614265, here's a report from before that landed: https://crash-stats.mozilla.com/report/index/2a99887d-71fe-4fca-9d1a-e54bf0200224

Comment 2

[Geoff Lankow (:darktrojan)]

Alice, can you see if you can find this please? I believe it was working in early February but I can't be any more specific than that.

Comment 3

[Alice0775 White]

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

Alice, can you see if you can find this please? I believe it was working in early February but I can't be any more specific than that.

What is "Windows address book"?

Comment 4

[Geoff Lankow (:darktrojan)]

Thunderbird can import contacts from the "Contacts" folder in Windows (C:\Users\Username\Contacts). It's not enabled by default. Setting the two prefs in comment 1 is enough to trigger a crash when starting. If Thunderbird starts without crashing then I consider that not broken for this bug.

Comment 5

[Alice0775 White]

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=306ac4b77c679440f825d487a9147544591db953&tochange=a04956310d2fe4df06c9ba41cb7bb239a09341d4
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5bfe22b4bd0d64b9d105b66b3ed0fcee10d70e93&tochange=5bfe22b4bd0d64b9d105b66b3ed0fcee10d70e93

Comment 6

[Geoff Lankow (:darktrojan)]

Thank you Alice!

Comment 7

[Geoff Lankow (:darktrojan)]

Oh no, my instructions were bad. You also need a value in ldap_2.servers.oe.description – any string value will do.

Could you try again please?

Comment 8

[Alice0775 White]

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

Oh no, my instructions were bad. You also need a value in ldap_2.servers.oe.description – any string value will do.

Could you try again please?

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=91f77a3708ffa978462e80a87764ca364ad669f6&tochange=eca9d62cfb2ca1c44e6bae3d66776ef5613b9c28
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=485e623584d63bbaba3a8b8267bb1d6e55c68042&tochange=7b794b53736c781545273accf33ed5bd23d0abea

Comment 9

[Geoff Lankow (:darktrojan)]

Aha! That's much more like what I was expecting. Thanks again.

Comment 10

[Geoff Lankow (:darktrojan)]

Ben, would you have a look please. https://hg.mozilla.org/comm-central/rev/eca9d62cfb2ca1c44e6bae3d66776ef5613b9c28 is looking very suspicious at the moment.

Comment 11

[Ben Campbell]

Attachment: 1617530-fix-borked-mutex-1.patch

Yes, looks like my fault all right...
I think I completely misunderstood StaticAutoPtr - I thought it created the mutex and deleted it upon shutdown, but it utterly doesn't.
This patch should fix the crash, but it won't delete the mutex upon shutdown, so that'll leak (but better a little leak than a big crash).
I'll come up with a more elegant solution tomorrow.

Comment 12

[Ben Campbell]

Attachment: 1617530-use-StaticMutex-in-nsAbWinHelper-1.patch

StaticMutex seems to fit the bill nicely here - don't have to worry about setting it up or destroying it. I'd prefer if it was just a plain Mutex, but they can't be used as global or class-static variables under windows, so StaticMutex it is.

I also took the opportunity to rename mEntryCounter to sEntryCounter, because it's class-static too and the 'm' was irking me :-)

Caveat - untested (I'm on Linux and this code is only compiled in under Windows).
Try run here:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=cecefa30458278d9bbb6a9cc8f5f027c7f019a12

... so it does compile OK.
A couple of test fails, but I'm not certain they're due to this...

Comment 13

[Geoff Lankow (:darktrojan)]

Comment on attachment 9129679 [details] [diff] [review]
1617530-use-StaticMutex-in-nsAbWinHelper-1.patch

Well it's stopped crashing, that works for me.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a2b845d81aa7
Switch to use StaticMutex in nsAbWinHelper. r=darktrojan

Comment 15

[Wayne Mery (:wsmwk)]

No nightly crashes after buildid 20200226125814 and patch landed 20200301 - crash rate too low to verify via crash-stats that the crash is gone.

Comment 16

[Geoff Lankow (:darktrojan)]

All of the existing crashes (that I know about) were me, so that's not really a surprise.