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)

x86
Windows 2000
defect
Not set
major

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)

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).
Blocks: 108795
Keywords: nsbeta1
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.
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?
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. 
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.
Attached patch Patch for PSM (obsolete) — Splinter Review
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
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
Nav triage team: nsbeta+, adt1
Keywords: nsbeta1nsbeta1+
Whiteboard: [adt1]
Whiteboard: [adt1] → [adt1][m5+]
Whiteboard: [adt1][m5+] → [adt1][m5+] [multi-profile]
Target Milestone: --- → mozilla1.0
Any chance BRyner can help with this, since Law has several ADT1 bugs?
Priority: -- → P1
Whiteboard: [adt1][m5+] [multi-profile] → [adt1][m5+] [multi-profile] [ETA ??/??]
lowering impact.  We'll be looking at this for rtm.
Whiteboard: [adt1][m5+] [multi-profile] [ETA ??/??] → [adt2][multi-profile] [ETA ??/??]
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.
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
Whiteboard: [adt2][multi-profile] [ETA ??/??] → [adt2 rtm][multi-profile] [ETA ??/??]
Bill, what is happening with this bug?
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.
Whiteboard: [adt2 rtm][multi-profile] [ETA ??/??] → [adt2 rtm][multi-profile] [ETA ??/??] custrtm-
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.
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.
Keywords: nsbeta1+meta
retargeting
Target Milestone: mozilla1.0 → Future
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 → ---
Assignee: quicklaunch → nobody
Priority: P1 → --
QA Contact: agracebush → quicklaunch
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
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: