You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by Aaron Mulder <am...@alumni.princeton.edu> on 2004/10/03 06:00:23 UTC
Connector Work ClassLoader
So it looks like when a "Work" is executed, the thread context
class loader is not useful. I have to set it to
getClass().getClassLoader() in order to access content in the connector's
JARs. Is it OK if I put in a bug to set the TCCL to the RA CL while a
Work from that RA is executed, and set it back to the otherone thereafter?
Thanks,
Aaron
Re: Connector Work ClassLoader
Posted by Dain Sundstrom <ds...@gluecode.com>.
Aaron it problem is setting the thread context classloader is a very
very expensive operation, so tend not to do it unless required by the
spec. For example, a gbean no longer operates in a TCCL since setting
it was taking 80% of our execution time.
-dain
--
Dain Sundstrom
Chief Architect
Gluecode Software
310.536.8355, ext. 26
On Oct 3, 2004, at 9:17 AM, Aaron Mulder wrote:
> On Sun, 3 Oct 2004, David Jencks wrote:
>> I've spent some time looking at the j2eeca 1.5 spec again and can't
>> fine any mention of which thread context classloader a Work submission
>> is supposed to operate under. Therefore I think if you want your
>> adapter to be portable you should not make any assumptions about it
>> and
>> we have no need to set the thread context classloader ourselves. So,
>> while I'm open to discussion, so far I'm -1 on this.
>
> I don't understand. If the spec is unclear (and I didn't even
> look so it may well be), why don't we take the option that is friendly
> to
> developers, instead of the option that will likely break things? It
> may
> be nice in practice to point out to people when their code is not
> perfectly portable, but it would be even nicer if their code just ran.
> I
> can understand being -1 on my non-portable Work code, but I can't agree
> with being -1 on providing a more forgiving Connector implementation.
>
> FYI, when I encountered this, it was because I tried to read a
> SOAP message in my Work, and the SAAJ API implementation apparently
> uses
> the TCCL to load its implementation. So I wasn't doing anything
> particularly offensive here, and it wasn't even clear why a ClassLoader
> should be involved (after all, if i can instantiate SAAJ classes,
> doesn't
> that automatically mean that they're accessible to me? Well, in this
> case, the API was but the implementation wasn't because it doesn't use
> Class.forName).
>
> Aaron
Re: Connector Work ClassLoader
Posted by Aaron Mulder <am...@alumni.princeton.edu>.
Okay, I give. :)
Aaron
On Sun, 3 Oct 2004, Dain Sundstrom wrote:
> On Oct 3, 2004, at 11:56 AM, Aaron Mulder wrote:
>
> > On Sun, 3 Oct 2004, David Jencks wrote:
> >> At the moment, the only way I see to implement this would be to supply
> >> a separate WorkManager instance to each ResourceAdapter that could
> >> associate the RA classloader with the work request.
> >
> > Couldn't the implementation go like this (in WorkerContext.run):
> >
> >
> > ClassLoader before = Thread.currentThread().getContextClassLoader();
> > try {
> > Thread.currentThread().setContextClassLoader(adaptee.getClass().
> > getClassLoader());
> > ...
> > adaptee.run();
> > ...
> > } finally {
> > Thread.currentThread().setContextClassLoader(before);
> > }
>
> That won't work. The adaptee class may be loaded from a parent class
> loader of the actual ra, which in my opinion would be actually worse
> then having a null TCCL.
>
> > The speed issue, well, I guess that's a more significant problem.
> > If it's not slow to *read* the TCCL, we may want to do something like
> > the
> > above as a defensive measure -- reset the TCCL to the proper one if the
> > Work changes it without changing it back. So we'd only record the
> > existing (or desired) CL before invoking adaptee.run, and we'd put an
> > "if(Thread.cT().getCCL() != before)" before resetting it in the finally
> > clause.
>
> Both operations are expensive. Of course dropping it from 3 operations
> to 1 is better, but the general policy of geronimo is to only set the
> TCCL is absolutely required by a specification (for example we set it
> for EJBs).
>
> -dain
>
>
Re: Connector Work ClassLoader
Posted by Dain Sundstrom <ds...@gluecode.com>.
On Oct 3, 2004, at 11:56 AM, Aaron Mulder wrote:
> On Sun, 3 Oct 2004, David Jencks wrote:
>> At the moment, the only way I see to implement this would be to supply
>> a separate WorkManager instance to each ResourceAdapter that could
>> associate the RA classloader with the work request.
>
> Couldn't the implementation go like this (in WorkerContext.run):
>
>
> ClassLoader before = Thread.currentThread().getContextClassLoader();
> try {
> Thread.currentThread().setContextClassLoader(adaptee.getClass().
> getClassLoader());
> ...
> adaptee.run();
> ...
> } finally {
> Thread.currentThread().setContextClassLoader(before);
> }
That won't work. The adaptee class may be loaded from a parent class
loader of the actual ra, which in my opinion would be actually worse
then having a null TCCL.
> The speed issue, well, I guess that's a more significant problem.
> If it's not slow to *read* the TCCL, we may want to do something like
> the
> above as a defensive measure -- reset the TCCL to the proper one if the
> Work changes it without changing it back. So we'd only record the
> existing (or desired) CL before invoking adaptee.run, and we'd put an
> "if(Thread.cT().getCCL() != before)" before resetting it in the finally
> clause.
Both operations are expensive. Of course dropping it from 3 operations
to 1 is better, but the general policy of geronimo is to only set the
TCCL is absolutely required by a specification (for example we set it
for EJBs).
-dain
Re: Connector Work ClassLoader
Posted by Aaron Mulder <am...@alumni.princeton.edu>.
On Sun, 3 Oct 2004, David Jencks wrote:
> At the moment, the only way I see to implement this would be to supply
> a separate WorkManager instance to each ResourceAdapter that could
> associate the RA classloader with the work request.
Couldn't the implementation go like this (in WorkerContext.run):
ClassLoader before = Thread.currentThread().getContextClassLoader();
try {
Thread.currentThread().setContextClassLoader(adaptee.getClass().
getClassLoader());
...
adaptee.run();
...
} finally {
Thread.currentThread().setContextClassLoader(before);
}
The speed issue, well, I guess that's a more significant problem.
If it's not slow to *read* the TCCL, we may want to do something like the
above as a defensive measure -- reset the TCCL to the proper one if the
Work changes it without changing it back. So we'd only record the
existing (or desired) CL before invoking adaptee.run, and we'd put an
"if(Thread.cT().getCCL() != before)" before resetting it in the finally
clause.
Aaron
> Aside from the extreme slowness Dain mentioned, I don't see how to
> implement this proposal without a major rewriting of the WorkManager.
> Nothing else in connector-land executes with the TCCL set to the
> connector's classloader, so I'm not really seeing why the Work
> submissions should.
Re: Connector Work ClassLoader
Posted by David Jencks <da...@yahoo.com>.
Aside from the extreme slowness Dain mentioned, I don't see how to
implement this proposal without a major rewriting of the WorkManager.
Nothing else in connector-land executes with the TCCL set to the
connector's classloader, so I'm not really seeing why the Work
submissions should.
At the moment, the only way I see to implement this would be to supply
a separate WorkManager instance to each ResourceAdapter that could
associate the RA classloader with the work request.
thanks
david jencks
On Oct 3, 2004, at 9:17 AM, Aaron Mulder wrote:
> On Sun, 3 Oct 2004, David Jencks wrote:
>> I've spent some time looking at the j2eeca 1.5 spec again and can't
>> fine any mention of which thread context classloader a Work submission
>> is supposed to operate under. Therefore I think if you want your
>> adapter to be portable you should not make any assumptions about it
>> and
>> we have no need to set the thread context classloader ourselves. So,
>> while I'm open to discussion, so far I'm -1 on this.
>
> I don't understand. If the spec is unclear (and I didn't even
> look so it may well be), why don't we take the option that is friendly
> to
> developers, instead of the option that will likely break things? It
> may
> be nice in practice to point out to people when their code is not
> perfectly portable, but it would be even nicer if their code just ran.
> I
> can understand being -1 on my non-portable Work code, but I can't agree
> with being -1 on providing a more forgiving Connector implementation.
>
> FYI, when I encountered this, it was because I tried to read a
> SOAP message in my Work, and the SAAJ API implementation apparently
> uses
> the TCCL to load its implementation. So I wasn't doing anything
> particularly offensive here, and it wasn't even clear why a ClassLoader
> should be involved (after all, if i can instantiate SAAJ classes,
> doesn't
> that automatically mean that they're accessible to me? Well, in this
> case, the API was but the implementation wasn't because it doesn't use
> Class.forName).
>
> Aaron
>
Re: Connector Work ClassLoader
Posted by Aaron Mulder <am...@alumni.princeton.edu>.
On Sun, 3 Oct 2004, David Jencks wrote:
> I've spent some time looking at the j2eeca 1.5 spec again and can't
> fine any mention of which thread context classloader a Work submission
> is supposed to operate under. Therefore I think if you want your
> adapter to be portable you should not make any assumptions about it and
> we have no need to set the thread context classloader ourselves. So,
> while I'm open to discussion, so far I'm -1 on this.
I don't understand. If the spec is unclear (and I didn't even
look so it may well be), why don't we take the option that is friendly to
developers, instead of the option that will likely break things? It may
be nice in practice to point out to people when their code is not
perfectly portable, but it would be even nicer if their code just ran. I
can understand being -1 on my non-portable Work code, but I can't agree
with being -1 on providing a more forgiving Connector implementation.
FYI, when I encountered this, it was because I tried to read a
SOAP message in my Work, and the SAAJ API implementation apparently uses
the TCCL to load its implementation. So I wasn't doing anything
particularly offensive here, and it wasn't even clear why a ClassLoader
should be involved (after all, if i can instantiate SAAJ classes, doesn't
that automatically mean that they're accessible to me? Well, in this
case, the API was but the implementation wasn't because it doesn't use
Class.forName).
Aaron
Re: Connector Work ClassLoader
Posted by David Jencks <dj...@gluecode.com>.
I've spent some time looking at the j2eeca 1.5 spec again and can't
fine any mention of which thread context classloader a Work submission
is supposed to operate under. Therefore I think if you want your
adapter to be portable you should not make any assumptions about it and
we have no need to set the thread context classloader ourselves. So,
while I'm open to discussion, so far I'm -1 on this.
thanks
david jencks
On Oct 2, 2004, at 9:00 PM, Aaron Mulder wrote:
> So it looks like when a "Work" is executed, the thread context
> class loader is not useful. I have to set it to
> getClass().getClassLoader() in order to access content in the
> connector's
> JARs. Is it OK if I put in a bug to set the TCCL to the RA CL while a
> Work from that RA is executed, and set it back to the otherone
> thereafter?
>
> Thanks,
> Aaron
>