msg84750 - (view) |
Author: Martin v. Löwis (loewis) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2011-07-03 21:35 |
Related issues with patches: #9116 and #11732
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:47 | admin | set | github: 49869 |
2014-06-30 21:50:19 | BreamoreBoy | set | nosy:
+ 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:30 | skrah | set | nosy:
- skrah
|
2012-07-24 08:46:59 | ishimoto | set | nosy:
+ ishimoto
|
2011-11-27 17:41:56 | catalin.iacob | set | nosy:
+ catalin.iacob
|
2011-07-03 21:35:28 | skrah | set | nosy:
+ skrah messages:
+ msg139709
|
2011-06-26 19:29:30 | terry.reedy | set | versions:
+ Python 3.2, Python 3.3, - Python 3.1 |
2010-09-10 11:27:49 | ocean-city | link | issue9116 dependencies |
2009-04-02 14:03:31 | loewis | set | messages:
+ msg85201 |
2009-04-02 13:23:04 | kristjan.jonsson | set | messages:
+ msg85200 |
2009-04-02 09:46:10 | kristjan.jonsson | set | messages:
+ msg85188 |
2009-04-01 17:55:45 | loewis | set | messages:
+ msg85061 |
2009-04-01 17:53:33 | loewis | set | messages:
+ msg85059 |
2009-04-01 17:47:03 | loewis | set | messages:
+ msg85058 |
2009-04-01 17:21:05 | asvetlov | set | messages:
+ msg85053 |
2009-04-01 16:54:46 | kristjan.jonsson | set | messages:
+ msg85050 |
2009-04-01 16:20:48 | loewis | set | messages:
+ msg85036 |
2009-04-01 16:11:16 | jnoller | set | status: closed -> open resolution: fixed -> messages:
+ msg85031
|
2009-04-01 16:04:49 | kristjan.jonsson | set | messages:
+ msg85029 |
2009-04-01 15:59:29 | jnoller | set | messages:
+ msg85027 |
2009-04-01 15:57:27 | jnoller | set | nosy:
+ ocean-city
|
2009-04-01 15:50:41 | kristjan.jonsson | set | nosy:
+ kristjan.jonsson messages:
+ msg85024
|
2009-03-31 22:45:00 | jnoller | set | status: open -> closed resolution: fixed messages:
+ msg84940
|
2009-03-31 21:19:34 | asvetlov | set | files:
+ _multiprocessing.patch.zip
messages:
+ msg84913 components:
+ Tests versions:
+ Python 3.1, Python 2.7 |
2009-03-31 04:13:01 | loewis | create | |