Class Loading issue about pljava HOT 31 CLOSED

This is interesting. To be sure, the jars are in PostgreSQL tables, and the classloader needs to call into PostgreSQL to query them, which can only be done on "the PG thread" (a notion whose exact meaning depends on the setting of pljava.java_thread_pg_entry). That can only happen while an upcall from PG into Java is on the stack; if the other thread were to try to load at some other time, it would either fail or block until the next PG into Java upcall. That does not seem to be the issue, because your worker.join() is there to ensure the original PG upcall is still on the stack.

The problem instead seems to be exactly what the exception is saying, that there has been some previous elog(ERROR, ...) Of course, you did not see a PostgreSQL error reported, which suggests some Java code may have caught the resulting ServerException, and neither rethrown it nor done the savepoint rollback that is needed in such a case to reset the errorOccurred flag. If that was in PL/Java's loader it would merit the name 'bug'.

It would be good to know what PostgreSQL error was raised (and why it is only raised in the different-thread case). There should be a DEBUG2 message with the name of the JNI function involved, at the moment a Java exception is constructed from the PostgreSQL error. Even better, a jdb breakpoint could be set on the constructor of ServerException to get the Java stacktrace and the details of the exception.

from pljava.

It's a bug. An unexpected "stack depth limit exceeded" is coming from SPI_cursor_close, because the call is made without the STACK_BASE_VARS fudging that java_thread_pg_entry=allow mode can require. The cursor is closed when a try-with-resources closes a ResultSet, and the exception is swallowed there.

There are probably a few more such bugs. For now, I would try to structure your code so any class loading happens on the thread making the PG to Java upcall, so that your code can run with a java_thread_pg_entry setting of throw or block. The allow setting, with its associated stack-base fudging, has always been kind of inherently dubious.

I intend eventually to introduce a fifth java_thread_pg_entry mode, in which the upcall thread from PG dispatches its work onto another thread immediately, and sticks around as a one-thread executor service handling doInPG blocks from other threads, until the original work completes and it returns. That will be less inherently dubious than the existing allow mode, and less limiting than the existing block, error, and throw modes. But it is still future work. Perhaps you can find a way to structure your code in a similar way.

The java_thread_pg_entry settings are described here.

from pljava.

Thank you very much for detailed response.

I have some additional questions just to make sure I understood correctly:

1. Is "upcall thread from PG" the same as "main" thread on which PL/Java code runs? So class loading should be done only in the main thread?
2. As loading classes beforehand in the main thread and using those classes in the other threads seems to fix this problem, do you see any downsides or caveats of this workaround?

from pljava.

I was careful to say "upcall thread from PG" because "main thread" can be a little too simple, depending on what setting is used for java_thread_pg_entry.

There is, of course, one original thread on which PostgreSQL makes the first call into PL/Java that starts the JVM. We can call that the main thread, or the original main thread, or something like that. We can also call it "the PG thread", at least at first.

Now, there is a construct inside the PL/Java internal code, doInPG(() -> foo()), which is used when foo() has to happen on "the PG thread".

What that construct means depends on the java_thread_pg_entry setting:

Historically, allow mode was the first and only one implemented in PL/Java, and it was necessary because PL/Java used object finalizers (deprecated in Java 9), and those could run on other threads.

The block, throw, and error modes are quite limiting, as you can see, and mainly exist as ways of asserting that the code does not contain any attempts to do PG things from other threads.

