[issue23314] Disabling CRT asserts in debug build
Steve Dower added the comment: I haven't seen any of these in a while, and the buildbot at http://buildbot.python.org/all/builders/AMD64%20Windows8%203.x (the only one running VS 2015 right now AFAIK) hasn't either, so I'm calling this fixed. -- resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23314 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23314] Disabling CRT asserts in debug build
Changes by David Bolen db3l@gmail.com: -- nosy: +db3l ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23314 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23314] Disabling CRT asserts in debug build
STINNER Victor added the comment: 23314_tf_inherit_check.diff: I would prefer to see this complex code in a function of the support module. For example, the SuppressCrashReport class is a good candidate. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23314 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23314] Disabling CRT asserts in debug build
Steve Dower added the comment: You're right, SuppressCrashReport also makes more sense here, though it still needs to be used explicitly in every new process. New patch attached. -- Added file: http://bugs.python.org/file38210/23314_tf_inhert_check_2.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23314 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23314] Disabling CRT asserts in debug build
Steve Dower added the comment: Having run some more tests, it may be that the only regular problem is in the test for inherited file descriptors. I've attached a patch for tf_inherit_check.py that will prevent assert dialogs. It's not pretty, but it avoids touching the interpreter internals. I still prefer the environment variable option (and maybe add an underscore at the start/making it 'officially' undocumented?), as that will help avoid issues when new tests are added, but for now it looks like only this test needs fixing. -- Added file: http://bugs.python.org/file38200/23314_tf_inherit_check.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23314 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23314] Disabling CRT asserts in debug build
STINNER Victor added the comment:
When we completely switch Windows builds over to VC14, we're going to
encounter some new assert dialogs from the CRT. (...) A number of tests attempt
operations on bad file descriptors, which will assert and terminate in MSVCRT
(I have a fix for the termination coming, but the assertions will still
appear).
Can you give some examples of tests which fail with an assertion error?
_PyVerify_fd() doesn't protect use against the assertion error?
I would prefer to keep CRT error checks, to protect us against bugs.
--
Instead of patching faulthandler, you should patch
test.support.SuppressCrashReport.__enter__. This function already calls
SetErrorMode(). Instead of ctypes, the function may be exposed in the _winapi
module for example. I'm talking about SetErrorMode() *and* _CrtSetReportMode().
+#if defined MS_WINDOWS defined _DEBUG
+if ((p = Py_GETENV(PYTHONNOCRTASSERT)) *p != '\0') {
+_CrtSetReportMode(_CRT_ASSERT, 0);
+}
+#endif
The function is not available if _DEBUG is not defined? Why not calling the
function if _DEBUG is not defined?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23314
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23314] Disabling CRT asserts in debug build
Steve Dower added the comment: _Py_verifyfd has to go away, unfortunately. It requires inside knowledge of the exact CRT version, and with VC14 the CRT will automatically upgrade so that we're always using the latest. I've gotten a function added to the CRT to make it unnecessary for release builds (which terminate on invalid fds). However, debug builds will display a message box still, which affects the buildbots. test already suppresses the dialogs for its own process, but when it creates subprocesses they don't inherit that setting. An environment variable is an easy way to make sure that all subprocesses also have assert dialogs disabled. In release builds they are always disabled, hence the _DEBUG check. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23314 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23314] Disabling CRT asserts in debug build
STINNER Victor added the comment: An environment variable is an easy way to make sure that all subprocesses also have assert dialogs disabled. In release builds they are always disabled, hence the _DEBUG check. If _PyVerify_fd() must go, I would prefer to always disable CRT check. Otherwise, os.dup(invalid fd) would open the CRT assertion popup, instead of raising OSError(EBADF). I prefer to have the same behaviour with any version of the CRT and any version of the Visual Studio. You may add an environment variable to explicitly *enable* the check, if you want. Programming with Python is different than programming with C. Python always raise an OSError(EBADF) exception. It's not like the C language, where you have to explicitly check the result of each function call. It's difficult to ignore OSError without notifying it. I don't know other CRT checks. Many other checks make sense for Python too? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23314 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23314] Disabling CRT asserts in debug build
Steve Dower added the comment: To be clearer (while still respecting the confidentiality agreement I'm under), previously this code would (if _DEBUG) display an assertion dialog and (regardless of _DEBUG) terminate the process: close(fd); // succeeds, assuming a good fd close(fd); // crashes here This code would not display the dialog, but would still terminate: _CrtSetReportMode(_CRT_ASSERT, 0); close(fd); close(fd); This code would not display the dialog or terminate, but depends on knowing implementation details of the CRT: if (!_PyVerify_fd(fd)) return posix_error() res = close(fd); if (res 0) return posix_error() if (!_PyVerify_fd(fd)) return posix_error() res = close(fd); if (res 0) return posix_error() Soon we'll have a safe way (including in the presence of threads) to do this: _DONT_TERMINATE_ON_INVALID_PARAMETER_ON_THIS_THREAD res = close(fd); if (res 0) return posix_error() res = close(fd); // would have terminated, but now returns EBADF if (res 0) return posix_error() _ALLOW_TERMINATE_ON_INVALID_PARAMETER_ON_THIS_THREAD In the last case, we prevent termination but can't safely suppress the _DEBUG dialog in the same way. SuppressCrashHandler works for its purpose, since the OS error mode is inherited by child processes. The _CrtSetReportMode setting is *not* inherited, and so it needs to be called in every child process created by tests. Maybe it's possible to add the call into every test, but I'm skeptical (especially considering the multiprocessing tests). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23314 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23314] Disabling CRT asserts in debug build
Steve Dower added the comment: EBADF will still be returned; _PyVerify_fd is only there to prevent the assertion dialogs in debug builds. Release builds will not need _PyVerify_fd at all (though it's public, so it will remain, but it won't be necessary to protect calls into the CRT). As I said, there is a change coming in a CRT update that will make file calls behave more like POSIX, ie. returning EBADF instead of crashing. See #4804 and #3545 for the discussions (and complaints) about turning off all assertion dialogs. Note that this issue only affects the dialogs in debug builds, and that for Python 3.5 we'll be making debug builds more widely available so that people can debug their extensions/hosts (see e.g. #22411). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23314 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23314] Disabling CRT asserts in debug build
Steve Dower added the comment: Attached a patch with options 1 and 2 implemented - both are fairly trivial. Any thoughts/preferences? -- assignee: - steve.dower keywords: +patch Added file: http://bugs.python.org/file38104/23314 Options.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23314 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23314] Disabling CRT asserts in debug build
New submission from Steve Dower: When we completely switch Windows builds over to VC14, we're going to encounter some new assert dialogs from the CRT. These won't appear in release builds, but because the buildbots run tests against debug builds they will get in the way. A number of tests attempt operations on bad file descriptors, which will assert and terminate in MSVCRT (I have a fix for the termination coming, but the assertions will still appear). Currently regrtest has a -n flag to disable assertions using an MSVCRT function. This works fine with the exception of subprocesses, that do not inherit the setting. The setting is process-wide, so we can't just turn assertions off around the code that may assert. We also can't turn them off completely without affecting users (e.g. #3545 and #4804). #4804 has the discussion leading to the current state where _PyVerify_fd and the -n flag above exist. The _PyVerify_fd function could break with future CRT changes (bearing in mind that Python 3.5 and later will automatically use the latest installed CRT), for which there is an official workaround coming soon, except it won't suppress the assertion dialog for debug builds. I'd like to propose three options: 1. Make faulthandler.enable() also disable assertions in debug builds. (Assertions are for debugging, and enabling faulthandler - at least on Windows - seems to suggest that you want to override the debugger. We also already enable faulthandler for subprocesses in the test suite.) 2. Add an environment variable (PYTHONDISABLECRTASSERT?) and use it to disable assertions on startup. (It looks like at least some tests deliberately block envvars flowing through to subprocesses, so this may require test changes too.) 3. Add a new xoption and use it to disable assertions on startup. Obviously we could do all three, though I kind of like the first one best. Currently, I'm planning to make debug builds available alongside the release versions, as they are necessary for people to build debuggable versions of their extensions, so this is more important than it was in the past. But the problem is very much only on the buildbots/in the test suite and not on user's machines. Thoughts? -- components: Windows messages: 234640 nosy: haypo, steve.dower, tim.golden, zach.ware priority: normal severity: normal status: open title: Disabling CRT asserts in debug build type: crash versions: Python 3.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23314 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
