Closed
Bug 1343582
Opened 8 years ago
Closed 8 years ago
On Win8 and higher, task desktop is not assigned to created process
Categories
(Taskcluster :: Workers, defect)
Taskcluster
Workers
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: pmoore, Unassigned)
References
Details
In https://github.com/taskcluster/runlib/blob/1dd020ead28b5c71a569d88535940d5d9a1e44e5/subprocess/subprocess_windows.go#L142 the desktop is not set in the StartupInfo if CreateProcessWithLogonW is used.
In other words, if a subprocess is created with a custom desktop, it is not used in the CreateProcessWithLogonW call, but instead the desktop of the calling process is used.
I've spoken to the original author of the package, who has advised me that we he last touched it, around 5 years ago, CreateProcessWithLogonW was failing on any value of desktop that was not NULL.
There are a couple of alternative solutions:
1) See if it works now on the Windows versions we are using, and write some tests for the CI to cover it, to be sure
2) Use same syscalls as we do in Win7
Note, the reason 2) wasn't implemented in the original library, was that 1) requires fewer user privileges to operate. Probably this is not a constraint for us, so 2) might be the quicker/easier solution, which would have an additional advantage of less code paths to maintain.
This is the *probable* cause of bug 1343580, so marking this new bug as blocking it.
Reporter | ||
Comment 1•8 years ago
|
||
> To use CreateProcessAsUser, you need to adjust privileges of the calling user.
> CreateProcessWithLogonW talks to runas service, which holds these privileges already.
> In my contest setup I prefer to use WithLogon to have fewer moving parts.
> Until Windows 8, it wasn't possible to put processes created by runas service in a job,
> that's why all the ifs around noJob change the behavior.
Comment 2•8 years ago
|
||
based on this comment, it might seem like going the route for #2 will unblock windows 10. https://bugzilla.mozilla.org/show_bug.cgi?id=1343580#c3
Reporter | ||
Comment 3•8 years ago
|
||
(In reply to Greg Arndt [:garndt] from comment #2)
> based on this comment, it might seem like going the route for #2 will
> unblock windows 10. https://bugzilla.mozilla.org/show_bug.cgi?id=1343580#c3
There are other considerations: both routes would potentially unblock, if 1) works it is probably the preferred option due to its simplicity and being a more standard approach recommended by Microsoft. 2) will no doubt work, but may introduce similar performance problems in Win 10 that we saw on Win 7.
Therefore, I'm first trying option 1 to see if it works at all:
https://github.com/taskcluster/runlib/commit/d5b357573f26f1ed9f2523367533eaea838b78a2
I'll see if I can run some tests with this change (and even add a unit test if possible that opens a window and takes a screenshot, then tests that it isn't just a black screen) before making a release for running on the win10 beta worker type.
Reporter | ||
Comment 4•8 years ago
|
||
I've gone ahead and made a release with this, as that will make it easier for testing - I'll initially roll it out to the win10 beta worker type and run a try push to see if that fixes the black screens.
https://github.com/taskcluster/generic-worker/releases/tag/v8.3.0
Reporter | ||
Comment 5•8 years ago
|
||
This fixed the issue.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•6 years ago
|
Component: Generic-Worker → Workers
You need to log in
before you can comment on or make changes to this bug.
Description
•