homepage

regrtest bailed out completely on one of the Windows 7 builders due to a problem with accessing the temporary working directory:
http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/410/steps/test/logs/stdio
It's seems a touch suspicious that test_fork1 just so happened to be the test executed right before test_startfile blew up.
Not marking as a blocker yet, since my initial assumption is that it's a race condition with a particular sequence of tests rather than a problem in Python itself. I'll change my mind if it happens again on the rebuild I just forced.

Well, test_startfile seems to be a common issue on that buildbot.

> Well, test_startfile seems to be a common issue on that buildbot.
I wouldn't really call it common (twice in the last 30 runs).
However I would wager that the PermissionError is being caused by an
unknown (without having Process Monitor running when the error occurs)
process temporarily grabbing a handle to a file/directory within the
tests temporary directory.
This just reinforces the need for the solution in issue15496 to be applied.

I have just completed an upgrade of the Win64 buildbot slave with a
faster hard drive and the issue is now much more pronounced.

It crashed again, despite issue15496 being fixed:
http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/496 

Unfortunately, this is a legitimate failure of the test. The test
(actually the support code) is attempting to remove a directory that
is the current directory of an active process. The test has
documented this issue and attempted to work around it by adding a
sleep().
To fix this test there a few options:
1) increase the sleep timeout
2) change the current working directory before calling os.startfile()
(possible that of the Python interpreter itself)
3) code a "wait for child process" function using ctypes and the
Toolhelp API (Process32First/Next)
4) change os.startfile() to use ShellExecuteEx and use the hProcess
handle as the return value and use that with os.waitpid()
#4 is the most accurate, but does introduce an API change (the
introduction of a return value to os.startfile)
#3 is more work but may be helpful for other tests
#2 will definitely work in this case but does violate the testing sandbox
#1 is the least disruptive, but is really just avoiding the root cause
I would like to see #4 but realize that it may be too late for 3.3.
#3 is not bad either as it has impact only on the test suite. But any
of the above should fix this particular case.

I think the reason that it is only this buildbot which fails is that the other Windows buildbots don't use multiple processes. Therefore they don't use a different dir for each test.
> 4) change os.startfile() to use ShellExecuteEx and use the hProcess
> handle as the return value and use that with os.waitpid()
Would this cause a handle leak if os.waitpid() is not used?

> I think the reason that it is only this buildbot which fails is that the other Windows buildbots don't use multiple processes. Therefore they don't use a different dir for each test.
That might be it. Also the failure possibly only happens when
multiple builds are being run thus slowing down process creation and
termination.
> Would this cause a handle leak if os.waitpid() is not used?
It seems so, yes.
So to expand on #4:
4a) create a new handle type that closes the handles on dealloc
4b) return the process ID instead using GetProcessId() and callers
interested in waiting would then need to use _winapi.OpenProcess() to
convert it to a handle for os.waitpid() or
_winapi.WaitForSingleObject()
4c) add a third optional argument to os.startfile() "mode" that mimics
the mode semantics of the os.spawn*() functions

I think the two "simple and stupid" solutions (#1 and #2) have a certain charm myself :) Especially #1, which is the simplest of all.

However #1 is the reason that is bug exists in the first place. The
designer of the test guessed wrong on the "magic value" for the
timeout. There will never be a correct timeout value as it varies
from machine to machine and from workload to workload on a given
machine. For any value that is picked, there exists a scenario where
it will fail.
#2 is certainly a viable work-around and it appears that other tests
(notably the test for fork/exec) do similar so it wouldn't be
unprecedented
#3 is really only useful if other tests need a "wait for process"
helper on Windows.
#4 really just highlights a deficiency with os.startfile() so I'm fine
with deferring that to a feature request for 3.4.
I'll cook up a patch implementing #2 unless anyone else is feeling ambitious.

Here is the patch implementing option #2

This test is still intermittently failing on the AMD64 Windows7 SP1 buildbot: 
http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/630
Any chance the patch could be committed?

New changeset bc5c5b79b7e1 by Antoine Pitrou in branch '3.2':
Issue #15526: try to fix test_startfile's inability to clean up after itself in time.
http://hg.python.org/cpython/rev/bc5c5b79b7e1
New changeset 1704deb7e6d7 by Antoine Pitrou in branch 'default':
Issue #15526: try to fix test_startfile's inability to clean up after itself in time.
http://hg.python.org/cpython/rev/1704deb7e6d7 

Ok, hopefully there'll no more be failures now.

New changeset 3837d9797c14c13d170256143c841d29645e772a by Victor Stinner in branch '2.7':
bpo-15526: test_startfile changes the cwd (#1537)
https://github.com/python/cpython/commit/3837d9797c14c13d170256143c841d29645e772a

I backported the fix to Python 2.7 to fix bpo-30334.