Closed Bug 286355 Opened 20 years ago Closed 10 years ago

Need win32 implementation of nsIProfileUnlocker

Categories

(Toolkit :: Startup and Profile System, defect)

Product:
Toolkit
Component:
Startup and Profile System
Platform:
x86
Windows 2000
Type:
defect
Priority:
Not set
Severity:
normal
Points:
13

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla34
Iteration:
34.3
Tracking Flags:
Tracking Status
relnote-firefox --- 34+

People

(Reporter: benjamin, Assigned: bugzilla)

References

(Depends on 2 open bugs, Blocks 5 open bugs)

Details

(Keywords: helpwanted)

Attachments

(2 files, 2 obsolete files)

Pseudocode
(deleted), text/plain
Details
Patch using Restart Manager (v2)
(deleted), patch
benjamin
: review+
Details | Diff | Splinter Review 
Sometimes (for reasons not entirely apparent), Firefox will mostly shutdown
(stop accepting DDE requests) but won't shut down completely and release its
profile lock. This seems to have something to do with having loaded Java and
popup blocking at the same time, or some similarly irreproducible series of events.

We want to give users the option to forcefully terminate this zombie process.
The code needs to figure out which current process owns the profile lock, and
kill that process. The code needs to be hooked up here:
http://lxr.mozilla.org/seamonkey/source/profile/dirserviceprovider/src/nsProfileLock.cpp#499

dbradley says he's willing to look at this over the weekend. Enterprising
experts in the windows API are also welcome to take this bug. Something like the
code in "forcedel" utility may be useful, except that we want to kill the
process, not merely release the file lock:

http://www.codeguru.com/Cpp/W-P/files/fileio/article.php/c1287

It's fine if this code only works in newer windows (win2k+), we can return a
null nsIProfileUnlocker object on older OSes. This code, however, cannot be
copied directly because it is not tri-licensed. We *can* copy header information
from the mingw w32api (specifically ntapi.h).
The forcedel utility uses undocumented/internal Windows functions, which would
be really bad practice for a production application like Firefox.  We can
achieve almost the same effect legitimately in a number of ways:

A) Once we've successfully locked a profile, create an event object and an extra
thread to act as a monitor.  The event object name should be derived from the
profile lock filename.  The thread waits for the event object to be signalled
then calls ExitProcess.  To end the process just signal the event object.  This
might not work if the process is in a really peculiar state, but should be
effective under most circumstances.

B) Once we've successfully locked a profile, record our process ID in it
somewhere (or record the process ID in the lockfile).  Create an event object
whose name is derived from the profile lock filename combined with the process
ID.  To kill the process, we read the process ID number, verify that the event
object exists, then call TerminateProcess.

Both of these approaches should work as far back as Windows 95.  I haven't
thought about it very long so there may well be superior methods.  Remember
though that the process might not be on the local computer, so it is important
to verify that you've got the right process before terminating it!

  Harry.

Harry, I think that solution (B) might be effective (recoding the
COMPUTERNAME:PID in the lockfile). If I'm reading the current code correctly,
that approach is backwards-compatible (e.g. If we launch a Firefox 1.0 while 1.1
is running, 1.0 must still recognize that the profile is locked).

