classification
Title: Pass MS CRT debug flags into subprocesses
Type: behavior Stage:
Components: Tests Versions: Python 3.4, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: asvetlov, catalin.iacob, ishimoto, jnoller, kristjan.jonsson, loewis, ocean-city, steve.dower, tim.golden, zach.ware
Priority: normal Keywords:

Created on 2009-03-31 04:13 by loewis, last changed 2014-06-30 21:50 by BreamoreBoy.

Files
File name Uploaded Description Edit
_multiprocessing.patch.zip asvetlov, 2009-03-31 21:19 patch for suppressing multiprocessing unittest run on Windows
Messages (17)
msg84750 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-03-31 04:13
To avoid bringing up CRT assert message dialogs, the CRT debug flags 
need to be passed into subprocesses for multiprocessing.

CRT doesn't have getters. Instead, you have to set to 0, get the current 
value, then restore it. This can be done with

modes = []
for m in [msvcrt.CRT_WARN, msvcrt.CRT_ERROR, msvcrt.CRT_ASSERT]:
    oldmode = msvcrt.CrtSetReportMode(m, 0)
    msvcrt.CrtSetReportMode(m, oldmode)
    modes.append((m, oldmode))

The same probably needs to be done for CrtSetReportFile, except
that msvcrt.CrtSetReportFile currently doesn't return the previous
value. (Also, it returns a HFILE - hopefully, the file handle value
will still be valid in the subprocess)
msg84913 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2009-03-31 21:19
Patch for multiprocessing added.
Using with regrtest -n option now allows to pass all regression test 
stack without any popup assertion dialog on Windows box.

Probably have to be backported to Python 2.7 branch
msg84940 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2009-03-31 22:45
Committed to py3k, 26 maint, trunk, 3k maint r70908
msg85024 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2009-04-01 15:50
R70908 doesn't compile with VS 2008:
22>..\Modules\_multiprocessing\win32_functions.c(135) : warning 
C4013: '_CrtSetReportMode' undefined; assuming extern returning int
22>..\Modules\_multiprocessing\win32_functions.c(135) : error 
C2065: '_CRT_ASSERT' : undeclared identifier
22>..\Modules\_multiprocessing\win32_functions.c(135) : error 
C2065: '_CRTDBG_MODE_DEBUG' : undeclared identifier

