+1 for merging the current fix. It at least makes it easier to deploy a jar in the happy day scenario. I'm not sure how much we should be trying to emulate an application server anyway. If people really want to combine the cache with the app server they can embed Geode into an app server. There may be some API sugar that would make that more useful, but it seems it's a lot less work than all this classloader stuff we're discussing, and would likely yield much better sandboxing.
-- Mike Stolz Principal Engineer, GemFire Product Manager Mobile: +1-631-835-4771 On Tue, Apr 11, 2017 at 7:46 PM, Jinmei Liao <jil...@pivotal.io> wrote: > I believe with the current fix, we at least do not have the "eager loading" > problem anymore. The classloader isolation was a problem before and would > remain as a problem after this for further larger scale changes. +1 for > merging the fix. > > On Tue, Apr 11, 2017 at 4:04 PM, Swapnil Bawaskar <sbawas...@pivotal.io> > wrote: > > > +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 > > > > >>> > > > > >>> > > > > >> > > > > > > > > > > > > > > > > > -- > Cheers > > Jinmei >
