Fix error in 3.12 during exception handling

[Thrameos]

While trying to examine user reported issue #1178, I found that exception handling was getting caught on an unhandled exception. The root source of the issue it that if we don't set a traceback then it produced NULL to the PyExeception_SetTraceback. The exception was meaningless and (mostly) harmless, but due to other changes in Python it was not cleared by the PyErr_SetString method and thus remained set through to a PyObject_IsInstance call which then through a SystemError.

To fix this error I added an "if" statement to avoid setting the traceback if there is nothing meaningful to set. We should technically be checking every single return value from Python CAPI calls, rather than blindly plowing forward. But that would be a huge modification.

The underlying Python code that triggered the exception is odd....

static int
BaseException_set_tb(PyBaseExceptionObject *self, PyObject *tb)
{
    if (tb == NULL) {
        PyErr_SetString(PyExc_TypeError, "__traceback__ may not be deleted");
        return -1;
    }
    else if (!(tb == Py_None || PyTraceBack_Check(tb))) {
        PyErr_SetString(PyExc_TypeError,
                        "__traceback__ must be a traceback or None");
        return -1;
    }

    Py_XINCREF(tb);
    Py_XDECREF(self->traceback);
    self->traceback = tb;
    return 0;
}

If tb is NULL then it is an invalid input, but the message "may not be deleted" doesn't really tell the user anything about what has gone wrong.

It is unclear if this is the same problem the user was reporting as I wasn't able to replicate the exact issue reported. Though given it was down the same code path I suspect it will be addressed.

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 87.44%. Comparing base (904fc43) to head (faaaca4).

Files	Patch %	Lines
native/common/jp_exception.cpp	50.00%	0 Missing and 1 partial ⚠️
native/python/pyjp_object.cpp	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1180      +/-   ##
==========================================
- Coverage   87.45%   87.44%   -0.01%     
==========================================
  Files         113      113              
  Lines       10236    10238       +2     
  Branches     4057     4059       +2     
==========================================
+ Hits         8952     8953       +1     
+ Misses        692      691       -1     
- Partials      592      594       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Christopher-Chianelli]

I tried this commit with

package org.acme;

public class MyThrowable extends RuntimeException {

    public MyThrowable(String message) {
        super(message);
    }

    public MyThrowable(String message, Throwable cause) {
        super(message, cause);
    }

    @Override
    public Throwable fillInStackTrace() {
        return this;
    }
}

package org.acme;

public class MyClass {
    public static void throwWithCause() throws Throwable {
        throw new MyThrowable("error");
    }
}

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <groupId>org.acme</groupId>
  <artifactId>issue-reproducer</artifactId>
  <name>Issue Reproducer</name>
  <version>1.0.0-SNAPSHOT</version>

  <properties>
    <maven.compiler.release>17</maven.compiler.release>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
  </properties>

  <build>
    <plugins>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.13.0</version>
      </plugin>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-dependency-plugin</artifactId>
        <version>3.6.1</version>
        <executions>
          <execution>
            <id>copy-dependencies</id>
            <goals>
              <goal>copy-dependencies</goal>
            </goals>
          </execution>
        </executions>
      </plugin>
    </plugins>
  </build>
</project>

def test_throwing():
    import jpype
    import jpype.imports
    jpype.startJVM(classpath=[
        'target/issue-reproducer-1.0.0-SNAPSHOT.jar',
    ])
    from org.acme import MyClass
    MyClass.throwWithCause()

and a SystemError is still raised: SystemError: <built-in method __subclasscheck__ of _jpype._JClass object at 0x5650b8a8bbc0> returned a result with an exception set
(also does not fix the exception in #1178, which I am trying to get a minimal reproducer for)

[Thrameos]

Found and corrected the second error point. Mind giving it another try on #1178?

[Christopher-Chianelli]

Can confirm this fixes the issue with MyThrowable. Does not fix the issue with generated classes w/o source infomation sadly.

[Thrameos]

Was the source compiled with debugging information? I think that the stacktrace is taking line numbers from the debug symbols. If it was generated using ASM then I believe you have to add the debug section after the procedure body. I have made dynamic procedures in the past, but I can’t say that I have tried making the debugging symbols.

[Christopher-Chianelli]

It generated with ASM, but the generated ASM does not have debug symbols. I did test adding the debug symbols via ASM (via visitSourceFile and visitLineNumber) and verified that prevented the crash (however, debug symbols are not strictly required in generated classes; some generated classes genuinely do not have a source). It compiled with the default parameters, so line numbers + source file (https://docs.oracle.com/en/java/javase/17/docs/specs/man/javac.html#standard-options, see -g).

[marscher]

Thanks!