I think this comes down to two current (realistic) options: 1) accept Jared's changes as are so we can avoid forced loading of all deployed jars up front (GEODE-2290 will be fixed in the next release) 2) reject the changes until someone has time to retrofit Geode with a new CL container (GEODE-2290 will not be fixed in the next release)
My choice is #1 -- I'm for accepting and applying the current changes to develop (deploy jar will then replace one CL for all deployed jars every time you deploy any jars). What do others want to do? 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/ques > tions/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 > >>> > >>> > >> > >
