391977 - Crash Reporter does not restart browser with right profile when crashing after an app-initiated restart (software update, extension install, etc)

Status: NEW

(Toolkit :: Crash Reporting, defect)

Description

[Daniel Luz]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007081105 Minefield/3.0a8pre
Build Identifier: 

The "Restart Firefox" button in the Mozilla Crash Reporter does not restore the browser in the right profile; it seems to not recover any command-line arguments from the crashed browser session.

This is quite annoying for those who try to not mess up with their stable-release profiles.

Reproducible: Always

Steps to Reproduce:
1. Start Minefield in a non-default profile
2. Make it crash (you may search Bugzilla for known crashers)
3. Click "Restart Firefox" in the Mozilla Crash Reporter dialog
Actual Results:  
Minefield will restart in your default profile (or will show a profile selection dialog, depending on your settings)

Expected Results:  
Minefield should restart in the profile it was previously being run (possibly even if it was not set by command-line arguments). Should also respect -no-remote and such arguments.

Browser restart (through automatic updates) does a quite good on that at the moment, but I'm not sure the same methods can be applied, as the circumstances are quite different.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007081222 Minefield/3.0a8pre

Comment 3

[Jesse Ruderman]

See also bug 399317, a similar bug about extension-manager restart.  These should probably share code :)

Comment 4

[(not currently active) Ted Mielczarek]

It does in fact attempt to restore the commandline arguments, it has always worked with -profile for me.  Is there a specific way you're running things that doesn't work?

Code for setting restart args:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/xre/nsAppRunner.cpp&rev=1.195#2584

Comment 5

[Daniel Luz]

(In reply to comment #4)

Nothing special, I think. I'm just using a standard shortcut on Windows XP SP2, pointing precisely to:
    "C:\Arquivos de programas\Minefield\firefox.exe" -p Minefield -no-remote
As you can see, no quoted arguments, though my browser is on a path containing spaces (and I'm using -p, not -Profile, btw).

Comment 6

[(not currently active) Ted Mielczarek]

This works sometimes and fails sometimes, and I don't know what the pattern is.  Might be crashing after a restart from an update?  Could use STR.

Comment 8

[(not currently active) Ted Mielczarek]

I think my musing in comment 6 is correct. I just hit this today, I had restarted from an update and then crashed later, and restarting brought up the default profile. Starting fresh and crashing, restarting brought up the correct profile.

I'll do some investigation.

Comment 9

[WildcatRay]

I had this happen with 20080623 nightly build of Minefield (Trunk). In my case, it was today (24-Jun-08) before I updated to 20080624 nightly. (I was troubleshooting another issue.) When I clicked Breakpad's Restart, Minefield used my "default" profile which is my Gran Paradiso profile instead of the Minefield profile that was in use when the crash occurred.

Comment 10

[(not currently active) Ted Mielczarek]

Ok, I've tracked down the problem.

When we restart for an "app initiated restart" (restarting for update, restarting from the EM, etc), we hit this line:
http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3360
and the variable "appInitiatedRestart" is true. 
You can see that second param is "aBlankCommandLine":
http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#1581

so we intentionally blank out the commandline in this case. The normal restart case saves some info in environment variables:
http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3326

But we don't persist those environment variables in the crashreporter, and they get unset after a successful restart, so that information is lost. I think the solution here is to save those environment variables and reset them in the crashreporter like we already do for XUL_APP_FILE:
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#870
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/client/crashreporter.cpp#546

bsmedberg: does that sound correct?

Comment 11

[(not currently active) Ted Mielczarek]

This probably hurts nightly testers more than normal users, because they:
1) Tend to use non-default profiles
2) Get a lot of app-intiated restarts (from nightly updates)
3) Crash more often (it is a Minefield!)

I hit this fairly often because my daily browser is Minefield, started via "firefox.exe -profile c:\profiles\trunk". Not that it crashes all that often on me, but I tend to test the crash reporter fairly often.

Comment 12

[Zack Weinberg (:zwol)]

<cranky> this is the sort of thing that defensive measures like the ones I outlined in bug 443708 would help with. </>

Comment 13

[Zack Weinberg (:zwol)]

gah, bug-number dyslexia strikes again.  I meant bug 443078.

Comment 14

[WildcatRay]

I just had this happen, again, to me. Had Minefield crash on me. When it restarted, it was using my default Fx profile! Hope this doesn't fall through the cracks.

