Last Comment Bug 263799 - crash in [@ NSSRWLock_LockRead] closing Mozilla after open Master Password pref panel
: crash in [@ NSSRWLock_LockRead] closing Mozilla after open Master Password pr...
Status: VERIFIED FIXED
: crash, regression, topcrash
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.9.3
: All All
: P1 critical (vote)
: 3.9.3
Assigned To: Wan-Teh Chang
: Bishakha Banerjee
Mentors:
http://talkback-public.mozilla.org/ta...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-10-10 19:21 PDT by Andrew Schultz
Modified: 2004-10-12 15:42 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
NSS 3.9 Branch: Make PK11LogoutAll safe even if NSS not initialized. (632 bytes, patch)
2004-10-11 14:20 PDT, Robert Relyea
wtc: review+
Details | Diff | Splinter Review
NSS Trunk: Make PK11LogoutAll safe even if NSS not initialized (627 bytes, patch)
2004-10-11 14:21 PDT, Robert Relyea
wtc: review+
Details | Diff | Splinter Review

Description Andrew Schultz 2004-10-10 19:21:04 PDT
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 2004-10-10 19:24:12 PDT
this looks like it might be an NSS bug
Comment 2 Andrew Schultz 2004-10-10 19:27:31 PDT
talkback says this affects Mac and Win as well.
Comment 3 Wan-Teh Chang 2004-10-11 13:36:57 PDT
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 2004-10-11 13:39:07 PDT
Changed product to NSS.
Comment 5 Robert Relyea 2004-10-11 14:07:14 PDT
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 2004-10-11 14:20:29 PDT
Created attachment 161790 [details] [diff] [review]
NSS 3.9 Branch: Make PK11LogoutAll safe even if NSS not initialized.
Comment 7 Robert Relyea 2004-10-11 14:21:14 PDT
Created attachment 161791 [details] [diff] [review]
NSS Trunk: Make PK11LogoutAll safe even if NSS not initialized
Comment 8 Robert Relyea 2004-10-11 14:22:20 PDT
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 2004-10-11 14:24:00 PDT
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 2004-10-11 14:24:41 PDT
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 2004-10-11 15:07:19 PDT
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 2004-10-11 15:11:57 PDT
Bob, do you think we should fix the application, too?
Comment 13 Robert Relyea 2004-10-11 15:41:58 PDT
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 2004-10-11 17:06:26 PDT
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 2004-10-12 14:03:41 PDT
Andrew, could you verify this fix with the latest trunk
build?
Comment 16 Andrew Schultz 2004-10-12 15:42:10 PDT
verified fixed with linux trunk 2004101205

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

Note You need to log in before you can comment on or make changes to this bug.