Could be the user is double-clicking too much or has set things to launch on single click but still double-clicks things. Installation seemed to go fine in both and it ended up launching two instances of Nightly when both finished.

Though it isn't unusual for an installer to allow multiple launches I don't have a problem with blocking this ability.

(In reply to Edward Lee :Mardak from comment #0) > Could be the user is double-clicking too much or has set things to launch on > single click but still double-clicks things. > > Installation seemed to go fine in both and it ended up launching two > instances of Nightly when both finished. Hmm...I actually hit a case just now that also causes the installer to never finish installing Nightly if you enter the "installing" phase of the stub installer at the exact same time with 2 stub installers. Although this took some timing to get it to work though.

Changed to create the mutex earlier.

Changed to wanted but not blocking based on comment #2

Comment on attachment 670216 [details] [diff] [review] patch rev2 Review of attachment 670216 [details] [diff] [review]: ----------------------------------------------------------------- What about using the brand name instead of the path? ::: browser/installer/windows/nsis/stub.nsi @@ +184,5 @@ > + ; multiple times. > + StrCpy $1 "$EXEPATH" > + ; Backslashes are illegal in a mutex name so replace all occurences of a > + ; backslash with a forward slash. > + ${WordReplace} "$1" "\" "/" "+" $1 I'm fine with this, but just wondering if you know the difference between ${WordReplace} with + and with +* ; + : replace all found ; +* : multiple-replace all found @@ +190,5 @@ > + > + ; The lpName parameter for CreateMutexW is limited to MAX_PATH characters so > + ; use the characters at the end since they are more likely to be unique. > + ${If} $2 > ${MAX_PATH} > + IntOp $3 $2 - ${MAX_PATH} nit: I don't thinkt his line is needed since $3 is not used. @@ +193,5 @@ > + ${If} $2 > ${MAX_PATH} > + IntOp $3 $2 - ${MAX_PATH} > + StrCpy $1 "$1" ${MAX_PATH} -${MAX_PATH} > + ${EndIf} > + System::Call "kernel32::CreateMutexW(i 0, i 0, w '$1') i .r0 ?e" If you want to protect against people in other logged on sessions starting from the same path then prefix with Global\ @@ +204,5 @@ > + ; executable but that is an uninteresting edge case. > + FindWindow $1 "#32770" "$(WIN_CAPTION)" 0 > + ${If} $1 != 0 > + ; Restore the window if it is minimized and make it the foreground window > + System::Call "user32::ShowWindow(i r1, i 9) i." nit: Add comment SW_RESTORE @@ +207,5 @@ > + ; Restore the window if it is minimized and make it the foreground window > + System::Call "user32::ShowWindow(i r1, i 9) i." > + System::Call "user32::SetForegroundWindow(i r1) i." > + ${EndIf} > + Abort Don't we want Quit here?

(In reply to Brian R. Bondy [:bbondy] from comment #6) > Comment on attachment 670216 [details] [diff] [review] > patch rev2 > > Review of attachment 670216 [details] [diff] [review]: > ----------------------------------------------------------------- > > What about using the brand name instead of the path? I considered that but I kind of prefer this since if the mutex uses the path the call to Findwindow with the window caption will always find the window and bring it to front whereas there may be developers or people getting squirrelly that launch the stub to install multiple copies. I wouldn't be at all surprised if a bug gets filed to allow this at some point and the patch prevents the case this bug is about. > ::: browser/installer/windows/nsis/stub.nsi > @@ +184,5 @@ > > + ; multiple times. > > + StrCpy $1 "$EXEPATH" > > + ; Backslashes are illegal in a mutex name so replace all occurences of a > > + ; backslash with a forward slash. > > + ${WordReplace} "$1" "\" "/" "+" $1 > > I'm fine with this, but just wondering if you know the difference between > ${WordReplace} with + and with +* > ; + : replace all found > ; +* : multiple-replace all found So, using replace with the string "a b" replacing " " with " " will result in " ". So, using multiple-replace with the string "a b" replacing " " with " " will result in " ". > > @@ +190,5 @@ > > + > > + ; The lpName parameter for CreateMutexW is limited to MAX_PATH characters so > > + ; use the characters at the end since they are more likely to be unique. > > + ${If} $2 > ${MAX_PATH} > > + IntOp $3 $2 - ${MAX_PATH} > > nit: I don't thinkt his line is needed since $3 is not used. Yep. Saw that earlier and forgot to remove it. Done > @@ +193,5 @@ > > + ${If} $2 > ${MAX_PATH} > > + IntOp $3 $2 - ${MAX_PATH} > > + StrCpy $1 "$1" ${MAX_PATH} -${MAX_PATH} > > + ${EndIf} > > + System::Call "kernel32::CreateMutexW(i 0, i 0, w '$1') i .r0 ?e" > > If you want to protect against people in other logged on sessions starting > from the same path then prefix with Global\ Right and I think that would be confusing for those people. > @@ +204,5 @@ > > + ; executable but that is an uninteresting edge case. > > + FindWindow $1 "#32770" "$(WIN_CAPTION)" 0 > > + ${If} $1 != 0 > > + ; Restore the window if it is minimized and make it the foreground window > > + System::Call "user32::ShowWindow(i r1, i 9) i." > > nit: Add comment SW_RESTORE I'll add a define if it isn't already available and use it. > @@ +207,5 @@ > > + ; Restore the window if it is minimized and make it the foreground window > > + System::Call "user32::ShowWindow(i r1, i 9) i." > > + System::Call "user32::SetForegroundWindow(i r1) i." > > + ${EndIf} > > + Abort > > Don't we want Quit here? Abort in oninit exits.

I moved the code to after elevation since the unelevated code it unable to bring an instance that is elevated to the front.

Comment on attachment 670230 [details] [diff] [review] patch rev3 Review of attachment 670230 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks for the info in the last comment.

If it's not too late, please also add a call to close the mutex handle.

... inside the ${Unless} $0 == 0. Otherwise the OS has to clean it up and it might not work correctly if you launch the app again right away before the OS cleans it up.

Agreed that the OS has to clean it up though I haven't had an issue ever with it doing so and I am ok on waiting on having it wait until the OS cleans it up.

Pushed to mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/20f9a0406524

(In reply to Robert Strong [:rstrong] (do not email) from comment #12) > Agreed that the OS has to clean it up though I haven't had an issue ever > with it doing so and I am ok on waiting on having it wait until the OS > cleans it up. The only time I've had a problem with this is in terms of automated scripts with repeated launches of the application.

Verified on Nightly 10/12.

Pushed to mozilla-aurora https://hg.mozilla.org/releases/mozilla-aurora/rev/2c27ae268b7d