Comment 15

[WildcatRay]

(In reply to comment #14)
> I just had this happen, again, to me. Had Minefield crash on me. When it
> restarted, it was using my default Fx profile! Hope this doesn't fall through
> the cracks.

Forgot to mention that this happened on my Win7 64-bit system.

Comment 16

[Andreas Jung]

(I don't know that much about C++ so I'm not sure if this is correct...)

If you start Firefox regularly with command line arguments, e.g. 
E:\Programme\Minefield\firefox.exe -p Minefield 
it will put these command line arguments into MOZ_CRASHREPORTER_RESTART_ARG_n environment variables (with n=0,1,2,...).

The setting of the environment variables seems to happen in:
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#1173

So in my case the (relevant) environment variables are set like this:
MOZ_CRASHREPORTER_RESTART_ARG_0=E:\Programme\Minefield\firefox.exe
MOZ_CRASHREPORTER_RESTART_ARG_1=-p
MOZ_CRASHREPORTER_RESTART_ARG_2=Minefield
MOZ_CRASHREPORTER_RESTART_ARG_3=
(^if I understand the code correct, this environment variable is cleared/deleted ( http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#1209 ), but this doesn't matter because it doesn't exist.

When Firefox is restarted because of an update, it is restarted without command line arguments.
This alone shouldn't cause problems, because the environment variables are inherited from the parent process.

But the code that sets the environment variables runs again, this time this means:
MOZ_CRASHREPORTER_RESTART_ARG_0=E:\Programme\Minefield\firefox.exe
MOZ_CRASHREPORTER_RESTART_ARG_1=
(^ this environment variable is cleared/deleted, see above)
MOZ_CRASHREPORTER_RESTART_ARG_2=Minefield
(^ this environment variable is left untouched)

Therefore if a crash occurs now, the crash reporter can't restart Firefox with the right command line arguments.

I first wondered why the explicit termination ( http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#1209 ) is even necessary and if it couldn't be removed (because that would fix this bug), but it is necessary e.g. if Firefox was started with command line arguments and then Thunderbird is started out of Firefox (with no / less command line arguments) because else you end up with a mix of environment variables belonging to Firefox and environment variables belonging to Thunderbird (set on the Thunderbird process).

I think to fix this bug without breaking the above case, the code should check if the MOZ_CRASHREPORTER_RESTART_ARG_0 is already set and identical to argv[0].

If both are the same it shouldn't try to fix something that isn't broken and leave the environment variables the same, else do what it does today.

Comment 18

[Andrew Poth]

This bug is now appearing in SeaMonkey 2.10 on Ubuntu Linux 10.04.  Crash Reporter runs, but when the "Restart SeaMonkey" button in Crash Reporter is clicked, nothing happens; SeaMonkey needs to be restarted manually each and every time.  In SeaMonkey 2.9 it usually restarted when the Restart button was clicked, except one time out of ten, or so, it would hang during restart with a futex_wait_queue_me in the waiting channel, requiring one to manually kill it and restart it.  (These futex_wait_queue_me hang events are never reported by Crash Reporter.)  Furthermore, SeaMonkey 2.10 appears to be just as unstable, if not more so, than 2.9.

Comment 19

[Worcester12345]

I lost my whole profile a week and a half ago. What the heck? I think this was on the Aurora version of Firefox, around the beginning of February.

Comment 20

[Jonathan Howard]

Using a restart add-on simple way to reproduce.

Rant: This has been bugging me for months, didn't know cause. Thought it was from me running default profile at same time as nightly. Didn't link it to restart on updates. Finally track it by accident when using a temp profile and find this old bug report.

Comment 21

[pierre.haessig]

I just got hit by this bug. I'm using 52.0a1 (2016-10-17) (64 bits) on Debian testing. The crash happened while just browsing a webpage (maybe somebody can retrieve the crash report. It happened about 30 minutes ago).

After restart, I was on my daily profile for Firefox stable, while I use Aurora with "firefox -p nightly -no-remote".

In fact I've been using Aurora only recently, since Pascal Chevrel call this Summer. I feel that for new contributor to feel safe with Aurora, it's important to keep their existing profile safe! (and now because of Bug 1255740, it's been the second time I end up launching Aurora on my stable profile!!)

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 3 duplicates.
:gsvelto, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 23

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.