Further, this should not be required, you are probably shooting the 
messenger here! We should be finding the cause of the assert, not 
silencing it.  (There are no CRT debug flags, they have been disabled 
recently, and we want everything to work cleanly)
msg85027 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2009-04-01 15:59
Note Hiro added r70953 as well to wrap the include in an ifdef
msg85029 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2009-04-01 16:04
To clarify, the compilation problem is in the trunk.
There appears to be no problem with test_multiprocessing in the trunk 
without these lines, though.
I also think that I found the original reason for you adding those 
lines in 3.1, and that was I fixed in R70843.  Indeed, 
test_multiprocessing runs through for python_d.exe in py3k
I suggest that with the issue fixed, we remove those lines again, since 
they go against the spirit of defect http://bugs.python.org/issue4804
msg85031 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2009-04-01 16:11
I'll reopen, but I am going to defer to Martin on this
msg85036 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-04-01 16:20
We absolutely must turn off GUI notifications if this runs on the 
buildbot. If this means to turn off GUI notications for non-buildbot usage 
also, so be it.
msg85050 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2009-04-01 16:54
But what is so special about the win32_ExitProcess() function, the 
changes apply only for the actual ExitProcess() call (not the duration 
of the rest of the forked job.

That looks like a forced fix to a specific problem, not a generic 
buildbot-friendly feature.

SetErrorMode(), btw, is inherited by child processes:
"Each process has an associated error mode that indicates to the system 
how the application is going to respond to serious errors. A child 
process inherits the error mode of its parent process. To retrieve the 
process error mode, use the GetErrorMode function."

For _CrtSetReportMode, don't we want to channel the output to stderr, 
rather than the debugger output?  No one has access to the debugger 
output of a child process normally.
You can get the handle of stdout with GetStdHandle(STD_ERROR_HANDLE)

Also, let's not forget, that parameter validation will throw up 
dialogue boxes in a release mode too() (in DEBUG an assert is thrown in 
to the mix as well, which is what you are silencing)

If we are doing this for the benefit of buildbot, then why don't we do 
it in python.exe?  We could add an --unattended flag or something 
similar which would silence or redirect any interactive notifications 
we can think of.
msg85053 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2009-04-01 17:21
Problem in _multiprocessing is:
FILE* fp = _fdopen(...);
int fd = _fileno(fp);
_close(fd);
_fclose(fp); // raises assertion

At process exit system tries to close all opened FILE* by _fcloseall,
but file closed by descriptor _close has invalid state and _fclose
raises assertion.
Real problem not in win32_ExitProcess function but in outer
finalization code. At exit time is impossible to point on real source
of problem.

This patch is just supressing error messages on exit of child process
(we sure what we don't want to see anything related to this one, at
least in buildbot test session). For particular problem in
multiprocessing only (all other test passed with regrtest -n without
assertion message boxes).

In general maybe will helpful to add os.close check for opened FILE*
for this descriptor and report about this problem (interesting
question, it should be exception or stderr output?). Of course it have
to be applied only to debug build.
msg85058 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-04-01 17:47
> I suggest that with the issue fixed, we remove those lines again, since 
> they go against the spirit of defect http://bugs.python.org/issue4804

I'm opposed, for the very same reasons I stated over and over again.
msg85059 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-04-01 17:53
> But what is so special about the win32_ExitProcess() function, the 
> changes apply only for the actual ExitProcess() call (not the duration 
> of the rest of the forked job.

Two things: a) it's not really likely that it uncovers an application
bug that people would run into, and b) there is no other way to suppress
the problem.

> That looks like a forced fix to a specific problem, not a generic 
> buildbot-friendly feature.

In case such a bug comes up again, it is important that no popup
will trigger again

> SetErrorMode(), btw, is inherited by child processes:

Correct. With -n restored to test.bat, that can be taken out.

> For _CrtSetReportMode, don't we want to channel the output to stderr, 
> rather than the debugger output?

That would work as well (and perhaps better)

> Also, let's not forget, that parameter validation will throw up 
> dialogue boxes in a release mode too() (in DEBUG an assert is thrown in 
> to the mix as well, which is what you are silencing)

