tada / pljava

PL/Java is a free add-on module that brings Java™ Stored Procedures, Triggers, Functions, Aggregates, Operators, Types, etc., to the PostgreSQL™ backend.
http://tada.github.io/pljava/
Other
238 stars 77 forks source link

PL/Java should not refuse to do further classloading once an exception has been thrown #422

Closed mikehearn closed 3 months ago

mikehearn commented 1 year ago

I see a few confused people in the issues db asking why they get spurious NoClassDefFoundError exceptions. I just tracked this down and it feels like a nasty issue that could bite a lot of people and be hard to debug.

Once an exception has occurred, PL/Java will refuse to do any more work with an error like An attempt was made to call a PostgreSQL backend function after an elog(ERROR) had been issued, thrown from native code. Unfortunately this code path is reachable via the following path:

java.sql.SQLException: An attempt was made to call a PostgreSQL backend function after an elog(ERROR) had been issued
    at org.postgresql.pljava.internal@1.6.4/org.postgresql.pljava.internal.Oid._forSqlType(Native Method)
    at org.postgresql.pljava.internal@1.6.4/org.postgresql.pljava.internal.Oid.lambda$forSqlType$1(Oid.java:68)
    at org.postgresql.pljava.internal@1.6.4/org.postgresql.pljava.internal.Backend.doInPG(Backend.java:89)
    at org.postgresql.pljava.internal@1.6.4/org.postgresql.pljava.internal.Oid.forSqlType(Oid.java:68)
    at org.postgresql.pljava.internal@1.6.4/org.postgresql.pljava.jdbc.SPIPreparedStatement.setObject(SPIPreparedStatement.java:238)
    at org.postgresql.pljava.internal@1.6.4/org.postgresql.pljava.jdbc.SPIPreparedStatement.setObject(SPIPreparedStatement.java:223)
    at org.postgresql.pljava.internal@1.6.4/org.postgresql.pljava.jdbc.SPIPreparedStatement.setInt(SPIPreparedStatement.java:124)
    at org.postgresql.pljava.internal@1.6.4/org.postgresql.pljava.sqlj.Loader.lambda$findClass$5(Loader.java:462)
    at org.postgresql.pljava.internal@1.6.4/org.postgresql.pljava.sqlj.Loader.findClass(Loader.java:464)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:588)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)

Oops - class loading triggers queries which triggers this check. It means that the following construct will yield a horribly confusing error:

try {
   doSomething();
} finally {
  SomeOtherClass.cleanUp();
}

You will get an error like:

java.lang.NoClassDefFoundError: SomeOtherClass

Why - well, because when the exception was thrown control passed to the finally block, which then attempted to load the SomeOtherClass helper, but that classloading operation in turn fails with an exception, which is then remembered by the JVM as NoClassDefFound even though it exists on the classpath. What it really means is it doesn't know if the class was found or not.

I'm not sure why PL/Java refuses to do continue after an error has occurred and I didn't see any mention of this in the docs. But it does mean that once something goes wrong, all the code on the unwind path had better be loaded already otherwise you're in for a world of pain!

Suggested fix: naively, I'd say don't refuse to do more work because an error was logged, but presumably there's a reason for it.

jcflack commented 1 year ago

Thanks for the report. I think the situation you describe will arise, not any time "an error has occurred", but when an error has occurred that gets reported with an elog(ERROR) in PostgreSQL. So the doSomething() in your example would have to involve an error reported that way.

The ERROR level in PostgreSQL's elog system has special behavior, and leaves PostgreSQL itself unable to accept any more queries until the current one is fully unwound and the transaction rolled back. So PL/Java is kind of out of options at that point.

That said, the way that PL/Java's exception handling integrates with PostgreSQL's could definitely be improved; there is discussion of that here:

https://github.com/tada/pljava/wiki/Thoughts-on-logging

That's a somewhat ambitious project.

It's also likely that the details of classloading, and the schema for installed jars, will be changing in a later major release, affecting issues like this.

jcflack commented 3 months ago

This report may well have been related to #471, believed resolved in PL/Java 1.6.7.