py::print doesn't work while a PyErr is set

[jagerman]

@dean0x7d
py::print fails when a python error is currently set; I eventually tracked it down to these lines in simple_collector/unpacking_collector:

        auto result = object(PyObject_CallObject(ptr, m_args.ptr()), false);
        if (!result)
            throw error_already_set();

If PyErr is currently set, the call fails, and py::print throws an exception. This is particular a problem if py::print is used in a destructor because the destructor may well have been invoked as part of stack unwinding because of a previous throw error_already_set().

I ran into this just now while updating PR #264, where exactly this happened: one of the tests was to test that an exception happened, but during the exception, ConstructorStats::print_destroyed is invoked, which now uses py::print.

I put in this workaround to temporarily work around the issue, but I think something similar should be moved up into py::print.

[wjakob]

I think this is a very reasonable workaround. Something similar is already done here: https://github.com/pybind/pybind11/blob/master/include/pybind11/cast.h#L115.

Probably the best thing would be to add PyErr_Fetch / PyErr_Restore to the py::print code itself, rather than the workaround in print_destroyed.

Thoughts?

[jagerman]

Probably the best thing would be to add PyErr_Fetch / PyErr_Restore to the py::print code itself, rather than the workaround in print_destroyed.

Yes, I agree.

Also, an RIAA-type struct might be nice for this and the one in cast.h:

class error_scope {
    PyObject *type, *value, *trace;
    error_scope() { PyErr_Fetch(&type, &value, &traceback); }
    ~error_scope() { PyErr_Restore(type, value, traceback); }
};

[wjakob]

I think you meant RAII :) -- pushed a commit for both cases.

[dean0x7d]

There is actually a more serious underlying issue here: any code calling the Python C API from a C++ destructor is potentially unsafe because of the way error_already_set works. It's not just py::print. Guarding every single C API call with an error_scope is not viable, but error_already_set can be tweaked a little. I'll prepare a test case and a proposed solution soon.

[jagerman]

I think you meant RAII :)

Oh god yes, let's keep the RIAA out of pybind!