homepage

_SuppressCoreFiles in test_subprocess is a useful facility for other tests, for instance test_faulthandler (which has its own logic) or test_capi (which actually happily dumps core in one of its subtests).
It could probably be factored out into test.support.

Ah, and the recursion limit tests in test_sys dump core too...

I have included a patch that moves _SuppressCoreFiles into test.support and a test that checks whether resource.RLIMIT_CORE is being properly set and reset. It would be nice if there was a more explicit way to test the creation of core files, however.
The patch also corrects a comment that stated ulimit was being set to (0, 0) when in the code only the soft limit was being set to 0. I'm pretty sure that the later should be done because then we don't run into trouble when we have to restore the hard limit to its original (higher) limit.

Could you post your patch without the '--git' option?
Apparently it doesn't work well with the code review tool.
> It would be nice if there was a more explicit way to test the
> creation of core files, however.
One possibility would be to fork() a child process, and make it call os.abort(). In the parent, call os.waitpid(), and call os.WCOREDUMP() on the status.

Will also be useful for issue #18808 :)

Of course, here is the patch without the '--git' option.

I've added a new test that uses fork() and os.abort(), then asserts os.WCOREDUMP() is false. 
However, this test is currently failing. Is my test incorrect, or is this an issue with SuppressCoreFiles() itself?
If its a problem with the test I'm guessing it might have to do with how os.WCOREDUMP() decides whether a process has dumped its core or not. Is there another way we could go about this test?

> If its a problem with the test I'm guessing it might have to do with how 
> os.WCOREDUMP() decides whether a process has dumped its core or not.
You are right, the status code doesn't seem affected by whether the core file was actually dumped or not:
$ ulimit -c
0
$ python -c "import os; os.abort()"; echo $?
Abandon
134
$ ulimit -c unlimited
$ python -c "import os; os.abort()"; echo $?
Abandon (core dumped)
134
And of course:
>>> os.WCOREDUMP(134)
True
I don't think there's any reliable way to test this: modern Linux kernels can intercept core file generation and run an executable instead (think Ubuntu's apport), so the only thing remaining to do is to just check that the context manager "works", i.e. doesn't raise anything.
(see http://linux.die.net/man/5/core "Piping core dumps to a program")

The test works for me. The only problem is that faulthandler dumps a
stack: it might be better to use subprocess with stderr=devnull).
As for the status code, it looks like a bash bug:
"""
$ cat /tmp/test_core.py
import os
if os.fork() == 0:
 os.abort()
else:
 pid, status = os.waitpid(-1, 0)
 print("%d: %s" % (status, os.WCOREDUMP(status)))
$ python /tmp/test_core.py
134: True
$ ulimit -c 0
$ python /tmp/test_core.py
6: False
$ python -c "print(0x80 | 6)"
134
"""
And it's funny, because bash does detect the coredump generation, compare:
"""
$ python -c "import os; os.abort()"; echo $?
Abandon
134
"""
to
"""
$ python -c "import os; os.abort()"; echo $?
Abandon (core dumped)
134
"""
I had a quick look at the code, and indeed there's a bug: there's a
function rebuilding the exit status from the exit code:
"""
static int
process_exit_status (status)
 WAIT status;
{
 if (WIFSIGNALED (status))
 return (128 + WTERMSIG (status));
 else if (WIFSTOPPED (status) == 0)
 return (WEXITSTATUS (status));
 else
 return (EXECUTION_SUCCESS);
}
"""
Unfortunately, adding 128 sets the coredump flag every time,
regardless of the actual coredump status.
Never thought I'd encounter such a bug in bash :-)

> As for the status code, it looks like a bash bug:
Your script gives different results here (on Ubuntu):
$ ulimit -c
0
$ ./python dumpcore.py 
6: False
$ ulimit -c unlimited
$ ./python dumpcore.py 
6: False
That's because Ubuntu overrides the core file pattern:
$ cat /proc/sys/kernel/core_pattern 
|/usr/share/apport/apport %p %s %c
If I ask for a regular core file, the script works:
$ sudo sh -c "echo core.%p > /proc/sys/kernel/core_pattern"
$ ./python dumpcore.py 
134: True
Which means the test really threatens to be unreliable (other systems may install similar mechanisms by default).

Running through both your scripts on my machine (using Ubuntu) WCOREDUMP is always True (though I'll only be able to find a core file when ulimit is set to unlimited, as expected).
Because there doesn't seem to be a good way to test this, I've cut the test from the patch. Is there anything else that this patch should address?

> Is there anything else that this patch should address?
I hoped Charles-François would answer this one, but no, I don't think there's anything left :)
Next step (for another issue perhaps) is to actually re-use this context manager in the other tests which deliberately crash the interpreter.

New changeset 28a1843c0fc1 by Antoine Pitrou in branch 'default':
Issue #18623: Factor out the _SuppressCoreFiles context manager into test.support.
http://hg.python.org/cpython/rev/28a1843c0fc1 

Patch committed, thanks!

>> Is there anything else that this patch should address?
>
> I hoped Charles-François would answer this one, but no, I don't think there's anything left :)
Sorry, I had completely forgotten this issue.
The only comment I have is this:
class SuppressCoreFiles(object):
The explicit 'object' superclass is old-school ;-)

> The explicit 'object' superclass is old-school ;-)
Ha. Ok, I've fixed it.