Closed
Bug 137881
Opened 22 years ago
Closed 15 years ago
Detect component failure when switching profiles (in Quick Launch mode)
Categories
(Core Graveyard :: QuickLaunch (AKA turbo mode), defect)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: law, Unassigned)
References
Details
(Keywords: meta, Whiteboard: [adt2 rtm][multi-profile] [ETA ??/??] custrtm-)
Attachments
(2 files, 1 obsolete file)
7.94 KB,
patch
|
Details | Diff | Splinter Review | |
5.65 KB,
patch
|
Details | Diff | Splinter Review |
In the course of discussing problems related to psm/nss while switching profiles while in Quick Launch mode, it became apparent that we likely need a way for components that are observing profile-switching to indicate that they are unable to successfully switch to the new profile. In such cases, the application should terminate after displaying a message to the user. Optionally, the application could respond to the error by starting another instance of the application with the new profile, and then terminate the current one. Note that in this case, the second instance must really start (i.e., not simply pass the request back to the already running, but about to terminate, process).
Updated•22 years ago
|
Comment 1•22 years ago
|
||
The notification part is doable. See http://lxr.mozilla.org/mozilla/source/profile/public/nsIProfileChangeStatus.idl#49 That's what the nsIProfileChangeStatus iface is for. Currently, observers can veto a change before it actually takes place. There isn't a way (but one could be added) for an observer to signal failure in switching. What sort of error can an observer encounter in a switch which should lead to termination of the app? Eventually, this error condition would be returned from nsIProfile::SetCurrentProfile. The app (and this goes for embedding apps which are using profile switching for things other than QuickLaunch) would need to now something about the error in order to decide what to do.
Thanks, Conrad. That interface (nsIProfileChangeStatus) is pretty much what I was visualizing; you're one step ahead of us it seems. The ability for observers to notify us that the app needs to be terminated comes from current discussions with the PSM/NSS team about problems we're having responding to profile changes. It seems that there will likely be cases where they can't properly re-initialize when a profile changes. Rather than run without ssl/certificates/etc. we're figuring it would be better to shut down and restart (preferably automatically). We need to iron out how the error notification flows from the source (the nss library shutdown?) back up the chain to code that can handle the error in the proper manner. It would be nice if that were somewhere in nsNativeAppSupportWin. From there, we could grab the mutex sempaphore that protects Win32 startup, shutdown the IPC mechanism, launch another instance of the application (with "-nosplash -p newProfileName"), and then terminate. That way, the user would get the expected result, just slower.
Comment 3•22 years ago
|
||
I can make it so that this: http://lxr.mozilla.org/mozilla/source/xpfe/bootstrap/nsNativeAppSupportWin.cpp#1661 will return something like NS_ERROR_PROFILE_SWITCH_FAILED. Profile mgr will just return that and it will propagate back to there. Once we're back in nsNativeAppSupportWin, knowing that we ran into that error, Bill, can you take it from there?
Yes. One question: is it possible to get the profile name the user wanted to start at that point? That's in case I can relaunch a new instance of the application with that profile. Maybe currentProfile could be arranged so it holds the name of the profile we tried to switch to?
Comment 5•22 years ago
|
||
From this comment: http://bugzilla.mozilla.org/show_bug.cgi?id=125561#c19 It looks like a fix is in hand the underlying problem with switching profiles. If that is fixed, I don't think this needs to be pursued. Also, this fallback does no good for embeddors requiring PSM to be able to switch profiles and think this attempts to sweep it under the carpet.
Comment 6•22 years ago
|
||
If a profile change observer QIs the "aSubject" param of nsIObserver::Observe() to an nsIProfileChangeStatus, it can call failed() on it. The error will be returned by nsIProfile::SetCurrentProfilie and propagated all the way back up (as far as nsNativeAppSupportWin) When this happens, the chosen profile has been set. Detection of an error in the midst of a switch does not stop the switch. I think it would be more dangerous to leave things in a halfway state.
Comment 7•22 years ago
|
||
This patch has the required changes to PSM. If NSS can not re-initialized, PSM will call Failed(NS_ERROR_NOT_INITIALIZED). (Let me know if you think we should define a new PSM specific error code instead.) I have a small additional fix included in this patch, which I saw when testing the profile failure. The mInitialized variable was set too early, the patch moves this to the point where init really succeeded. (When we set it too early, async entropy can happen on another thread, an the call to feed entropy to NSS would cause a crash. I saw this while PSM showed the error message that NSS init failed.) Right now the patch is not yet what we need. What we need is the ability to restart without having PSM show any error messages. But PSM will show error messages to the user, if init fails. This had been added to support environments where the profile directory is read-only. Currently, I can't detect why init failed. The real approach is to react on a return code from NSS_Shutdown, which should tell us explicitly about the bad state. But right now, NSS_Shutdown returns nothing (void). Bob Relyea already agreed to change that in bug 125561 comment 29. Once we have that change from Bob, we need to update my patch slightly, and activate the block, which is currently inside #idef NSS_Shutdown_ReturnsFailure
Comment 8•22 years ago
|
||
In bug 125561 Bob wrote: > Wan-Teh and I have already discussed this. My only issue was the worry about > backward binary compatibility with previous versions of NSS. I don't think > that's a problem, so we can return a error status on shutdown for you. > Error codes on init: I'll have to check to see what NSS error we map > "CK_MODULE_ALREADY_INITIALIZED" to. It's probably too generic, but I don't think > it would be too difficult to add a new NSS error code and have the > MODULE_ALREADY_INITIALIZED error to map to it. You couldn't distiguish which > module was already initialized (for instance you couldn't tell if it was a smart > card or the built-ins token complaining), but the remedy is the same for either > situation. Bob, this bug tracks the patches needed for implementing the workaround for beta, which adds automatic restart, in case there is a NSS leak / failure detected at runtime. In addition to your patch from 133584, and to being able to suppress PSM triggered error messages, I need at least one of your suggested approaches. I'm fine with either either a failure return code from NSS_Shutdown (prefered) or a specific result code from the init functions.
Depends on: 133584
Comment 9•22 years ago
|
||
Nav triage team: nsbeta+, adt1
Updated•22 years ago
|
Whiteboard: [adt1] → [adt1][m5+]
Updated•22 years ago
|
Whiteboard: [adt1][m5+] → [adt1][m5+] [multi-profile]
Target Milestone: --- → mozilla1.0
Comment 10•22 years ago
|
||
Any chance BRyner can help with this, since Law has several ADT1 bugs?
Priority: -- → P1
Updated•22 years ago
|
Whiteboard: [adt1][m5+] [multi-profile] → [adt1][m5+] [multi-profile] [ETA ??/??]
Comment 11•22 years ago
|
||
lowering impact. We'll be looking at this for rtm.
Whiteboard: [adt1][m5+] [multi-profile] [ETA ??/??] → [adt2][multi-profile] [ETA ??/??]
Comment 12•22 years ago
|
||
Wan-Teh helped us and attached a new patch to bug 133584. It is the patch he added with comment #42. With that new NSS patch, I'll attach an updated version of my patch to this bug, that allows us to automatically detect the failure situation.
Comment 13•22 years ago
|
||
This udpated patch uses the new NSS shutdown failure detection facility, as provided with the patch in bug 133584.
Attachment #80624 -
Attachment is obsolete: true
Updated•22 years ago
|
Whiteboard: [adt2][multi-profile] [ETA ??/??] → [adt2 rtm][multi-profile] [ETA ??/??]
Comment 14•22 years ago
|
||
Bill, what is happening with this bug?
Comment 15•22 years ago
|
||
This might have been fixed by the new turbo mode introduced by the checkin for bug 98673. Can someone confirm if this is so or not.
Updated•22 years ago
|
Whiteboard: [adt2 rtm][multi-profile] [ETA ??/??] → [adt2 rtm][multi-profile] [ETA ??/??] custrtm-
Comment 16•22 years ago
|
||
Stephen, in response to your comment: This bug is not about a problem, it is a tracking bug for a suggested workaround. This workaround will be needed, if we/you are not satisfied that bug 125561 is fixed in a pleasing way. Once bug 145836 and bug 125561 have landed on the branch, testing needs to be done, and a decision reached, whether we think everything is fine, or whether this workaround will still be needed.
Comment 17•22 years ago
|
||
Nav triage team: replacing nsbeta1+ with meta keyword per comment #16. If any work is still needed on this for MachV, please nominate the actual bug that needs fixing.
Comment 19•21 years ago
|
||
the major problem described by this bug (psm) was fixed, and the current turbo code always restarts, so the remaining question is do we want this api? (personally at some point i'd like to change turbo's behavior back to the non restart.)
Assignee: law → quicklaunch
Target Milestone: Future → ---
Updated•16 years ago
|
Assignee: quicklaunch → nobody
Priority: P1 → --
QA Contact: agracebush → quicklaunch
Comment 20•15 years ago
|
||
Quicklaunch/Turbo Mode is no longer supported in Seamonkey 2 and Seamonkey1.X is in the maintenance mode (fixing only security bugs)
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WONTFIX
Assignee | ||
Updated•12 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•