Closed Bug 1374837 Opened 6 years ago Closed 6 years ago

Browser Toolbox still not working in Windows 7 or Windows 10 in a local build

Categories

(DevTools :: General, defect, P2)

Product:
DevTools
Component:
General
Type:
defect

Tracking

(firefox54 unaffected, firefox55- wontfix, firefox56- wontfix, firefox57 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 57
Tracking Flags:
Tracking Status
firefox54 --- unaffected
firefox55 - wontfix
firefox56 - wontfix
firefox57 --- fixed

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(2 files, 1 obsolete file)

Bug 1374837 - Add some additional logging in case the Browser Toolbox fails to open;
59 bytes, text/x-review-board-request
jryans
: review+
Details
Bug 1374837 - Pipe stderr to stdout to fix the Browser Toolbox in certain Windows builds;
59 bytes, text/x-review-board-request
kmag
: review+
Details 
I just tested a local build on Windows 7 and am not able to open the Browser Toolbox.  Same issue as in Bug 1371548
I've confirmed in a local build that backing out https://hg.mozilla.org/mozilla-central/rev/0d16a08bbcfa fixes the problem.  Kris, are you able to reproduce this in Win7?  Or is there any other info that would help?  No errors are showing up in the Browser Console.
Flags: needinfo?(kmaglione+bmo)
status-firefox54: --- → unaffected
status-firefox55: --- → affected
status-firefox56: --- → affected
tracking-firefox55: --- → ?
Target Milestone: --- → Firefox 55
I don't have a working Windows 7 VM at the moment, so I've only been able to test on Windows 10, but I assumed that if it was failing on Windows 7 we'd be seeing test failures.

If you don't see any browser console output, it might help to run with -console or change the dump() line here:

http://searchfox.org/mozilla-central/rev/714606a8145636d93b116943d3a65a6a49d2acf8/toolkit/modules/subprocess/subprocess_common.jsm#137

to Cu.reportError or Services.console.logStringMessage.
Flags: needinfo?(kmaglione+bmo)
A strange but also good update: the browser toolbox does open in Nightly on the same profile, just not locally
Caught the promise rejection at https://dxr.mozilla.org/mozilla-central/source/devtools/client/framework/ToolboxProcess.jsm#283, with this error:

message:"Failed to create pipe"
fileName:"resource://gre/modules/subprocess/subprocess_worker_win.js"
lineNumber:400
column:(void 0)
errorCode:(void 0)
stack:
initPipes@resource://gre/modules/subprocess/subprocess_worker_win.js:400:13
spawn@resource://gre/modules/subprocess/subprocess_worker_win.js:465:19
BaseProcess@resource://gre/modules/subprocess/subprocess_worker_common.js:64:5
Process@resource://gre/modules/subprocess/subprocess_worker_win.js:323:11
spawn@resource://gre/modules/subprocess/subprocess_worker_common.js:126:19
onmessage/<@resource://gre/modules/subprocess/subprocess_worker_common.js:197:27
onmessage@resource://gre/modules/subprocess/subprocess_worker_common.js:196:3
EventHandlerNonNull*@resource://gre/modules/subprocess/subprocess_worker_common.js:191:1
@resource://gre/modules/subprocess/subprocess_worker_win.js:13:1
Interesting... Is this a 32 bit or 64 bit build?
Attached a patch with some extra logging that was useful to find the error.  That can be landed alongside or folded into any other fix
Comment on attachment 8880066 [details]
Bug 1374837 - Add some additional logging in case the Browser Toolbox fails to open;

https://reviewboard.mozilla.org/r/151412/#review156362
Attachment #8880066 - Flags: review?(jryans) → review+
Also, can you add a |debug(`srcHandle: ${srcHandle}`)| call somewhere above that failure?
(In reply to Kris Maglione [:kmag] from comment #5)
> Interesting... Is this a 32 bit or 64 bit build?

It's a 32 bit build on a 64 bit machine (built using the start-shell-msvc2015 shell from mozilla-build)
(In reply to Brian Grinstead [:bgrins] from comment #10)
> (In reply to Kris Maglione [:kmag] from comment #5)
> > Interesting... Is this a 32 bit or 64 bit build?
> 
> It's a 32 bit build on a 64 bit machine (built using the
> start-shell-msvc2015 shell from mozilla-build)

Can you try to reproduce it with a 64 bit build on Windows 7? I'll also try to reproduce it with a 32 bit build on Windows 10.
So before the condition here https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/subprocess/subprocess_worker_win.js#383:

`srcHandle` == ctypes.voidptr_t(ctypes.UInt64("0x13"))

Then it goes into the `else` and we end up with:

`ok` == false
`their_pipes[2]` == false
`handle.address()` == ctypes.voidptr_t.ptr(ctypes.UInt64("0xeab2810"))
Summary: Browser Toolbox still not working in Windows 7 → Browser Toolbox still not working in Windows 7 in a local build
DevTools bug triage (filter on CLIMBING SHOES).
Priority: -- → P2
I'm affected by this bug as well. As a workaround, I had to download a Nightly build, but obviously I couldn't use the Toolbox to test my changes, just the initial state.
Does this affect our beta builds as well, or just local builds?  Do we know why nightly builds aren't affected?
(In reply to Kris Maglione [:kmag] from comment #11)
> (In reply to Brian Grinstead [:bgrins] from comment #10)
> > (In reply to Kris Maglione [:kmag] from comment #5)
> > > Interesting... Is this a 32 bit or 64 bit build?
> > 
> > It's a 32 bit build on a 64 bit machine (built using the
> > start-shell-msvc2015 shell from mozilla-build)
> 
> Can you try to reproduce it with a 64 bit build on Windows 7? I'll also try
> to reproduce it with a 32 bit build on Windows 10.

I tried doing a build with start-shell-msvc2015-x64 and the BT still didn't work, but I'm not sure that it's actually a 64 bit build (as the objdir is still named obj-i686-pc-mingw32).  Did you have any luck reproducing a 32 bit build?
Flags: needinfo?(kmaglione+bmo)
(In reply to :Paolo Amadini from comment #14)
> I'm affected by this bug as well. As a workaround, I had to download a
> Nightly build, but obviously I couldn't use the Toolbox to test my changes,
> just the initial state.

Sorry about that - what version of Windows are you using? Also, is it a 32 or 64 bit machine, and a 32 or 64 bit build?
Flags: needinfo?(paolo.mozmail)
Recent regression, tracking for 55/56. 
Kris, are you going to take this on, or can you help find someone to own the bug? Thanks.
tracking-firefox55: ? → +
tracking-firefox56: --- → +
(In reply to Brian Grinstead [:bgrins] from comment #16)
> I tried doing a build with start-shell-msvc2015-x64 and the BT still didn't
> work, but I'm not sure that it's actually a 64 bit build (as the objdir is
> still named obj-i686-pc-mingw32).

That will still give you a 32-bit build. You need to add something like this to your mozconfig to get 64bit:

ac_add_options --target=x86_64-pc-mingw32
ac_add_options --host=x86_64-pc-mingw32

> Did you have any luck reproducing a 32 bit build?

Not yet, but I'm looking into it more today.
Flags: needinfo?(kmaglione+bmo)
I haven't been able to reproduce this on 32bit Windows 10, so my best guess is that you have a non-inheritable stderr handle, or something along those lines.

I suppose we may as well just print a warning and ignore the failure, in that case.
(In reply to Kris Maglione [:kmag] from comment #19)
> (In reply to Brian Grinstead [:bgrins] from comment #16)
> > I tried doing a build with start-shell-msvc2015-x64 and the BT still didn't
> > work, but I'm not sure that it's actually a 64 bit build (as the objdir is
> > still named obj-i686-pc-mingw32).
> 
> That will still give you a 32-bit build. You need to add something like this
> to your mozconfig to get 64bit:
> 
> ac_add_options --target=x86_64-pc-mingw32
> ac_add_options --host=x86_64-pc-mingw32

OK, I've done a 64 bit build and am still getting the failure
(In reply to Brian Grinstead [:bgrins] from comment #17)
> Sorry about that - what version of Windows are you using? Also, is it a 32
> or 64 bit machine, and a 32 or 64 bit build?

It's a 32-bit artifact build on Windows 7 Home Premium Service Pack 1 64-bit.

Also, running with -console as suggested in bug 1371548 did not help.
Flags: needinfo?(paolo.mozmail)
Looking back at this, I don't think release management needs to track the bug - it wouldn't block a release, and the dev tools team has already triaged it and has it on their radar.
tracking-firefox55: + → -
tracking-firefox56: + → -
(In reply to Kris Maglione [:kmag] from comment #20)
> I haven't been able to reproduce this on 32bit Windows 10, so my best guess
> is that you have a non-inheritable stderr handle, or something along those
> lines.
> 
> I suppose we may as well just print a warning and ignore the failure, in
> that case.

Would you be able to make a patch that does this so we can check to see if this fixes Paolo's issue?
Flags: needinfo?(kmaglione+bmo)
I have the same issue. Opening the Browser Toolbox does nothing for me, and nothing shows up in the Browser Console. 

This happens randomly and is fixable by using a new profile. But once it happens the profile will never load the Browser Toolbox anymore. 

I'm on Windows 10 on 56.0a1 (2017-07-27) (64-bit). This just happened with the official Nightly build, but also has happened many times before on my local builds.
status-firefox55: affected → wontfix
status-firefox57: --- → affected
Target Milestone: Firefox 55 → ---
Attached patch subprocess-win-fix.patch (obsolete) — — Splinter Review 
This seems to fix it locally for me.  Want to confirm with Jared and Paolo - does this patch fix the Browser Toolbox for you?
Flags: needinfo?(paolo.mozmail)
Flags: needinfo?(jaws)
This problem is intermittent for me. When it starts happening again I'll try out the patch here. Thanks!
This patch didn't fix the issue for me unfortunately.
Flags: needinfo?(jaws)
Summary: Browser Toolbox still not working in Windows 7 in a local build → Browser Toolbox still not working in Windows 7 or Windows 10 in a local build
This patch _does_ fix the issue for me!
Flags: needinfo?(paolo.mozmail) → needinfo?(bgrinstead)
Comment on attachment 8901375 [details] [diff] [review]
subprocess-win-fix.patch

Is something like this (+ a warning) similar to what you meant in Comment 20? I'm not sure what the implications of removing the call to libc.GetStdHandle are here, but it does seem to fix the issue, at least for for Paolo and I
Flags: needinfo?(bgrinstead)
Attachment #8901375 - Flags: feedback?(kmaglione+bmo)
Comment on attachment 8901375 [details] [diff] [review]
subprocess-win-fix.patch

Review of attachment 8901375 [details] [diff] [review]:
-----------------------------------------------------------------

That's interesting. It would be good to have more details about what's causing this failure. If we need to go this route, we should just add `stderr: "stdout"` to the Subprocess call for the browser toolbox, but I'd rather we find a way to detect the problem and add a sensible fallback.
Attachment #8901375 - Flags: feedback?(kmaglione+bmo) → feedback-
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #31)
> Comment on attachment 8901375 [details] [diff] [review]
> subprocess-win-fix.patch
> 
> Review of attachment 8901375 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> That's interesting. It would be good to have more details about what's
> causing this failure. If we need to go this route, we should just add
> `stderr: "stdout"` to the Subprocess call for the browser toolbox, but I'd
> rather we find a way to detect the problem and add a sensible fallback.

I don't have time right now to debug this further but I'd **really** like to get a fix in for our developers who are being affected by this. If you are OK with it I will file a new bug for debugging further and land a workaround patch.
Comment on attachment 8906710 [details]
Bug 1374837 - Pipe stderr to stdout to fix the Browser Toolbox in certain Windows builds;

https://reviewboard.mozilla.org/r/178444/#review183410
Attachment #8906710 - Flags: review?(kmaglione+bmo) → review+
Attachment #8901375 - Attachment is obsolete: true
See Also: → 1398901
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Flags: needinfo?(kmaglione+bmo)
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6616cbb6460f
Add some additional logging in case the Browser Toolbox fails to open;r=jryans
https://hg.mozilla.org/integration/autoland/rev/1bc15f7fe1af
Pipe stderr to stdout to fix the Browser Toolbox in certain Windows builds;r=kmag
Too late for 56. Mark 56 won't fix.
status-firefox56: affected → wontfix
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.