Do you have the time to try implementing this solution? I can provide assistance
as necessary (as can most of irc.mozilla.org#developers).

I'm not sure we care all that much about profile locks from processes that
aren't on the local computer, but perhaps I could be convinced to care.
The problem with both of these approaches is they are good for the future, but
won't work if you want this to work for the existing Firefox 1.0x builds.

But I agree with the previous comment that we should not use undocumented Win32
api functions. I haven't looked yet, so I'm not yet ready to give up on the idea
externally terminating Firefox.
Benjamin: sorry, no.  I'm expert in certain narrow fields, but something like
Firefox is well outside my experience - and I can't afford the amount of time it
would take to get up to speed.  If it would help I'm happy to write some code
fragments but someone else would have to fit them into context.

David: yes, I'm aware this doesn't give us backwards compatibility, but I'm not
sure whether that's a serious problem under normal circumstances?  After all,
the process will go away by itself as soon as the machine is rebooted.

Regarding remote instances of Firefox, there isn't much to be done about them,
we just need to be absolutely sure that we don't (a) kill a random local process
when we think we're killing firefox or (b) think it's safe to ignore or override
the profile lock when it isn't.  It would be nice to tell the user the name of
the computer the process is running on, though.

  Harry.

Can we count on a well known process name to check when we lookup the process by
PID? What I'm thinking is that the code would read in the PID, lookup the
process and verify the name, and then terminate. That would help guard against
accidentally getting the wrong process, though I think Windows doesn't reuse
PID's right away.

Also should I furnish a patch for the lock file change to write the PID?

Lastly any preference on how to store the PID in the lock file? I'm assuming I
can  just take the first number I find in the file.
Attached file Pseudocode (deleted) — 
Might be useful for David to compare with what he's got.

The event object in my code is probably redundant if we check the executable
module name.
> Can we count on a well known process name to check when we lookup the process by
> PID?

No. This is xulrunner code, so the process name might be xulrunner.exe and you
don't know which XUL application a particular xulrunner.exe process is running.

> Also should I furnish a patch for the lock file change to write the PID?

Yes.

> Lastly any preference on how to store the PID in the lock file? I'm assuming I
> can  just take the first number I find in the file.

How about MACHINENAME:PID:CreationDate or somesuch?
I guess the executable name could be written to the lock file along with the PID?

  Harry.

sure... whatever we do, we need to do it right, because we're not going to get a
second chance.
*** Bug 299576 has been marked as a duplicate of this bug. ***
*** Bug 301056 has been marked as a duplicate of this bug. ***
Depending on the resolution of this bug, there may be 110n dependancies so it
really needs to go in before the freeze. This bug fix is needed to finally
cleanup the "profile in use" issue for average users.
Flags: blocking1.8b4?
Flags: blocking-aviary1.1?
What are the l10n dependencies? I'm pretty sure all the strings are already in
place.
There probably are no 110n dependancy issues if this bug gets fixed as proposed.
 However, if this bug does not get fixed for 1.8/Firefox 1.1, the bug I filed
that was duped to this one was for a simple text string to be added to the
dialog indicating a machine restart may be required.  I assume that would be a
new string.  

Beltzner, do you have any comment on this from a usability perspective?  Is this
as big a support issue for average users as I suspect?

Well, it certainly seems to come up a lot (as indicated in bug 253950, comment
8, and from my own anecdotal experience hanging around in #firefox), so I think
that it's safe to say that it's possible and even probabe that this case will be
encountered by the average user.

The current dialog (see attachment 189566 [details]) is an improvement over 1.0.x, but
will still strand the majority of our users who won't understand why if "Firefox
is running" they don't see it on the taskbar. Even of those who are smart enough
to know what the task manager is, it's a subset who know how to kill a lurking
process.

Ideally we'd get a fix in that allows us to simply kill the errant processes
(the installer and update system seem to be able to do this) and then we could
show a dialog like the one bsmedberg mentions in bug 253950, comment 25.

If that isn't possible in the 1.1 timeframe, then I think a slight modification
of the current dialog will go a long way to helping users, even though getting
them to restart the system sucks:

 Firefox is already running, but not responding. To open a new window, you
 must first close all Firefox processes, or restart your system.
Flags: blocking1.8b4?
Flags: blocking1.8b4+
Flags: blocking-aviary1.1?
The bug about wording is 302039, that's the blocker.
Flags: blocking1.8b4+ → blocking1.8b4-
Flags: blocking-aviary1.5?
Flags: blocking-aviary1.5? → blocking-aviary1.5-
Where would a good place be to begin learning about writing nsIimpl.cpp code?
I'd like to do some research on such a subject, and possibly try my hand at
implementing nsIProfileUnlocker on Linux (in a separate bug, if need be).
*** Bug 324516 has been marked as a duplicate of this bug. ***
*** Bug 353498 has been marked as a duplicate of this bug. ***
I've been seeing a lot more of this lately in both nightly branch Firefox builds and nightly trunk Seamonkey builds. So, it seems to go across the lines on both product and trunk/branch. Hope that helps.
Component: XRE Startup → Startup and Profile System
QA Contact: nobody → startup
bsmedberg asked in the last dup a way to enumerate the process(es) locking a file: always on sysinternals I found some kind of solution, even if it uses ZwQuerySystemInformation, that isn't a "frozen" API:

http://forum.sysinternals.com/forum_posts.asp?TID=17533&PID=87788#87788

The example is coded in C#, so it's merely an hint.
As long as the process is not running as a different user this can be done fairly easily.

Here is another example that doesn't use ZwQuerySystemInformation
http://www.codeguru.com/cpp/w-p/system/processesmodules/article.php/c2827/
It is likely that we wouldn't want to enable the SeDebugPrivilege for the app's token
Seems to be doing this a lot more in 3.5.2 version. Not sure on 3.5.3 as of yet.
Blocks: start-faster
Whiteboard: c=startup_misc u= p=
Whiteboard: c=startup_misc u= p= → p=0
Blocks: 982324
No longer blocks: fxdesktopbacklog
Flags: firefox-backlog+
Whiteboard: p=0 → p=13
Assignee: dbradley → nobody
Depends on: 1046942
Assignee: nobody → aklotz
Attached patch Patch using Restart Manager (obsolete) (deleted) — — Splinter Review 
This patch uses the Windows Restart Manager to find the process holding the existing lock. It uses runtime dynamic linking to ensure that we don't break Windows XP support.
Points: --- → 13
Whiteboard: p=13
Attached patch Patch using Restart Manager (obsolete) (deleted) — — Splinter Review 
Removed an extraneous comment
Attachment #8465661 - Attachment is obsolete: true
Attachment #8465692 - Flags: review?(benjamin)
Planning on getting to this on Monday.
Comment on attachment 8465692 [details] [diff] [review]
Patch using Restart Manager

>diff --git a/profile/dirserviceprovider/src/ProfileUnlockerWin.cpp b/profile/dirserviceprovider/src/ProfileUnlockerWin.cpp

>+class ScopedRestartManagerSession

This class needs a little bit of docs. I'm pretty sure from the name that I know what it does, but inputs/outputs should be clearer: it does its work on construction, right? What state changes does it make to its parent ProfileUnlockerWin?

It should probably also be MOZ_STACK_CLASS.

>+{
>+public:
>+  explicit ScopedRestartManagerSession(ProfileUnlockerWin& aUnlocker)
>+    : mError(ERROR_INVALID_HANDLE)
>+    , mHandle((DWORD)-1) // 0 is a valid restart manager handle

Use INVALID_HANDLE_VALUE ?

>+  operator bool()
>+  {
>+    return mError == ERROR_SUCCESS;
>+  }
>+
>+  operator DWORD()
>+  {
>+    return mHandle;
>+  }

Having these two operators looks fragile to me. Can we make these methods like bool ok() and HANDLE handle() ?

>+
>+private:
>+  DWORD               mError;
>+  DWORD               mHandle;

Not HANDLE?

>+nsresult
>+ProfileUnlockerWin::Init()
>+{
>+  if (mRestartMgrModule) {

This looks assertable... we shouldn't ever be dual-initing right?

>+NS_IMETHODIMP
>+ProfileUnlockerWin::Unlock(uint32_t aSeverity)
>+{
>+  if (!mRestartMgrModule) {
>+    nsresult rv = Init();
>+    if (NS_FAILED(rv)) {
>+      return rv;
>+    }
>+  }

Why did you decide that it's better to implicit-initialize instead of explicitly initializing once when we create the object (before we pass it around)? It seems that we might end up showing unlocking UI even when we can't successfully unlock.

>+  // Since we know the use case for the list, we are going to assume that there
>+  // is one process accessing the lock file.
>+  UINT numEntries = 1;
>+  UINT numEntriesNeeded = 0;
>+  RM_PROCESS_INFO info = {0};
>+  DWORD reason = RmRebootReasonNone;
>+  error = mRmGetList(session, &numEntriesNeeded, &numEntries, &info, &reason);
>+  if (error != ERROR_SUCCESS) {
>+    MOZ_ASSERT(error != ERROR_MORE_DATA);

I don't understand this assert. We assume above that there is only one process locking the file, which is usually correct, but I don't know why we'd need to assert it that way.

If there is more than one process locking the file, why don't we at least kill the first one?

>+  DWORD accessRights = PROCESS_QUERY_LIMITED_INFORMATION;
>+  if (aSeverity == FORCE_QUIT) {
>+    accessRights |= PROCESS_TERMINATE;
>+  }

Why this "if"? Don't we already know the severity from above?

>+  WCHAR imageName[MAX_PATH];
>+  DWORD imageNameLen = MAX_PATH;
>+  if (!mQueryFullProcessImageName(otherProcess, 0, imageName, &imageNameLen)) {
>+    return NS_ERROR_FAILURE;
>+  }
>+  nsCOMPtr<nsIFile> otherProcessImageName;
>+  if (NS_FAILED(NS_NewLocalFile(nsDependentString(imageName, imageNameLen),
>+                                false, getter_AddRefs(otherProcessImageName)))) {
>+    return NS_ERROR_FAILURE;
>+  }
>+
>+  imageNameLen = MAX_PATH;
>+  if (!mQueryFullProcessImageName(::GetCurrentProcess(), 0, imageName,
>+        &imageNameLen)) {
>+    return NS_ERROR_FAILURE;
>+  }
>+  nsCOMPtr<nsIFile> thisProcessImageName;
>+  if (NS_FAILED(NS_NewLocalFile(nsDependentString(imageName, imageNameLen),
>+                                false, getter_AddRefs(thisProcessImageName)))) {
>+    return NS_ERROR_FAILURE;
>+  }
>+
>+  // Make sure the image file names match

Why? Especially if you're using nightly and beta or win32 and win64 builds, this check sounds overly conservative. I could probably support checking that the leafname matches, though...


>diff --git a/profile/dirserviceprovider/src/nsProfileLock.cpp b/profile/dirserviceprovider/src/nsProfileLock.cpp
>--- a/profile/dirserviceprovider/src/nsProfileLock.cpp
>+++ b/profile/dirserviceprovider/src/nsProfileLock.cpp
>@@ -2,16 +2,20 @@
> /* This Source Code Form is subject to the terms of the Mozilla Public
>  * License, v. 2.0. If a copy of the MPL was not distributed with this
>  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> 
> #include "nsProfileStringTypes.h"
> #include "nsProfileLock.h"
> #include "nsCOMPtr.h"
> 
>+#if defined(XP_WIN)
>+#include "mozilla/ProfileUnlockerWin.h"
>+#endif
>+
> #if defined(XP_MACOSX)
> #include <Carbon/Carbon.h>
> #include <CoreFoundation/CoreFoundation.h>
> #endif
> 
> #ifdef XP_UNIX
> #include <unistd.h>
> #include <fcntl.h>
>@@ -570,17 +574,21 @@ nsresult nsProfileLock::Lock(nsIFile* aP
>     mLockFileHandle = CreateFileW(filePath.get(),
>                                   GENERIC_READ | GENERIC_WRITE,
>                                   0, // no sharing - of course
>                                   nullptr,
>                                   CREATE_ALWAYS,
>                                   0,
>                                   nullptr);
>     if (mLockFileHandle == INVALID_HANDLE_VALUE) {
>-        // XXXbsmedberg: provide a profile-unlocker here!
>+        if (aUnlocker) {
>+          nsCOMPtr<nsIProfileUnlocker> unlocker(
>+              new mozilla::ProfileUnlockerWin(filePath));

As noted above, this should be explicitly-initialized here and reset to null if we're on an old OS where this won't work.

>diff --git a/toolkit/profile/moz.build b/toolkit/profile/moz.build
>--- a/toolkit/profile/moz.build
>+++ b/toolkit/profile/moz.build
>@@ -10,16 +10,17 @@ XPIDL_SOURCES += [
>     'nsIProfileMigrator.idl',
>     'nsIToolkitProfile.idl',
>     'nsIToolkitProfileService.idl',
> ]
> 
> XPIDL_MODULE = 'toolkitprofile'
> 
> UNIFIED_SOURCES += [ TOPSRCDIR + '/profile/dirserviceprovider/src/nsProfileLock.cpp' ]
>+UNIFIED_SOURCES += [ TOPSRCDIR + '/profile/dirserviceprovider/src/ProfileUnlockerWin.cpp' ]

Pretty sure this needs a windows check.
Attachment #8465692 - Flags: review?(benjamin) → review-
> 
> >diff --git a/toolkit/profile/moz.build b/toolkit/profile/moz.build
> >--- a/toolkit/profile/moz.build
> >+++ b/toolkit/profile/moz.build
> >@@ -10,16 +10,17 @@ XPIDL_SOURCES += [
> >     'nsIProfileMigrator.idl',
> >     'nsIToolkitProfile.idl',
> >     'nsIToolkitProfileService.idl',
> > ]
> > 
> > XPIDL_MODULE = 'toolkitprofile'
> > 
> > UNIFIED_SOURCES += [ TOPSRCDIR + '/profile/dirserviceprovider/src/nsProfileLock.cpp' ]
> >+UNIFIED_SOURCES += [ TOPSRCDIR + '/profile/dirserviceprovider/src/ProfileUnlockerWin.cpp' ]
> 
> Pretty sure this needs a windows check.

Fixed.
Attachment #8465692 - Attachment is obsolete: true
Attachment #8472756 - Flags: review?(benjamin)
Comment on attachment 8472756 [details] [diff] [review]
Patch using Restart Manager (v2)

Still need INVALID_HANDLE_VALUE in the ScopedRestartManagerSession constructor and s/DWORD/HANDLE/ for the mHandle declaration?

r+ with those changes.

Please file a followup bug to use something other than TerminateProcess to kill the other process: we'd probably like to get crash report stacks from this case and that involves either doing some further kind of magic.

Thank you for fixing this!
Attachment #8472756 - Flags: review?(benjamin) → review+
Thanks all for the hard work. Just a quick question. Is there a chance of WinXP support for this?
No. WinXP doesn't expose the API necessary to do this (at least without hacking at the kernel data structures, which is not something we're prepared to do).
Aw dang ok thanks for fast reply. Lots of users on XP though I heard wouldn't that extra data help us out?
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #35)
> Comment on attachment 8472756 [details] [diff] [review]
> Patch using Restart Manager (v2)
> 
> Still need INVALID_HANDLE_VALUE in the ScopedRestartManagerSession
> constructor and s/DWORD/HANDLE/ for the mHandle declaration?

The restart manager APIs have their "handles" typed as DWORDs, not HANDLEs.

> 
> Please file a followup bug to use something other than TerminateProcess to
> kill the other process: we'd probably like to get crash report stacks from
> this case and that involves either doing some further kind of magic.
>
Will do.

> Thank you for fixing this!
Flags: needinfo?(benjamin)
ok
Flags: needinfo?(benjamin)
https://hg.mozilla.org/mozilla-central/rev/3ef71326b263
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Comment on attachment 8472756 [details] [diff] [review]
Patch using Restart Manager (v2)

Excellent work, I'm glad we got this fixed!

>diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp

>       rv = ps->ConfirmEx(nullptr, killTitle, killMessage, flags,
>-                         killTitle, nullptr, nullptr, nullptr, 
>+                         nullptr, killTitle, nullptr, nullptr,

Re-purposing this title string for the button is not ideal from an l10n point of view (using the same string in different context is generally bad, even if they are the same in English). More importantly, in this case the resulting UX is also far from ideal - you need to hit "Close Firefox" to "continue starting" Firefox, from the user's point of view. Verdi called this out on IRC and is filing a UX bug to improve this dialog, so we'll fix that there.
Is this the dialog we see when process is still open:

http://cdn.ghacks.net/wp-content/uploads/2014/08/firefox-already-running.png


This dialog shows when users launches firefox while it is still running. So the "Close Firefox" button would make more sense as "Close & Restart Firefox". Otherwise users have to click "Close Firefox" then go back and click on the icon agian to launch firefox.
(In reply to noitidart from comment #45)
> Is this the dialog we see when process is still open:
> 
> http://cdn.ghacks.net/wp-content/uploads/2014/08/firefox-already-running.png
> 
> 
> This dialog shows when users launches firefox while it is still running. So
> the "Close Firefox" button would make more sense as "Close & Restart
> Firefox". Otherwise users have to click "Close Firefox" then go back and
> click on the icon agian to launch firefox.

Please see bug 1057052.
Release Note Request (optional, but appreciated)
[Why is this notable]: improved UX 
[Suggested wording]: Improved the "Firefox is already running" dialog with an option to recover from a locked Firefox process.
[Links (documentation, blog post, etc)]:
relnote-firefox: --- → ?
Depends on: 1057868
I fixed a trivial fixup for crosscompiling on case sensitive OSes:

https://hg.mozilla.org/integration/mozilla-inbound/rev/f1b290e959ff
Why not use GetModuleFileNameEx instead of QueryFullProcessImageName for WinXP?
(In reply to Xu Zhen from comment #50)
> Why not use GetModuleFileNameEx instead of QueryFullProcessImageName for
> WinXP?

Using GetModuleFileNameEx would not make this patch compatible with XP. There is no documented equivalent to the restart manager APIs in Windows XP.
Added to the release notes: "Improved "Firefox is already running" dialog with an option to recover from a locked Firefox process"
relnote-firefox: ? → 34+
Iteration: --- → 34.3
Flags: qe-verify?
Flags: qe-verify? → qe-verify+
QA Contact: florin.mezei
I've verified this using the latest Firefox 34 Nightly (BuildID=20140829030204) on Win XP x86, Win 7 x64, Win 8 x64 and Win 8.1 x64.

To get this message, I've set Firefox to not ask to pick a profile on startup, started Firefox and loaded multiple tabs (~10 tabs), then closed and quickly reopened Firefox. I was able to get the message on each of the systems I've tried this.

Win XP displays the message "Firefox is already running, but is not responding. To open a new window, you must first close the existing Firefox process, or restart your system." with <OK> button. According to comment 37 this is expected though.

Win 7, 8 and 8.1 display message "Firefox is already running, but is not responding. The old Firefox process must be closed to open a new window." with <Cancel> and <Close Firefox> buttons. I cannot say with full certainty that the <Close Firefox> button works as expected as I do not know a reliable way to lock the process for a long period of time. However, I tried this multiple times, and sometimes Nightly was restarted right after pressing the button, while other times it took several seconds to be able to manually start it, but in all cases the process was closed and Nightly could be started.

Aaron, do you think that the fact that sometimes it takes several seconds for the process to be killed is an issue? Should it be terminated immediately after clicking on <Close Firefox>?
Flags: needinfo?(aklotz)
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #53)
> Aaron, do you think that the fact that sometimes it takes several seconds
> for the process to be killed is an issue? Should it be terminated
> immediately after clicking on <Close Firefox>?

The process is terminated aggressively; I would think that it should be fairly quick. But how long it takes is not really the issue: the important part is that clicking <Close Firefox> should always result in the browser bring restarted.
Flags: needinfo?(aklotz)
(In reply to Aaron Klotz [:aklotz] from comment #55)
> The process is terminated aggressively; I would think that it should be
> fairly quick. But how long it takes is not really the issue: the important
> part is that clicking <Close Firefox> should always result in the browser
> bring restarted.

I would have to double check this but I'm pretty sure that clicking <Close Firefox> does not always restart the browser. Several times, when clicking it, the window was dismissed but Firefox was NOT reopened (I had to manually reopen it)... this happened whenever it took like 3-4 or more seconds to kill the process. Firefox was restarted after clicking the button only when it took around 1-2 seconds max for the process to be killed.

Aaron, what's your take on this?
Flags: needinfo?(aklotz)
I did check in some follow up fixes to address some possible situations where this could occur in bug 1057466. Can you please ensure that your test build includes those changes?
Flags: needinfo?(aklotz)
https://hg.mozilla.org/mozilla-central/rev/f3aa5e7c39e4

Note that this landed on m-c post-uplift, i.e. Gecko 35. In general, this is why we prefer new bugs for follow-up patches.
I did some additional testing today on Win 7 x64 with the latest Firefox 34 Aurora (BuildID=20140904004005) and latest Firefox 35 Nightly (BuildID=20140904030202). 

I can confirm that I no longer see the behavior where clicking <Close Firefox> does not always restart the browser. I tried several times and Firefox is always restarted automatically after clicking <Close Firefox>, even if you wait longer before clicking the button. There are though two cases where Firefox does NOT start automatically. See them detailed below. 

The scenario I used for testing was the following:
1. Opened Firefox with a clean profile and set it to start without asking to pick a profile on startup.
2. Opened 25 tabs with different websites.
3. Closed Firefox.
4. Reopened Firefox and restored session.
5. Quickly selected all tabs so they start to load.
6. Quickly closed Firefox and attempted to restart it.

Many times I got the "Firefox already running" dialog and choosing to close Firefox restarted the application as expected. However, sometimes I got the following two exceptions where Firefox was not restarted:
1. "Profile Missing" dialog after clicking <Close Firefox> - http://www.screencast.com/t/d8cXT4J3AMKZ
- after dismissing the dialog, Firefox does not restart automatically
- after the Firefox process is killed (automatically), then you can manually start Firefox again without getting the dialog
2. Nothing happens when trying to restart Firefox - http://www.screencast.com/t/L0pb2JceY3bP
- sometimes, instead of displaying the "Firefox already running" dialog a new Firefox process is created, and nothing visible happens
- multiple new processes can be launched without displaying anything
- after the initial process is killed (automatically), then you can manually start Firefox again

Aaron, can you review the two exceptions above and let me know:
- are they issues? 
- if yes, should I file them separately or reopen this?
- if these will be tracked separately should I close this issue?
Flags: needinfo?(aklotz)
(In reply to Florin Mezei, QA (:FlorinMezei) [on vacation - Sep 8-15 ] from comment #59)
> Aaron, can you review the two exceptions above and let me know:
> - are they issues? 

Yes, they are both issues since they affect the user experience when using the profile unlocker.

> - if yes, should I file them separately or reopen this?

Let's file each of them separately with their own bugs.

> - if these will be tracked separately should I close this issue?

I think we should make those bugs dependencies of this bug. Let's not flag this one as VERIFIED until those other two are resolved.
Flags: needinfo?(aklotz)
No longer depends on: 1068657
Depends on: 1068657
No longer depends on: 1068650
hey guys, instead of using restart manager, why not this way, so you can even support xp:

on start of firefox profile, create a file with the pid of firefox. or registry entry. on succesfull close of firefox profile it should delete that file. then on next startup look for that file and if its still there then read the pid from the file, and prompt if you want to kill that pid?
well i guess this method wont work if say the user ctrl alt delled the firefox. or computer shutdown. but you can work around that. you can read hte pid from the file and test if its running. OR you can put a lock on that file while firefox is running and when firefox dies then its unlockd and you can access.

what u guys think?
hey all i figured out the undocumented way to do it. it works on winxp. did it in jsctypes here:

https://gist.github.com/Noitidart/d752e2c59793fa2cab3c

can we implement this so we can support winXP?

i know we dont want to use undocumented stuff but this is getting documented over at msdn now.

and also its better than no support :(
I really don't think that the extra work to use an undocumented Windows XP feature is worth the extra engineering.
Do you think they'll get rid of it one day is that why? A lot of stuff seems to rely on NtQueryInformationFile and NtQuerySystemInformation
(In reply to noitidart from comment #65)
> Do you think they'll get rid of it one day is that why? A lot of stuff seems
> to rely on NtQueryInformationFile and NtQuerySystemInformation

Microsoft isn't supporting XP any more. Why should Firefox? Time to move on.
Depends on: 1112710
Flags: qe-verify+
I just wanted to report that I'm still seeing this dead background process once in a while with Mozilla/5.0 (Windows NT 6.1; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0 ID:20151029151421
That's probably bug 1112710.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: