+1 for merging Jared's fix. it gets us closer to fixing classloading in a later release.
On Tue, Apr 11, 2017 at 1:41 PM Jacob Barrett <jbarr...@pivotal.io> wrote: > I can support the idea of taking the small step of fixing the eager class > loading if it doesn't: > 1) break the current expected behavior. > 2) dig is deeper into a hole with class loading isolation in the future. > > If it breaks the current expected behavior, that in itself is ok until you > consider that adding class loader isolation later will most definitely > break the expected behavior, can we afford do break it twice? > > -Jake > > On Tue, Apr 11, 2017 at 10:06 AM Jared Stewart <jstew...@pivotal.io> > wrote: > > > I would like to distinguish between the following two objectives: > > 1) Do not eagerly load every class contained in a deployed jar > > 2) Establish robust classloader isolation for deployed jars > > > > The aim of my current line of work is to fix 1) (GEODE-2290). This is > not > > to say that 2) is not a valuable pursuit, but I think getting 2) done > > correctly is a larger task than fixing 1) in isolation. > > > > To get into the specifics of the libraries you mentioned, > > > > JCL: > > - JCL has no support for removing/undeploying jars. In this respect, I > > don't see anything that JCL would add over simply using a URLClassLoader. > > We would have to rebuild the JCL class loader in exactly the same set of > > circumstances that we would need to rebuild a URLClassLoader. > > - I have seen a fair number of warnings from people that JCL is buggy > and > > incomplete, e.g. ( > > > http://stackoverflow.com/questions/60764/how-should-i-load-jars-dynamically-at-runtime#comment43066872_1450837 > ) > > This would seem to be supported by a quick look at the github issues page > > for JCL, which includes things like a bug report of a deadlock from Jun > > 2016 to which the developers have never responded. > > > > JBoss Modules: > > - JBoss Modules (or Java 9/Jigsaw Modules) seem like a good strategic > > direction towards which to strive. Yet the fact that they require an > > external module descriptor (XML) would again seem to make this a much > > larger task than 1) alone. (If the idea here is to have a user provide > us > > with a JBoss Module rather than a plain jar file, this would be a large > > breaking change for existing users. On the other hand, if the idea is > for > > us to autogenerate the necessary metainformation to transform the user's > > jar file into a JBoss Module, this would seem to be a large task which is > > again independent from the goals of 1). > > > > - Jared > > > > > On Apr 10, 2017, at 9:41 PM, Jacob Barrett <jbarr...@pivotal.io> > wrote: > > > > > > There are certainly many projects in the OS community that have solved > > this > > > same problem. Perhaps we can find a class loader from a project that > > would > > > suite this need. > > > > > > Quick search of standalone frameworks comes up with a few popular hits: > > > https://github.com/kamranzafar/JCL > > > https://github.com/jboss-modules/jboss-modules > > > > > > -Jake > > > > > > > > > > > > > > > On Mon, Apr 10, 2017 at 1:40 PM Kirk Lund <kl...@apache.org> wrote: > > > > > >> I think Jared started down this path because we had a custom > classloader > > >> implementation that he was trying to get rid of -- that impl was > pretty > > >> limited and forced loading of all classes up-front. > > >> > > >> Now the code uses fast classpath scanner and that old custom > classloader > > >> can't be used. The old classloader is so simplistic and limited that > > trying > > >> to modify it looks like more work than writing a new one from scratch. > > >> Implementing a new custom classloader or switching our codebase to > use a > > >> new classloader container (using an existing solution) are both > possible > > >> directions. Until that's completed the "deploy jar" command will > > continue > > >> to force ALL classes to be loaded up-front. > > >> > > >> One major concern is that implementing a new custom classloader is > > >> non-trivial and could also introduce some new classloader bugs -- of > > >> particular interest to "deploy jar" is the fact that Java remembers > TWO > > >> classloaders for each class [1] -- the CL that *loaded* the class AND > > the > > >> CL that *initiated* the request to load the class -- dropping the > > >> *initiating* CL at runtime can result in failures to load additional > > >> classes from the CL that directly loaded the class even though that CL > > is > > >> intact and available. > > >> > > >> [1] states: "When one class loader delegates to another class loader, > > the > > >> loader that initiates the loading is not necessarily the same loader > > that > > >> completes the loading and defines the class. If L creates C, either by > > >> defining it directly or by delegation, we say that L initiates loading > > of C > > >> or, equivalently, that L is an *initiating* loader of C." > > >> > > >> For better or worse, this is one of the reasons why that old custom CL > > was > > >> naively force loading all classes up-front -- ie, to avoid runtime > > >> classloading failures if the initiating CL was dropped or replaced and > > >> ultimately GC'ed. Java won't let you drop the *loading* CL but it will > > >> allow you to drop the *initiating* CL (or it did historically -- the > > >> reference seems to be down in native code, not in java.lang.Class). > > You'd > > >> have to find some way to force all initiating requests up to the > parent > > >> application CL (or somehow prevent code in deployed jars from > initiating > > >> requests from other CLs) and maybe that's what this old custom > > classloader > > >> was missing all along. > > >> > > >> The tradeoff mentioned by Jared is only necessary if we want a release > > >> (soon) that does NOT eagerly class load all deployed classes up-front. > > >> Otherwise, this is a feature request that users might have to wait a > > little > > >> longer for (and maybe that's ok!). > > >> > > >> [1] > > >> > https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-5.html#jvms-5.3 > > >> > > >> On Mon, Apr 10, 2017 at 10:30 AM, Anthony Baker <aba...@pivotal.io> > > wrote: > > >> > > >>> What about this: > > >>> > > >>> 1) Each jar is deployed into it’s own classloader. > > >>> 2) If the classloader for jar1 is unable to load a class, it > delegates > > to > > >>> its parent which can load classes from siblings to jar1. > > >>> > > >>> The classloader hierarchy would be: > > >>> > > >>> bootstrap << system << application << (deploy jar)* > > >>> > > >>> where the application classloader manages the delegation to all > > deployed > > >>> jars. > > >>> > > >>> Anthony > > >>> > > >>> > > >>>> On Apr 10, 2017, at 10:20 AM, Jared Stewart <jstew...@pivotal.io> > > >> wrote: > > >>>> > > >>>> There is last one implementation decision for GEODE-2290 that I'm > torn > > >>> about, namely having one classloader for all deployed jars vs having > > >>> separate classloaders for each deployed jar. > > >>>> > > >>>> If we have one class loader, e.g. new UrlClassLoader(jar1, jar2), > > then: > > >>>> > > >>>> - Jar1 will be able to load classes/resources from jar2 (this was > the > > >>> previous behavior of deployed jars with our custom class loader) > > >>>> - But if we redeploy jar 1, jar 2 will also get its class loader > > >>> rebuilt, which may raise the likelihood of weird classloader problems > > >>> arising > > >>>> > > >>>> Does anyone else have thoughts about this tradeoff? > > >>>> > > >>>> Thanks, > > >>>> Jared > > >>> > > >>> > > >> > > > > >
