RFE: faulthandler.register() should support file descriptors

[vstinner]

BPO	23566
Nosy	@vstinner, @kilowu
Files	issue23566.patch issue23566_update.patch issue23566_v3.patch issue23566_fd_tests.patch issue23566_fd_tests_v2.patch

^{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 = <Date 2015-04-06.21:34:37.683>
created_at = <Date 2015-03-03.00:00:15.069>
labels = ['easy']
title = 'RFE: faulthandler.register() should support file descriptors'
updated_at = <Date 2015-04-06.21:34:37.682>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2015-04-06.21:34:37.682>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2015-04-06.21:34:37.683>
closer = 'vstinner'
components = []
creation = <Date 2015-03-03.00:00:15.069>
creator = 'vstinner'
dependencies = []
files = ['38417', '38436', '38445', '38480', '38546']
hgrepos = []
issue_num = 23566
keywords = ['patch', 'easy']
message_count = 19.0
messages = ['237091', '237092', '237730', '237749', '237843', '237852', '237879', '237939', '237940', '237941', '238006', '238018', '238019', '238044', '238073', '238189', '238460', '239870', '240187']
nosy_count = 3.0
nosy_names = ['vstinner', 'python-dev', 'kilowu']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue23566'
versions = ['Python 3.5']

[vstinner]

Currently, functions of the faulthandler module require a file-like object (with a fileno() method). It would be nice to accept also file descriptors (int).

Example of code using faulthandler which creates an useless file object just to pass a file descriptor:
https://github.com/nicoddemus/pytest-faulthandler/blob/master/pytest_faulthandler.py#L13

[vstinner]

Only faulthandler_get_fileno() should be modified, but a new unit test should be added.

[kilowu]

Attached a patch to this issue. Made some changes to faulthandler_get_fileno to accept integer as fd. If a fd is provided, the file member of fatal_error struct is set NULL.

An new test case is added and some common testing code is also changed in order to be reused.

[vstinner]

@kilowu: Thanks, I reviewed your patch.

[kilowu]

@Haypo: Thank you for your review. I attached an updated patch addressing the review comments. In addition, I also added a change note in Misc/NEWS.

[vstinner]

issue23566_update.patch looks good to me, but I suggested some minor changes. Usually, I do such changes myself, but I proposed this issue on the Python menthorship list, so I prefer to do the changes to learn the process of reviewing patches and taking comments in account ;-)

[kilowu]

Updated the patch and addressed the previous review comments:

• Fixed the hasattr problem
• Added a default value None for filename in check_dump_traceback
• Reverted unnecessary code change in check_dump_traceback_later
• Added a new paragraph to the enable, dump_traceback_later and
  register in doc

Let me know if there is possible further improvement in this patch.

: )

[python-dev]

New changeset e78de5b1e3ef by Victor Stinner in branch 'default':
Issue bpo-23566: enable(), register(), dump_traceback() and dump_traceback_later()
https://hg.python.org/cpython/rev/e78de5b1e3ef

[vstinner]

Let me know if there is possible further improvement in this patch.

I was going to push your change, but I noticed that you use stderr.fileno() in tests. I modified the test to use a different file descriptor. sys.stderr is also the default value, so the test may pass because the file descriptor was ignored (if there was a bug in your change, which is not the case).

I pushed the modified patch.

Thanks for your contribution Wei Wu!

[vstinner]

Oops, I forgot Windows. On Windows, we should maybe use stderr.fileno() and avoid pass_fds.

(Or maybe just skip the tests on Windows?)

http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/5828/steps/test/logs/stdio

======================================================================
FAIL: test_dump_traceback_fd (test.test_faulthandler.FaultHandlerTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_faulthandler.py", line 378, in test_dump_traceback_fd
    self.check_dump_traceback(fd=fp.fileno())
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_faulthandler.py", line 365, in check_dump_traceback
    trace, exitcode = self.get_output(code, filename, fd)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_faulthandler.py", line 60, in get_output
    process = script_helper.spawn_python('-c', code, pass_fds=pass_fds)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\script_helper.py", line 136, in spawn_python
    **kw)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\subprocess.py", line 855, in __init__
    restore_signals, start_new_session)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\subprocess.py", line 1096, in _execute_child
    assert not pass_fds, "pass_fds not supported on Windows."
AssertionError: pass_fds not supported on Windows.

======================================================================
FAIL: test_enable_fd (test.test_faulthandler.FaultHandlerTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_faulthandler.py", line 235, in test_enable_fd
    fd=fd)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_faulthandler.py", line 106, in check_fatal_error
    output, exitcode = self.get_output(code, filename=filename, fd=fd)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_faulthandler.py", line 60, in get_output
    process = script_helper.spawn_python('-c', code, pass_fds=pass_fds)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\script_helper.py", line 136, in spawn_python
    **kw)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\subprocess.py", line 855, in __init__
    restore_signals, start_new_session)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\subprocess.py", line 1096, in _execute_child
    assert not pass_fds, "pass_fds not supported on Windows."
AssertionError: pass_fds not supported on Windows.

[kilowu]

Or we could reuse the file created by filename in subprocess?

if filename:
    file = open(filename, "wb")
    if use_fd:
        file = file.fileno()
else:
    file = None

In this case, we need to pass two arguments(both filename and a bool use_fd) to check_xxx functions.

[python-dev]

New changeset 211e29335e72 by Victor Stinner in branch 'default':
Issue bpo-23566: Skip "fd" tests of test_faulthandler on Windows
https://hg.python.org/cpython/rev/211e29335e72