The trouble with the allow mode is that the I am now part is pretty hacky, and has to mess with some PostgreSQL internals that try to detect excessive recursion (and that hasn't been done in every possible place, which was the bug that made SPI_cursor_close blow up in your example). Without that bug, your example would have worked.

unimplemented future mode will be less limiting than block, throw, and error and less hacky than allow, and then allow mode can go away (and many lines of hacks along with it). But that mode just isn't ready yet.

So for now, yes, you probably want to try to use throw mode, so you'll detect any time your code tries to do PG stuff on a different thread. And in throw mode, yes, "the PG thread" will always mean that original main thread.

The only downside or caveat I see is that classes are normally loaded dynamically, so it may be a bit of a job for you to work out the full set of classes to load ahead of time, and not be surprised later when one of those wants to load some obscure inner class it uses. I wish I had unimplemented future mode ready for you already, but I just don't.

from pljava.

Thanks a lot. Your detailed explanation is really helpful.

The only downside or caveat I see is that classes are normally loaded dynamically, so it may be a bit of a job for you to work out the full set of classes to load ahead of time, and not be surprised later when one of those wants to load some obscure inner class it uses. I wish I had unimplemented future mode ready for you already, but I just don't.

I agree on that, I ultimately came to the same conclusions.

I'm wondering about another workaround that you might have some thoughts on.
If it was possible to load jars from PL/Java into memory from PG (on the PG thread) and instruct ClassLoader to use those loaded jars instead of ones in the PG tables. Then accessing PG when loading classes won't be needed, so there should not be a non-PG thread access to PG anymore.
Of course I understand that the issue with allow mode is more generic, but I'm just trying to find a solution for the narrowed use case.

from pljava.

Hey, wait a sec. I was a little hasty in analyzing the bug. It has to do with allow mode sure enough, but it's a more superficial bug than I thought.

If you don't mind building from an unreleased branch, you might try a build from the bug/REL1_6_STABLE/issue471 branch and see where that gets you.

For me, your minimal example does get past the class loading now and try to connect to the server. (My test instance doesn't listen on 5432 and I didn't bother altering the example further, but just confirmed it gets that far.) I also had to grant in pljava.policy:

  permission java.util.PropertyPermission "postgresql.url.encoding", "read";
  permission java.util.PropertyPermission "socksProxyHost", "read";
  permission java.net.SocketPermission "127.0.0.1:5432", "connect";

from pljava.

I've build PL/Java from the branch, installed it and successfully run some simple tests with java_thread_pg_entry set to allow.
I will try it with some more extensive usage of Threads and will back to you with the update.

Thank you!

from pljava.

Hello everyone, I really appreciate your software. However, I am having a version of this issue.

I'm trying to integrate a legacy library I can't edit using PL/Java. This poorly implemented library launches a thread by extending Thread with an inner class. I can isolate this thread from interacting with the PG thread but it still doesn't work. This thread can't load its classes because they come from the PG Thread's context class loader and not the system class loader. I can fix this by adding my jar to your classpath, but now I find that you are using modules instead of the classpath (nobody uses modules because they don't integrate with the central repos or existing tool ecosystem very well).

What is the easiest way for me to fix this issue? I can think of 2: modularizing my jar and adding it using --add-module and pljava.vmoptions. Or two, overriding the system classloader (again via pljava.vmoptions) to insert my jar. Would these both work (in theory) and are there drawbacks to either approach? The use of an unpopular technology like modules is already a big drawback for me.

Thank you

PS I don't think removing the classpath in backend.c when modules were added was the right call. I'm in this situation just because of it.

from pljava.

I think I know what I am doing wrong now. I ended up modularizing my jar but it still doesn't work because I didn't understand the lifetime of the vmoptions and other pljava variables. Can someone explain exactly how long these variables last (per connection, per statement, etc) or point me to where in the docs this is. I looked and was unable to find it.

from pljava.

Hello @hunterpayne,

I might be more help if your situation could be described starting a few steps back.

• did you first try loading the needed code with install_jar, before the alternative of putting it on the system class path or module path?
• if so, what problem exactly did you experience when running that way?
• did you experience the problem in a released version of PL/Java, or a version built from the branch for this issue (see 17 October comments above)?
• if it was a released version, do you have an opportunity to try with a build from this branch?

If you prefer to put your jars on the filesystem and load via the system classloader, it is your choice whether to:

