geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jared Stewart <jstew...@pivotal.io>
Subject Re: One class loader vs multiple class loaders for deployed jars
Date Wed, 12 Apr 2017 18:15:25 GMT
I have submitted a review board request for this change, comments and feedback are welcome:
 https://reviews.apache.org/r/58393/ <https://reviews.apache.org/r/58393/>  

I don’t believe it breaks the current expected behavior, and I don’t think it will pose
any additional barriers to classloader isolation in the future (the fast-classpath-scanner
library is agnostic about the class loader implementation you give to it).  

Anthony - I was never able to find any substantial documentation for Plexus Classworlds. 
This makes it both hard to evaluate and potentially scary to use.

- Jared

> On Apr 12, 2017, at 8:55 AM, Michael Stolz <mstolz@pivotal.io> wrote:
> 
> +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 <jiliao@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 <sbawaskar@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 <jbarrett@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 <jstewart@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 <jbarrett@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 <klund@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 <
>> abaker@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 <
>> jstewart@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
>> 


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message