Pass MS CRT debug flags into subprocesses

@loewis

BPO	5619
Nosy	@loewis, @atsuoishimoto, @kristjanvalur, @tjguk, @asvetlov, @zware, @zooba
Files	_multiprocessing.patch.zip: patch for suppressing multiprocessing unittest run on Windows

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2009-03-31.04:13:01.768>
labels = ['type-bug', 'tests']
title = 'Pass MS CRT debug flags into subprocesses'
updated_at = <Date 2014-06-30.21:50:19.674>
user = 'https://github.com/loewis'

bugs.python.org fields:

activity = <Date 2014-06-30.21:50:19.674>
actor = 'BreamoreBoy'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Tests']
creation = <Date 2009-03-31.04:13:01.768>
creator = 'loewis'
dependencies = []
files = ['13526']
hgrepos = []
issue_num = 5619
keywords = []
message_count = 17.0
messages = ['84750', '84913', '84940', '85024', '85027', '85029', '85031', '85036', '85050', '85053', '85058', '85059', '85061', '85188', '85200', '85201', '139709']
nosy_count = 10.0
nosy_names = ['loewis', 'ishimoto', 'kristjan.jonsson', 'ocean-city', 'tim.golden', 'jnoller', 'asvetlov', 'catalin.iacob', 'zach.ware', 'steve.dower']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue5619'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

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)

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

Committed to py3k, 26 maint, trunk, 3k maint r70908

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)

Note Hiro added r70953 as well to wrap the include in an ifdef

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

I'll reopen, but I am going to defer to Martin on this

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.

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.

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.

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.

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.

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);

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"

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?

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?

Related issues with patches: bpo-9116 and bpo-11732

Tests which are expected to crash now use with support.SuppressCrashReport(): ....

Moreover, since 2009, Python got new debug features like faulthandler, timeout in regrtest (implemented with faulthandler), etc.

I close the issue.

Navigation Menu

Repository navigation

Pass MS CRT debug flags into subprocesses #49869

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Pass MS CRT debug flags into subprocesses #49869

Description

Activity

loewis commented on Mar 31, 2009

asvetlov commented on Mar 31, 2009

jnoller commented on Mar 31, 2009

kristjanvalur commented on Apr 1, 2009

jnoller commented on Apr 1, 2009

kristjanvalur commented on Apr 1, 2009

jnoller commented on Apr 1, 2009

loewis commented on Apr 1, 2009

kristjanvalur commented on Apr 1, 2009

asvetlov commented on Apr 1, 2009

loewis commented on Apr 1, 2009

loewis commented on Apr 1, 2009

loewis commented on Apr 1, 2009

kristjanvalur commented on Apr 2, 2009

kristjanvalur commented on Apr 2, 2009

loewis commented on Apr 2, 2009

skrah commented on Jul 3, 2011

vstinner commented on Sep 7, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions