Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix backtrace formatting in YPython::PyErrorHandler() #36

Merged
merged 3 commits into from
Mar 31, 2021
Merged

Fix backtrace formatting in YPython::PyErrorHandler() #36

merged 3 commits into from
Mar 31, 2021

Conversation

petrpavlu
Copy link
Contributor

An exception value returned by PyErr_Fetch() can be in certain situations "unnormalized", which means the value does not have to be an instance of the returned exception type. This typically happens when an exception is set from a native library. For instance:

PyObject *w = Py_BuildValue("(i,i)", 123, 456);
PyErr_SetObject(NativeError, w);

The exception value is in this case a tuple object and not an instance of NativeError, until the exception gets normalized.

Code in YPython::PyErrorHandler() passes an obtained exception value directly to traceback.format_exception(). This method however expects that the value is already a valid exception object and fails quickly with an AttributeError when that is not the case. This subsequently results in an unhandled error in YPython::PyErrorHandler() and in a SystemError when other Python/C API functions are invoked.

The problem is fixed in the pull request by calling PyErr_NormalizeException() in YPython::PyErrorHandler() to normalize the value to be an exception object. Additionally, a fix for a leaked reference to the traceback module in the same method is included.

The pull request is for the SLE-15-SP2 branch. (I'm not sure if it is worth targeting any earlier branch.) A separate pull request for merging the changes to master will be made afterwards.

The following is a reproducer for the problem:

Test files:

  • main.rb:

    #!/usr/bin/ruby
    
    require "yast"
    Yast.import "PyTest"
    Yast::PyTest.raise_with_tuple()
    Yast::PyTest.raise_with_exception()
    
  • PyTest.py:

    from yast import Declare
    import native
    
    def inner_dummy():
        pass
    
    @Declare('void')
    def raise_with_tuple():
        inner_dummy()
        native.raise_with_tuple()
    
    @Declare('void')
    def raise_with_exception():
        inner_dummy()
        native.raise_with_exception()
    
  • native/native.c:

    #include "Python.h"
    
    static PyObject *NativeError;
    
    static PyObject *native_raise_with_tuple(PyObject *self, PyObject *args)
    {
        PyObject *w = Py_BuildValue("(i,i)", 123, 456);
        PyErr_SetObject(NativeError, w);
        Py_XDECREF(w);
        return NULL;
    }
    
    static PyObject *native_raise_with_exception(PyObject *self, PyObject *args)
    {
        PyObject *w = Py_BuildValue("(i,i)", 123, 456);
        PyObject *e = PyObject_Call(NativeError, w, NULL);
        PyErr_SetObject(NativeError, e);
        Py_XDECREF(e);
        Py_XDECREF(w);
        return NULL;
    }
    
    static PyMethodDef native_methods[] = {
        {"raise_with_tuple", native_raise_with_tuple, METH_NOARGS,
         "Raise with a tuple value."},
        {"raise_with_exception", native_raise_with_exception, METH_NOARGS,
         "Raise with an exception value."},
        {NULL, NULL, 0, NULL}
    };
    
    static struct PyModuleDef nativemodule = {
        PyModuleDef_HEAD_INIT,
        "native",
        NULL,
        -1,
        native_methods
    };
    
    PyMODINIT_FUNC
    PyInit_native(void)
    {
        PyObject *m = PyModule_Create(&nativemodule);
        if (m == NULL)
            return NULL;
    
        NativeError = PyErr_NewException("native.NativeError", NULL, NULL);
        Py_XINCREF(NativeError);
        if (PyModule_AddObject(m, "NativeError", NativeError) < 0) {
            Py_XDECREF(NativeError);
            Py_CLEAR(NativeError);
            Py_DECREF(m);
            return NULL;
        }
    
        return m;
    }
    
  • native/setup.py:

    from distutils.core import setup, Extension
    module = Extension('native', sources = ['native.c'])
    setup(name = 'NativePackage', ext_modules = [module])
    

Steps to build and run the example:

$ (cd native; python3 setup.py build)
$ mkdir ~/.yast2/modules
$ ln -s "$PWD/PyTest.py" ~/.yast2/modules/PyTest.py
$ PYTHONPATH=$PWD/native/build/lib.linux-x86_64-3.8 ./main.rb

Information logged in /var/log/YaST2/y2log:

2021-02-13 18:39:41 <1> localhost(4177) [ui-component] YUIComponentCreator.cc(createInternal):124 Creating UI component for ""
2021-02-13 18:39:41 <2> localhost(4177) [libycp] pathsearch.cc(find):313 Using special search prefix '/root/.yast2/modules' for '/root/test/PyTest.py'
2021-02-13 18:39:41 <1> localhost(4177) [Y2Python] Y2PythonComponent.cc(Y2PythonComponent):37 Creating Y2PythonComponent
2021-02-13 18:39:41 <2> localhost(4177) [libycp] pathsearch.cc(find):313 Using special search prefix '/root/.yast2/modules' for '/root/test/PyTest.py'
2021-02-13 18:39:41 <1> localhost(4177) [YCPDeclarations] YCPDeclarations.cc(_init):199 YCPDeclarations successfuly initialized!
2021-02-13 18:39:41 <1> localhost(4177) [Y2PythonNamespace] YPythonNamespace.cc(YPythonNamespace):212 YPythonNamespace finish
2021-02-13 18:39:41 <3> localhost(4177) [Y2Python] YPython.cc(callInner):291 PyObject_CallObject(pFunc, pArgs) failed!
2021-02-13 18:39:41 <3> localhost(4177) [Y2Python] YPython.cc(callInner):293 Python error: error type: <class 'native.NativeError'>; error value: (123, 456); error traceback:
2021-02-13 18:39:41 <3> localhost(4177) [Y2Python] YPython.cc(callInner):291 PyObject_CallObject(pFunc, pArgs) failed!
2021-02-13 18:39:41 <3> localhost(4177) [Y2Python] YPython.cc(callInner):293 Python error: error type: <class 'SystemError'>; error value: PyEval_EvalFrameEx returned a result with an error set; error traceback: Traceback (most recent call last):

  File "/usr/lib64/python3.8/traceback.py", line 120, in format_exception
    return list(TracebackException(

  File "/usr/lib64/python3.8/traceback.py", line 479, in __init__
    if (exc_value and exc_value.__cause__ is not None

AttributeError: 'tuple' object has no attribute '__cause__'


The above exception was the direct cause of the following exception:


Traceback (most recent call last):

  File "/root/test/PyTest.py", line 14, in raise_with_exception
    inner_dummy()

SystemError: PyEval_EvalFrameEx returned a result with an error set
2021-02-13 18:39:41 <1> localhost(4177) [Y2Python] YPython.cc(destroy):156 Shutting down embedded Python interpreter.

Fixed behavior:

2021-02-13 18:40:50 <1> localhost(4579) [ui-component] YUIComponentCreator.cc(createInternal):124 Creating UI component for ""
2021-02-13 18:40:50 <2> localhost(4579) [libycp] pathsearch.cc(find):313 Using special search prefix '/root/.yast2/modules' for '/root/test/PyTest.py'
2021-02-13 18:40:50 <1> localhost(4579) [Y2Python] Y2PythonComponent.cc(Y2PythonComponent):37 Creating Y2PythonComponent
2021-02-13 18:40:50 <2> localhost(4579) [libycp] pathsearch.cc(find):313 Using special search prefix '/root/.yast2/modules' for '/root/test/PyTest.py'
2021-02-13 18:40:50 <1> localhost(4579) [YCPDeclarations] YCPDeclarations.cc(_init):199 YCPDeclarations successfuly initialized!
2021-02-13 18:40:50 <1> localhost(4579) [Y2PythonNamespace] YPythonNamespace.cc(YPythonNamespace):212 YPythonNamespace finish
2021-02-13 18:40:50 <3> localhost(4579) [Y2Python] YPython.cc(callInner):291 PyObject_CallObject(pFunc, pArgs) failed!
2021-02-13 18:40:50 <3> localhost(4579) [Y2Python] YPython.cc(callInner):293 Python error: error type: <class 'native.NativeError'>; error value: (123, 456); error traceback: Traceback (most recent call last):

  File "/root/test/PyTest.py", line 10, in raise_with_tuple
    native.raise_with_tuple()

native.NativeError: (123, 456)
2021-02-13 18:40:50 <3> localhost(4579) [Y2Python] YPython.cc(callInner):291 PyObject_CallObject(pFunc, pArgs) failed!
2021-02-13 18:40:50 <3> localhost(4579) [Y2Python] YPython.cc(callInner):293 Python error: error type: <class 'native.NativeError'>; error value: (123, 456); error traceback: Traceback (most recent call last):

  File "/root/test/PyTest.py", line 15, in raise_with_exception
    native.raise_with_exception()

native.NativeError: (123, 456)
2021-02-13 18:40:50 <1> localhost(4579) [Y2Python] YPython.cc(destroy):156 Shutting down embedded Python interpreter.

An exception value returned by PyErr_Fetch() can be in certain
situations "unnormalized", which means the value does not have to be an
instance of the returned exception type. This typically happens when an
exception is set from a native library. For instance:
> PyObject *w = Py_BuildValue("(i,i)", 123, 456);
> PyErr_SetObject(NativeError, w);

The exception value is in this case a tuple object and not an instance
of NativeError, until the exception gets normalized.

Code in YPython::PyErrorHandler() passes an obtained exception value
directly to traceback.format_exception(). This method however expects
that the value is already a valid exception object and fails quickly
with an AttributeError when that is not the case. This subsequently
results in an unhandled error in YPython::PyErrorHandler() and in a
SystemError when other Python/C API functions are invoked.

The patch fixes the problem by calling PyErr_NormalizeException() in
YPython::PyErrorHandler() to normalize the value to be an exception
object.
PyImport_ImportModule("traceback") in YPython::PyErrorHandler() returns
a new reference. Add a call to Py_XDECREF() once the code is done with
the module to avoid leaking its reference.
Copy link
Member

@dmulder dmulder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@noelpower just pushed part of this fix into master. I'll approve this one. @petrpavlu would you please submit the leak fix to master?

@dmulder dmulder merged commit ffb87ad into yast:SLE-15-SP2 Mar 31, 2021
@noelpower
Copy link
Contributor

I didn't know there was a pull request for this, probably we should have taken @petrpavlu 's complete pull (with just the bug number changed to the one I raised) sorry about that Petr

@petrpavlu
Copy link
Contributor Author

No worries, forward merges are at #38 (to SLE15-SP3) and #39 (master).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants