1404857 - Spawning the pingsender must not block shutdown if permission dialog is displayed on Windows 7

Status: RESOLVED WORKSFORME

(Toolkit :: Telemetry, defect, P1)

Description

[Alessio Placitelli [:Dexter]]

As found in bug 1315306 comment 16, on Windows 7 Firefox is blocking shutdown the first time a new version of Firefox/pingsender is deployed because a permission dialog is displayed [1].

If the user waits more than 60s to authorize the execution of the pingsender, Firefox automatically terminates itself (crash).

We should find a way to make ShellExecuteEx not fail/return:

(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from bug 1315306  comment #13)
> Based on comment 12, I think what happens here is when the dialog is shown,
> ShellExecuteExW here fails
> <https://searchfox.org/mozilla-central/rev/
> 6326724982c66aaeaf70bb7c7ee170f7a38ca226/xpcom/threads/nsProcessCommon.
> cpp#500> and as a result the caller in
> https://searchfox.org/mozilla-central/rev/
> 6326724982c66aaeaf70bb7c7ee170f7a38ca226/toolkit/components/telemetry/
> TelemetrySend.jsm#1323 throws an exception which somehow messes up the state
> of the async shutdown code.
> 
> Does this sound plausible?



[1] - https://bugzilla.mozilla.org/attachment.cgi?id=8892443

Comment 1

[Georg Fritzsche [:gfritzsche]]

Is it possible to "inherit" the UAC permissions from the main Firefox binary?
Does this happen with every Firefox update, or just those where pingsender is updated?

Comment 2

[Georg Fritzsche [:gfritzsche]]

Also:
Does this happen once per install or on subsequent updates?
Does it happen when using the proper installer or just when running Fx from a zip archive?

Comment 3

[Alessio Placitelli [:Dexter]]

(In reply to Georg Fritzsche [:gfritzsche] [away Sep 19 - Oct 3] from comment #2)
> Also:
> Does this happen once per install or on subsequent updates?
> Does it happen when using the proper installer or just when running Fx from
> a zip archive?

Emil, any chance you could help us answering these questions and the last one from comment 1?

Comment 4

[Emil Ghitta, QA [:emilghitta]]

I managed to reproduce this issue only on Fx that is run from the zip archive. It seems that running Fx from the installer doesn't pop up the pingsender Windows notification.

It seems that this issue occurs every time I close Firefox.

I had some hard time trying to reproduce this issue on Fx 58 (latest Nightly) but I managed to reproduce this issue with ease on Fx 57 (beta 6).

Comment 5

[Alessio Placitelli [:Dexter]]

(In reply to Emil Ghitta, QA [:emilghitta] from comment #4)
> I managed to reproduce this issue only on Fx that is run from the zip
> archive. It seems that running Fx from the installer doesn't pop up the
> pingsender Windows notification.

Ehsan, do you have any idea on how common is for user to install Firefox using the zip archives? Or who I can ask?

Comment 6

[Alessio Placitelli [:Dexter]]

(In reply to Emil Ghitta, QA [:emilghitta] from comment #4)
> I had some hard time trying to reproduce this issue on Fx 58 (latest
> Nightly) but I managed to reproduce this issue with ease on Fx 57 (beta 6).

Thanks Emil! So, just to confirm, this doesn't happen with Nightly 58 on Windows 7, out of a zip file?

Comment 7

[Emil Ghitta, QA [:emilghitta]]

Yes, I didn't managed to reproduce this issue on the latest Firefox 58(BuildId:20171009220104).

I tried to find the fix for this issue since the last time that this issue was reproducible on Nightly was on 58.0a1 (BuildId:20171002100134).

I found the following pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?startdate=2017-10-02&enddate=2017-10-03

After searching a little more, I found that ,maybe, the patch for Bug 1396855 fixed this issue as well since this was no longer reproducible with the autoland build that contained the patch.

I am not sure about my finding, can you have a look into this? 

Thanks a lot!

Comment 8

[Alessio Placitelli [:Dexter]]

(In reply to Emil Ghitta, QA [:emilghitta] from comment #7)
> Yes, I didn't managed to reproduce this issue on the latest Firefox
> 58(BuildId:20171009220104).
> 
> I tried to find the fix for this issue since the last time that this issue
> was reproducible on Nightly was on 58.0a1 (BuildId:20171002100134).
> 
> I found the following pushlog:
> https://hg.mozilla.org/integration/autoland/pushloghtml?startdate=2017-10-
> 02&enddate=2017-10-03
> 
> After searching a little more, I found that ,maybe, the patch for Bug
> 1396855 fixed this issue as well since this was no longer reproducible with
> the autoland build that contained the patch.
> 
> I am not sure about my finding, can you have a look into this? 

Thank you very much for the pointers! Bug 1396855 is definitely not related to this.
I'll try to take a closer look at the pushlog, but nothing really stood out.

Comment 9

[Alessio Placitelli [:Dexter]]

(In reply to Emil Ghitta, QA [:emilghitta] from comment #7)
> Yes, I didn't managed to reproduce this issue on the latest Firefox
> 58(BuildId:20171009220104).

Ha! This buildId was very, very helpful: looks like bug 1400294 landed around that date and might have affected this.
Emil, sorry if I keep abusing your patience, any chance you could verify if bug 1400294 fixed that problem?

Comment 10

[(no longer active)]

(In reply to Alessio Placitelli [:Dexter] from comment #5)
> (In reply to Emil Ghitta, QA [:emilghitta] from comment #4)
> > I managed to reproduce this issue only on Fx that is run from the zip
> > archive. It seems that running Fx from the installer doesn't pop up the
> > pingsender Windows notification.
> 
> Ehsan, do you have any idea on how common is for user to install Firefox
> using the zip archives? Or who I can ask?

I have no idea, sorry.

Comment 11

[Emil Ghitta, QA [:emilghitta]]

No problem, I'm glad to help.

I can confirm that this issue is not reproducible with the 58.0a1 (BuildId:20171003100226)autoland build that contained the fix for bug 1400294.

Comment 12

[Alessio Placitelli [:Dexter]]

(In reply to Emil Ghitta, QA [:emilghitta] from comment #11)
> No problem, I'm glad to help.
> 
> I can confirm that this issue is not reproducible with the 58.0a1
> (BuildId:20171003100226)autoland build that contained the fix for bug
> 1400294.

Thanks Emil! Out of curiosity, is the pingsender running *at all* when shutting down, on Windows 7, when getting Firefox from a zip archive?

That's the very last question, I promise. I can't verify myself as I don't have a Windows 7 machine around :-)

Thank you for your patience and efforts so far.

Comment 13

[Emil Ghitta, QA [:emilghitta]]

I can see that the pingsender.exe process is running for a short period of time upon closing Firefox (Latest Nightly opened from a zip archive). 

Please needinfo me without hesitation if you have any questions.

Glad to help.

Comment 14

[Alessio Placitelli [:Dexter]]

(In reply to Emil Ghitta, QA [:emilghitta] from comment #13)
> I can see that the pingsender.exe process is running for a short period of
> time upon closing Firefox (Latest Nightly opened from a zip archive). 

Thank you Emil, that's all I needed!