263799 - crash in [@ NSSRWLock_LockRead] closing Mozilla after open Master Password pref panel

Status: VERIFIED FIXED

(NSS :: Libraries, defect, P1)

Description

[Andrew Schultz]

If I just open Mozilla, open the Master Password pref panel and then close
Mozilla (this is with a profile without any stored passwords), it crashes:

#6  0x027a180d in NSSRWLock_LockRead (rwlock=0x0) at nssrwlk.c:209
#7  0x0277b388 in SECMOD_GetReadLock (modLock=0x0) at pk11list.c:73
#8  0x027740e7 in PK11_LogoutAll () at pk11slot.c:1259
#9  0x04735a60 in nsSecretDecoderRing::LogoutAndTeardown (this=0x89fdbf0) at
nsSDR.cpp:354
#10 0x02728767 in WLLT_ExpirePassword (status=0x8135548) at wallet.cpp:725
#11 0x02721f9e in nsWalletlibService::Observe (this=0x88a7d50, aSubject=0x82c2938, 
    aTopic=0x6bce58 "profile-before-change", someData=0x6bd130) at
nsWalletService.cpp:211
#12 0x0040a9c1 in nsObserverService::NotifyObservers (this=0x82455e0,
aSubject=0x82c2938, 
    aTopic=0x6bce58 "profile-before-change", someData=0x6bd130) at
nsObserverService.cpp:208
#13 0x006a7c55 in nsProfile::ShutDownCurrentProfile (this=0x82c2938,
shutDownType=1) at nsProfile.cpp:1364
#14 0x08062095 in DoOnShutdown () at nsAppRunner.cpp:807
#15 0x0806445b in main (argc=3, argv=0xfef1cfe4) at nsAppRunner.cpp:1803

this regressed between linux trunk 2004100806 and 2004100907, but backing out
bug 261267 and bug 262689 didn't seem to help.

Comment 1

[Andrew Schultz]

this looks like it might be an NSS bug

Comment 2

[Andrew Schultz]

talkback says this affects Mac and Win as well.

Comment 3

[Wan-Teh Chang]

I confirmed this is a known NSS bug in the current
NSS_CLIENT_TAG (which I updated last Friday 2004-10-08
around 5 PM US Pacific Daylight Time).

Bob Relyea will implement a fix and we will get the
fix into NSS_CLIENT_TAG as soon as possible.

Comment 4

[Wan-Teh Chang]

Changed product to NSS.

Comment 5

[Robert Relyea]

In some sense this is a latent bug in the application. It's calling NSS
functions even when NSS has not been initialized. Since PK11_LogoutAll has up to
this point been accidentally safe, NSS, at the very least, should continue to
allow calls to PK11_LogoutAll to occur even if NSS is not initialized (in this
case it will continue to be a noop). This will allow bug for bug binary
compatibility with previous versions of NSS. I'll attach a patch to make it so..

bob

Comment 6

[Robert Relyea]

Attachment: NSS 3.9 Branch: Make PK11LogoutAll safe even if NSS not initialized.

Comment 7

[Robert Relyea]

Attachment: NSS Trunk: Make PK11LogoutAll safe even if NSS not initialized

Comment 8

[Robert Relyea]

There are two patches mostly because the function moved from one file to another
between the NSS 3.9 Branch and the current trunk build.

Comment 9

[Robert Relyea]

Comment on attachment 161790 [details] [diff] [review]
NSS 3.9 Branch: Make PK11LogoutAll safe even if NSS not initialized.

wtc please review the patch.

Comment 10

[Robert Relyea]

Comment on attachment 161791 [details] [diff] [review]
NSS Trunk: Make PK11LogoutAll safe even if NSS not initialized

Same here (patch is identical, file location has changed).

Comment 11

[Wan-Teh Chang]

Comment on attachment 161790 [details] [diff] [review]
NSS 3.9 Branch: Make PK11LogoutAll safe even if NSS not initialized.

r=wtc.

I reproduced the crash and verified that this patch
fixed the crash.

Bob, please check this patch in on the trunk and
the NSS_3_9_BRANCH.

Comment 12

[Wan-Teh Chang]

Bob, do you think we should fix the application, too?

Comment 13

[Robert Relyea]

I think it would be a good idea to bracket the calls with

if (NSS_IsInitialized()) {
}

in code fragments that are trying shutdown NSS even if it wasn't already shutdown.

bob

Comment 14

[Wan-Teh Chang]

Comment on attachment 161790 [details] [diff] [review]
NSS 3.9 Branch: Make PK11LogoutAll safe even if NSS not initialized.

Bob checked in the fix on the NSS trunk
(NSS 3.10) and NSS_3_9_BRANCH (NSS 3.9.3).

I've checked in this patch in the
NSS_CLIENT_TAG for the Mozilla trunk.

Comment 15

[Wan-Teh Chang]

Andrew, could you verify this fix with the latest trunk
build?

Comment 16

[Andrew Schultz]

verified fixed with linux trunk 2004101205

I filed bug 264096 to implement Robert's suggestions for things calling NSS
functions