• "modularize" your code into a full-fledged named module and add to the module path
• add the jar to the module path without changes (it becomes an "automatic module" whose name is derived from the jar file name)
• add to the module path after only adding an Automatic-Module-Name entry in the manifest (same as above but you control the module's name), or
• continue to treat it as unnamed-module code and add it to the class path.

The section "configuring the launch-time class path" on this page describes how to set that class path if that is what you find convenient.

I take your point that the configuration variable reference doesn't explicitly state the temporal scope of each variable. Nearly all of those are consulted only once at the startup of the JVM, which is started only once in a PostgreSQL session and runs for the rest of that session.

The only ones that can be meaningfully changed within a session are a few like pljava.implementors (which is how conditional inclusion in deployment descriptors works) or the non-PL/Java-specific check_function_bodies (which can change what verification of subsequently-created functions will be done).

from pljava.

First, I got it to work. I'm going to answer your questions and then describe what worked for me and what lessons I think I learned.

Yes I tried putting all the classes in an shaded/uber jar. That didn't work. The reason why is the aforementioned thread, which the client jar creates, gets a different thread context classloader descended from the system classloader. So when it launches it tries to load classes and doesn't find any of my classes since at that time they were all from the PL thread's context classloader. This is on version 1.6.5. I tried the allowed option mentioned above but like I wrote, I am already isolating all the threads from PG.

I ended up breaking off the 3rd party jars including this troublesome client jar into its own Maven meta sub-module. From here I have to do all sorts of horrible things to make the shaded jar it creates be a module that exports and provides all the necessary packages/classes via a module-info.java I wrote by hand. Then I put this on the module path via one of your PLJava variables. This was very painful and absurdly time consuming but it did work. I suspect it will be a continuing source of bugs for me as I keep updating the security model to keep things working.

I put the definitions of the PLJava variables into my postgresql.conf file so I no longer have any issues about those variable's lifetimes. Further, I understand that the definitions of those lifetimes are up to Postgres and the JDBC threading model and not part of your project. It would probably still save users some time to have it spelled out for them somewhere. But I see now it is a bit outside the scope of this project. It would be nice to mark the PL variables that influence the runtime verses the ones that are "config" (only honored at JVM startup). It will be something that many users don't think about at first when using your project.

Finally, I see that Java modules (Jigsaw) really works well for you. It fits your use case almost perfectly. However, that project was doomed to failure the moment they decided to not work with Maven Central (where there are 11m jars ATM). My advice to clients has always been updating past Java 8 was nice to have but not necessary. After using it for a week, that advice will change to updating past Java 8 is like heating your house with $100 bills. I can't see a future for it and I suspect they will kill that project someday and leave you in the lurch. Perhaps make sure you have a plan if/when that happens. I like your project and I don't want to see it negatively effected by external decisions.

PS I got the exception mentioned above along with other ClassNotFoundExceptions for classes that were in my shaded jar. That was the original symptom.

from pljava.

First, I got it to work.

I'm glad to hear that.

Yes I tried putting all the classes in an shaded/uber jar.

Whether to uber-jar it should really be up to your convenience. PL/Java is used routinely with code across multiple jars, so the choice to uber or not to uber is probably not a factor here.

That didn't work. The reason why is the aforementioned thread, which the client jar creates, gets a different thread context classloader descended from the system classloader.

It might be important, here, to know the details of how you determined that. The context classloader of a newly-created thread is normally taken from the thread that created it, which will have been a PG thread whose classloader should be the right one. So, this library may have called the Thread constructor with inheritThreadLocals=false, but that constructor first appears in Java 9, so if the library is an older one, that probably isn't the explanation.

Alternatively, the library might be creating a Thread subclass that overrides getContextClassLoader to return a different value, or the library might be calling setContextClassLoader after creating the thread, supplying a different loader.

However, you would probably know if it's doing either of those things, as you would have had to edit pljava.policy to grant a RuntimePermission (enableContextClassLoaderOverride or setContextClassLoader, respectively).

Or there might have been something else to the explanation. Exception chains and stack traces can be very helpful in an issue like this.

I ended up breaking off the 3rd party jars including this troublesome client jar into its own Maven meta sub-module. From here I have to do all sorts of horrible things to make the shaded jar it creates be a module that exports and provides all the necessary packages/classes via a module-info.java I wrote by hand. ... This was very painful and absurdly time consuming but it did work. I suspect it will be a continuing source of bugs for me as I keep updating the security model to keep things working.

Can you clarify why you did all that to put the stuff on the module path, instead of leaving it alone and putting it on the class path? Was that just because of not seeing the "configuring the launch-time class path" section in the docs, or did you try it and have something go wrong? If so, what?

If you are concerned about your modularization being a source of ongoing effort and bugs, using the class path might be the most convenient choice for you.

I put the definitions of the PLJava variables into my postgresql.conf file so I no longer have any issues about those variable's lifetimes. Further, I understand that the definitions of those lifetimes are up to Postgres and the JDBC threading model and not part of your project.

It really is part of the project; when an extension declares configuration variables, it tells PostgreSQL when and how they can be set, and supplies check hooks for enforcement. All that information is in the code already (for example, you'll find you aren't allowed to set pljava.vmoptions in a given session, if the JVM has already started in that session; you'll even be told why, and what you can do about it). But it wouldn't hurt to repeat more of that information in the docs.

Finally, I see that Java modules (Jigsaw) really works well for you. It fits your use case almost perfectly. However, that project was doomed to failure the moment they decided to not work with Maven Central (where there are 11m jars ATM).

"Doomed to failure" seems a bit strong to me; even if a lot of stuff on Maven Central never gets modularized and people continue putting things on the class path for decades, it has already succeeded in greatly improving the organization of the JRE itself, and made strong encapsulation possible for a project like PL/Java to protect its own abstractions.

I also have not seen that the JPMS design entails any decision "to not work with Maven Central". I see that the repository contains both modularized and non-modularized libraries, and Maven has grown corresponding features and plugins, and people are managing to follow paths of faster or slower evolution according to their priorities.

I suspect they will kill that project someday and leave you in the lurch. Perhaps make sure you have a plan if/when that happens.

That's always a danger (look at some of the things already being deprecated), but I get the sense that JPMS will be around a while, even if only because the JRE's internal development is all in ... and because per-module is now the granularity of newly-added security checks (such as enable-native-access for the foreign function and memory API), with older mechanisms being deprecated. If some future decision 'kills' JPMS, there will be plenty of things to worry about.

from pljava.

Hi @kamillo,

I will try it with some more extensive usage of Threads and will back to you with the update.

Did you ever try with more extensive usage and have any further findings?

Thanks!

from pljava.

I determined that by the stack traces. There were several in addition to the one at the top of this ticket. In this case it was loading the static variables of the inner Thread class. Since in this case these were classes of static fields of the inner class, loading the inner class depended on loading those static field classes first. All of this happens before the things you mentioned because at this point, the JVM doesn't even know the inner class subclasses Thread. It needs to finish loading the class before it can do that.

Those static variables classes were in my uber jar. So the top of the stack trace is the line of the static variables and the rest of the stack trace is calls to the JVM's classloaders. There is no chance, even if I could change this code, to slip a new context classloader into this thread before these classes are required. It is just an inner class that subclasses Thread. It seems in this case, it gets an empty context classloader descended from the system classloader that only has PL/Java's 2 jars on it. I suspect if the inner class loaded, it might receive the correct classloader but that seems to happen after the inner class gets loaded which is too late in the process to save my code.

I have used multiple jars with PL/Java before, that works just fine. I was shading my jar to make the DDL easier.

Finally, security is about economics first and foremost. The cost of breaking in needs to be higher than the cost of the thing you are protecting. Its similar to in a conflict where you use a million dollar missile to destroy a 10,000 dollar arms cache. Its just a bad trade. Similarly, using Jigsaw is like taking the cost of building the protection system and multiplying it by 10. It isn't that Jigsaw doesn't work, it does. It is that the cost of using it is far too high in comparison with the economic trade-offs involved in security decisions. The original Java security model included a tool which would run your program with AllPermissions on, then record the permissions your program required and generate a .policy file containing just those permissions. If Jigsaw was like that to use, it might make sense. It isn't, so it doesn't. Its the complete lack of understanding of the economics of security that Jigsaw shows that is its main problem. That is highlighted by the fact that they think the million or so OpenSource projects are all going to spend weeks modularizing their code which clearly isn't happening and won't happen.

PS As for Jigsaw, I've been on the Maven dev list for many years. I have never seen even a mention of Jigsaw there. The parts of Jigsaw that do similar things to the Maven dependency module are woefully inadequate compared to the Maven dependency module. So even if the Maven folks wanted to work with Jigsaw, Jigsaw can't do it. So why are things like requires even part of the Jigsaw spec? This is one of many questionable decisions in Jigsaw that all seem to spring from the same human frailty.

from pljava.

using Jigsaw is like taking the cost of building the protection system and multiplying it by 10. It isn't that Jigsaw doesn't work, it does. It is that the cost of using it is far too high in comparison with the economic trade-offs involved in security decisions. The original Java security model included ...

The developers of Java clearly have opinions about those tradeoffs as well; just look at the rationales given for JEP 411. I won't say I completely agree with theirs either, but it seems safe to say that they and you both believe such calculations are worth doing, and theirs and yours didn't come out the same way.

But this GitHub issue is about classloading in PL/Java, so maybe the merits or drawbacks of JPMS can be left for elsewhere.

I determined that by the stack traces. There were several in addition to the one at the top of this ticket. In this case it was loading the static variables of the inner Thread class. Since in this case these were classes of static fields of the inner class, loading the inner class depended on loading those static field classes first. All of this happens before the things you mentioned because at this point, the JVM doesn't even know the inner class subclasses Thread. It needs to finish loading the class before it can do that.

It's fairly easy to paste or attach stack traces to an issue like this. The key benefit of including that information, rather than just talking about it, is that more eyeballs on it sometimes reveal more ways of interpreting it. Without the information to look at, that can't happen.

Some key questions here would be: when that classloading was happening, what thread was doing it, and why? And what was that thread's context classloader?

from pljava.

It would take an hour or so to reproduce now that I have fixed the problem. I really don't want to do that. Everyone knows what a ClassNotFoundException looks like.

As for what that thread was doing, nothing. It hadn't started yet. It was still loading the classes necessary to instantiate the static fields of the inner class (which extends Thread). BTW, just load a Java Hadoop HDFS FS, in a PL/Java jar and you will be able to reproduce this yourself. That's the client library I got working. Just put this in a jar. It will create the same issue (yes its Scala, I'm sure you can manage).

    val conf = new Configuration()
    conf.addResource(new Path("src/test/resources/core-site.xml"));
    conf.addResource(new Path("src/test/resources/hdfs-site.xml"));
    conf.set(
      "fs.hdfs.impl",
      classOf[org.apache.hadoop.hdfs.DistributedFileSystem].getName)
    conf.set(
      "fs.file.impl", classOf[org.apache.hadoop.fs.LocalFileSystem].getName)
    Configuration.dumpConfiguration(
      conf, new java.io.OutputStreamWriter(System.out));

    val path = new Path("hdfs://localhost:9000/user/hunter/")
    val fs = path.getFileSystem(conf)
    val fos = fs.open(path); // this triggers the creation of the thread

As for the economic trade-offs, again, they are assuming everyone has the same economics. That's clearly not the case. But somehow my use case (which is the large majority of Java code) is punished at a time where people are looking to leave Java. And what keeps them on Java? Those million open source projects which Jigsaw discounts. So no, I really don't think their decisions were very wise and don't really respect that decision (which is why Java 8 still is more used than anything after). Until they make using it less expensive and difficult, that won't change.

from pljava.

It would take an hour or so to reproduce now that I have fixed the problem. I really don't want to do that.

Understood. You have it working at this point, so there is no need. It will be beneficial if you have another issue (or a followup to this issue) in the future.

Everyone knows what a ClassNotFoundException looks like.

True enough, and everyone also knows the details of the stack trace above it and the causal chain below it are often of use in diagnosing a situation.

As for what that thread was doing, nothing. It hadn't started yet. It was still loading the classes necessary to instantiate the static fields of the inner class (which extends Thread).

Are you thinking that the loading of a new Thread subclass takes place on a thread that an instance of that class has not yet been constructed to represent? That would surprise me; I suspect that a closer look at the stack trace might support some other conclusion. Or if that's not what you're thinking, it's back to the earlier question: what thread was doing that loading?

(yes its Scala, I'm sure you can manage).

Again, no need at this point, as you've got your thing working. If something comes up later, a minimal working reproducer is something it's often courteous to supply.

Other than maybe looking at using the class path instead of the module path so you can be out of the painful modularization business, it sounds like what you've got now does what you need.

from pljava.

The classpath didn't work for me when I tried it. It is entire possible I did something wrong. I was setting -cp on the vmoptions instead of -Djava.class.path so perhaps only one of those works? Also, there is this comment in Backend.c which might be confusing me.

No longer relies on an environment variable. What CLASSPATH variable might happen to be randomly set in the environment of a PostgreSQL backend?

from Backend.c line 1223

from pljava.

The comment in Backend.c is a red herring. That change removed a former dependency on whatever the process environment might have in a variable named CLASSPATH. (Imagine wondering why your PL/Java code is seeing wrong classes because of some variable that happened to be in the shell environment when PostgreSQL was started.)

To set the class path while a module path is present, you have to use -Djava.class.path=... as described in the docs.

from pljava.

I'm thinking that the JVM starts loading the inner Thread subclass. It realizes its a thread and sets up a new context to handle the thread. In that new context it starts loading the Thread subclass which requires loading the classes upon which it depends. This includes the classes of types of static fields of the inner Thread subclass. Somehow, those classes are loaded in a new classloader instead of a clone of the PG main thread's context classloader as we would expect. Its a pretty aweful use case. Here is the client code:

Client.java line 384

from pljava.

I'm doubtful of that analysis. AIUI, a Thread instance, once fully constructed, represents a new thread and its start method can then be called, but nothing executes on that thread before start. The steps up to that point are taking place on an existing thread.

from pljava.

But the thread instance isn't fully constructed in this case. The inner class hasn't even finished loading at this point. Remember the loading of a class depends on the loading of all its fields' classes first. And that would have to happen in a new classloader anyway. Perhaps its a race condition in the JVM, IDK. There also might be another thread involved as there are several inner classes in Client that extend Thread.

PS This is just bad code anyway. There is a reason we don't extend Thread anymore.

from pljava.

We agree it's not fully constructed yet ... which means there isn't any code executing yet on the thread it is being constructed to represent. The loading and construction are happening on some existing thread.

Clearly, there is an explanation for what you saw happening, but it doesn't seem to have been found yet.

Remember the loading of a class depends on the loading of all its fields' classes first.

As a minor quibble, the spec doesn't quite require that; see JLS 12.3: "an implementation may choose to resolve each symbolic reference in a class or interface individually, only when it is used (lazy or late resolution), or to resolve them all at once while the class is being verified (static resolution). This means that the resolution process may continue, in some implementations, after a class or interface has been initialized."

from pljava.

I agree that we don't understand exactly what is happening in the JVM in this case. I also think that it doesn't matter. What matters is that sometimes when a new thread is created, you have to put some sort of jar in the system classloader containing the classes the thread needs. It is also likely that the way you create the thread matters for triggering this bug. The symptoms of this sort of bug are ClassNotFoundExceptions coming from initializing resources (especially static fields) associated with the new thread. Potential factors that can trigger this are threads creating threads and classes extending Thread.

PS Edited, the fields in question are static. I think that will matter too.

from pljava.

What matters is that sometimes when a new thread is created, you have to put some sort of jar in the system classloader containing the classes the thread needs.

At least, we can cal that a workaround that has worked for you.

I think it's also true, if I read one of your earlier posts correctly, that your observations were on a released PL/Java version, not one built from the fixed branch for this issue, and that might also matter. So you may have encountered something the work in October already fixed in this branch, or you may have encountered an additional complication. But as long as your project is working now, a bit of mystery may remain until the issue is encountered by someone else and can be diagnosed further.

from pljava.

I was using a personal build off the V1_6_5 tag. I don't think anything is wrong with your code. I think this is a JVM bug. The only change I would recommend is to add a note in the docs somewhere.

from pljava.

All the same, you reported your issue by attaching it to this already-open GitHub issue that did uncover a bug in PL/Java, and proposed a fix in the branch that is linked here, and not included in the version you have built.

So whatever you or I may think about whether the same bug is a factor in what you observed, that hasn't been tested yet.

from pljava.

Mine is a different bug with the same error message.

The error message above is also the only unique error message in my bug as the others are all ClassNotFoundException which doesn't make for useful search terms. This way anyone searching for that specific error message sees both possibilities.

from pljava.

And that might be just as well for now. The result is we have an issue ticket that may be about two unrelated issues or about one issue you and kamillo both encountered. And the discussion on the ticket already indicates what could be tested to resolve that question, which will be of use to anyone who searches and comes here. That'll do.

from pljava.

Hi @kamillo,

I will try it with some more extensive usage of Threads and will back to you with the update.

Did you ever try with more extensive usage and have any further findings?

Thanks!

Hi @jcflack, not really to be honest. I've done some generic testing but nothing beyond that. Still, I didn't find any further issues.

from pljava.

Believed resolved in 1.6.7.

from pljava.