Yes - because I know you want these error boxes, we only added it
in debug mode (because that's what the buildbots use)

> If we are doing this for the benefit of buildbot, then why don't we do 
> it in python.exe?  We could add an --unattended flag or something 
> similar which would silence or redirect any interactive notifications 
> we can think of.

There is the -n flag for regrtest already. Unfortunately, there is
no easy way to pass it to the subprocess. I'm opposed to a new command
line flag, because there are already so many of them.

I would hope that multiprocessing could pass the CRT flags to
subprocesses (see the subject of this report). Somebody would
have to implement it, though.
msg85061 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-04-01 17:55
> Problem in _multiprocessing is:
> FILE* fp = _fdopen(...);
> int fd = _fileno(fp);
> _close(fd);
> _fclose(fp); // raises assertion

Actually, Kristjan fixed the *real* bug, which was more like

FILE* f2=fdopen(fileno(f1));
fclose(f1);
fclose(f2);
msg85188 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2009-04-02 09:46
Correction, Martin, just to be pedantic:
FILE * fp = _fdopen(fh)
_close(fh)
... leaving an automatic fclose(fp) at process exit to fail.

Look, I understand that buildbot can't put up dialogue boxes.  And as 
frustrating as it may be, it appears that Windows can't cause dialogue 
box functions (such as MessageBox()) to simply 'fail' when run 
unattended, e.g. as a Service.  In fact, if the service isn't allowed 
to interact with the desktop, these functions just hang.  So, we need 
to disable them.

But what I'm trying to say, in the case of subprocess, is that this 
disablement should be done _early_ then.  Not as a last resort before 
exit.  Turning them on before exit caught this particular problem, 
which needed fixing anyway, but it won't cause other strange things 
that might happen _during_ the executionof the subprocess code.

Since you object to a command line parameter, how about an environment 
variable? PYTHONUNATTENDED could be used in the library startup to turn 
off any conceivable dialogue boxes and would be passed on to child 
processes.  This could then also easily be added in the build slave 
config script:  sys.env[PYTHONUNATTENDED] = "1"
msg85200 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2009-04-02 13:23
There is another issue for py3k:
test_sys has a test, test_recursionlimit_fatalerror(), but PyFatalError
() will invoke a DebugBreak() in DEBUG mode, causing another dialogue 
popup.  What do do here?
Adding 
    import msvcrt
    msvcrt.SetErrorMode(msvcrt.SEM_FAILCRITICALERRORS)
to the executed code doesn't seem to help.  Any ideas?
msg85201 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-04-02 14:03
> Since you object to a command line parameter, how about an environment 
> variable? PYTHONUNATTENDED could be used in the library startup to turn 
> off any conceivable dialogue boxes and would be passed on to child 
> processes.  This could then also easily be added in the build slave 
> config script:  sys.env[PYTHONUNATTENDED] = "1"

That would work for me. I suppose test.bat would set this variable,
right?
msg139709 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2011-07-03 21:35
Related issues with patches: #9116 and #11732
History
Date User Action Args
2014-06-30 21:50:19BreamoreBoysetnosy: + tim.golden, zach.ware, steve.dower

type: behavior
versions: + Python 3.4, Python 3.5, - Python 3.2, Python 3.3
2014-05-13 21:50:30skrahsetnosy: - skrah
2012-07-24 08:46:59ishimotosetnosy: + ishimoto
2011-11-27 17:41:56catalin.iacobsetnosy: + catalin.iacob
2011-07-03 21:35:28skrahsetnosy: + skrah
messages: + msg139709
2011-06-26 19:29:30terry.reedysetversions: + Python 3.2, Python 3.3, - Python 3.1
2010-09-10 11:27:49ocean-citylinkissue9116 dependencies
2009-04-02 14:03:31loewissetmessages: + msg85201
2009-04-02 13:23:04kristjan.jonssonsetmessages: + msg85200
2009-04-02 09:46:10kristjan.jonssonsetmessages: + msg85188
2009-04-01 17:55:45loewissetmessages: + msg85061
2009-04-01 17:53:33loewissetmessages: + msg85059
2009-04-01 17:47:03loewissetmessages: + msg85058
2009-04-01 17:21:05asvetlovsetmessages: + msg85053
2009-04-01 16:54:46kristjan.jonssonsetmessages: + msg85050
2009-04-01 16:20:48loewissetmessages: + msg85036
2009-04-01 16:11:16jnollersetstatus: closed -> open
resolution: fixed ->
messages: + msg85031
2009-04-01 16:04:49kristjan.jonssonsetmessages: + msg85029
2009-04-01 15:59:29jnollersetmessages: + msg85027
2009-04-01 15:57:27jnollersetnosy: + ocean-city
2009-04-01 15:50:41kristjan.jonssonsetnosy: + kristjan.jonsson
messages: + msg85024
2009-03-31 22:45:00jnollersetstatus: open -> closed
resolution: fixed
messages: + msg84940
2009-03-31 21:19:34asvetlovsetfiles: + _multiprocessing.patch.zip

messages: + msg84913
components: + Tests
versions: + Python 3.1, Python 2.7
2009-03-31 04:13:01loewiscreate