pytest: pytest assumes __repr__ is safe on an object with unsuccessful constructor - causing Segmentation Fault

When presenting a failed stack trace, pytest assumes that it is safe to repr an object even if the failure occurred in that object’s constructor. Take the following case:

import ctypes
import pytest

class SegRepr:
    """The most horrific class... raises a SegFault if you try to repr."""
    def __init__(self):
        raise ValueError("You really don't want me.")

    def __repr__(self):
        print("You can't get here, which is lucky because I'm "
              "about to SegFault.")
        import ctypes
        ctypes.string_at(0)


def test_bad():
    # The test case (it fails because of a ValueError being raised).
    assert SegRepr()

In everyday python, if you try to print(SegRepr()) you will get a ValueError because you can’t construct the object (though if you really try, you could do print(SegRepr.__new__(SegRepr))).

However with pytest, we end up calling through to the repr:

pytest seg.py -sv
======================================================================== test session starts ========================================================================
platform darwin -- Python 3.7.2, pytest-4.1.1, py-1.7.0, pluggy-0.8.1 -- /Users/pelson/dev/scitools/cf-units/seg_env/bin/python
cachedir: .pytest_cache
rootdir: /Users/pelson/dev/scitools/cf-units, inifile:
collected 1 item                                                                                                                                                    

seg.py::test_bad You can't get here, which is lucky because I'm about to SegFault.
Segmentation fault: 11

This might sound like an esoteric case, but this is a boiled down version of a case that was genuinely seen in https://github.com/SciTools/cf-units/issues/133#issuecomment-456051865. In my comment on that issue I do state that I think it could be a challenge to address this issue without losing some of the functionality that pytest already provides (like giving a really helpful stack trace with information about the objects at each level). I therefore think this issue should be addressed in one of two ways:

  • Recognise that it was the constructor that failed, and only do a repr if the exception was caused elsewhere (potentially hard)
  • Document the pytest feature that it will call __repr__ on potentially incomplete classes, and that objects that make use of state to call out to external interfaces run the real risk of Segmentation Faulting.

To be honest, I consider this issue as the beginning of addressing bullet-point 2 - hopefully others will see this and be able to recognise why their own libraries are failing with such ugly seg faults.

For my own next course of action, I intend to make my __repr__ more tolerant of incomplete initialisation within the library that this turned up for.

Apologies if this is a duplicate, however I went through each of the issues relating to Seg Faulting and could not find such a simple case that reproduced the issue.

Full environment definition

Using a basic conda environment on OSX Mojave, created with:

$ conda install pip python
$ pip install pytest

Spec:

# Name                    Version                   Build  Channel
atomicwrites              1.2.1                     <pip>
attrs                     18.2.0                    <pip>
ca-certificates           2018.12.5                     0    defaults
certifi                   2018.11.29               py37_0    defaults
libcxx                    4.0.1                hcfea43d_1    defaults
libcxxabi                 4.0.1                hcfea43d_1    defaults
libedit                   3.1.20181209         hb402a30_0    defaults
libffi                    3.2.1                h475c297_4    defaults
more-itertools            5.0.0                     <pip>
ncurses                   6.1                  h0a44026_1    defaults
openssl                   1.1.1a               h1de35cc_0    defaults
pip                       18.1                     py37_0    defaults
pluggy                    0.8.1                     <pip>
py                        1.7.0                     <pip>
pytest                    4.1.1                     <pip>
python                    3.7.2                haf84260_0    defaults
readline                  7.0                  h1de35cc_5    defaults
setuptools                40.6.3                   py37_0    defaults
six                       1.12.0                    <pip>
sqlite                    3.26.0               ha441bb4_0    defaults
tk                        8.6.8                ha441bb4_0    defaults
wheel                     0.32.3                   py37_0    defaults
xz                        5.2.4                h1de35cc_4    defaults
zlib                      1.2.11               h1de35cc_3    defaults

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Comments: 42 (41 by maintainers)

Most upvoted comments

Some more thoughts on this:

  • While there’s ways to trigger this outside of pytest, it’s much easier to make it happen in pytest as it happens with a common pattern (throw an exception in __init__ if some low-level member can’t be initialized).
  • In general, pytest recognizes that pattern, that’s why safe_repr exists. Saying “It’s likely that __repr__ throws an exception when __init__ did throw one, as members aren’t fully initialized.” seems natural! You probably wouldn’t say “if you don’t check all your members with hasattr(...) in your __repr__, it’s a bug in your code”. It’s just a really unlikely case (outside of pytest), so it seems reasonable to not care about it.
  • Whether those members are internally implemented in Python or via some bindings is largely an implementation detail.
  • If we could implement safe_repr() to handle this case (e.g. say “if there is a segfault in __repr__, return an error string instead”) I believe we wouldn’t need to argue that it makes sense to do so.

Even if you consider it an error in the code:

  • The entire job of a test runner is to help dealing with code that’s incorrect in some way. If code tested with pytest would always be correct, we wouldn’t need pytest.
  • Segfaults are a very unhelpful way of saying “something is wrong”. Especially in cases like this, where they actually overshadow the “real” issue.

So IMHO, pytest definitely should handle this better, the question then is if/how it can.

What about just not printing the self argument inside __init__ on exceptions there? You could argue that it’s not helpful anyways, the chance that you see a Python exception there (or data from an object in an invalid state) is very high given that __init__ failed.

the object is constructed (that is, it has been created through __new__, it’s just potentially not initialized.

it’s pretty easy to repr such an object through debugger or (in this case) traceback. the fix should be in the faulty __repr__ implementation

Do you agree that this should be documented in pytest?

I think @pelson’s suggestion is reasonable, in the sense that we might warn that pytest on failure will call repr() on the object, and depending on the library this might cause a segfault. This might seem “obvious”, but at least will appear on a search on the docs and might save someone some time.