You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Elena Semukhina <el...@gmail.com> on 2007/02/07 14:27:31 UTC
Re: [classlib][net] java.net.URLClassLoader implementation is not thread-safe
On 1/30/07, Geir Magnusson Jr. <ge...@pobox.com> wrote:
>
> I have a comment, that I'll bring out to the dev list -
>
> Is there any problem with some other thread adding the entry for
> currentURL while the if (jf==null) block is doing it's work? That's
> one difference I can see between sync-ing the block, vs just syncing
> the map.
>
> I don't know if it matters, though.
This remark is quite relevant. Each thread which gets jf == null for some
URL does the same job on openning connection, getting jar file and putting
it to a map. Block synchronization ensures that this job is done once for
each jar file.
Moreover, I ran the classloader.StressLoader test on an unpatched build and
see stable hang on a Windows 2CPUx4core machine. Threads are deadlocked
while doing the same job somewhere in
org/apache/harmony/luni/internal/net/www/protocol/jar/JarURLConnection.findJarFile().
The patch resolves the issue.
Mikhail, could you please add some more comments to
https://issues.apache.org/jira/browse/HARMONY-2982?
Elena
geir
>
> On Jan 30, 2007, at 7:48 AM, Mikhail Markov wrote:
>
> > Elena, thanks for the patch!
> > I've reviewed it and posted the comment to this JIRA.
> >
> > Regards,
> > Mikhail
> >
> >
> > On 1/30/07, Elena Semukhina <el...@gmail.com> wrote:
> >>
> >> The DRLVM smoke test classloader.StressLoader fails intermittently.
> >> Investigation revealed that java.net.URLClassLoader implementation
> >> does
> >> not
> >> take care of synchronizing on access to its IdentityHashMap field. I
> >> suggested a patch to fix the issue at
> >> https://issues.apache.org/jira/browse/HARMONY-2982.
> >>
> >> Could anyone please take a look and review the patch?
> >>
> >> --
> >> Thanks,
> >> Elena
> >>
> >>
>
>
--
Thanks,
Elena
Re: [classlib][net] java.net.URLClassLoader implementation is not thread-safe
Posted by Elena Semukhina <el...@gmail.com>.
Me and Mikhail reached a consensus in improvement of the patch for
https://issues.apache.org/jira/browse/HARMONY-2982. While working on the
patch I found another bug described in
https://issues.apache.org/jira/browse/HARMONY-3207 and Mikhail provided a
patch for it.
We need these patches to be committed now. Could anyone of committers take a
look at them?
Thanks,
Elena
On 2/14/07, Elena Semukhina <el...@gmail.com> wrote:
>
>
>
> On 2/8/07, Tim Ellison <t....@gmail.com> wrote:
> >
> > Elena Semukhina wrote:
> > > On 1/30/07, Geir Magnusson Jr. <geir@pobox.com > wrote:
> > >>
> > >> I have a comment, that I'll bring out to the dev list -
> > >>
> > >> Is there any problem with some other thread adding the entry for
> > >> currentURL while the if (jf==null) block is doing it's work? That's
> > >> one difference I can see between sync-ing the block, vs just syncing
> > >> the map.
> > >>
> > >> I don't know if it matters, though.
> > >
> > >
> > > This remark is quite relevant. Each thread which gets jf == null for
> > some
> > > URL does the same job on openning connection, getting jar file and
> > putting
> > > it to a map. Block synchronization ensures that this job is done once
> > for
> > > each jar file.
> >
> > In general we would allow the concurrent update of a cache, but in this
> > case it sounds like the cost of a cache miss is very high, so that it
> > may be worth synchronizing.
> >
> > > Moreover, I ran the classloader.StressLoader test on an unpatched
> > build and
> > > see stable hang on a Windows 2CPUx4core machine. Threads are
> > deadlocked
> > > while doing the same job somewhere in
> > >
> > org/apache/harmony/luni/internal/net/www/protocol/jar/JarURLConnection.findJarFile().
> > >
> > > The patch resolves the issue.
> >
> > Does that imply that there is a synchronization problem 'lower down'?
>
>
> I'm not sure that we have a "lower" synchronization problem here. The
> deadlock happens somewhere in internal classes when many threads try to find
> the same file while it needs to be found once. Synchronization on an upper
> level helps avoiding this.
>
> Thanks,
> Elena
>
> Regards,
> > Tim
> >
> > > Mikhail, could you please add some more comments to
> > > https://issues.apache.org/jira/browse/HARMONY-2982?
> > >
> > > Elena
> > >
> > >
> > > geir
> > >>
> > >> On Jan 30, 2007, at 7:48 AM, Mikhail Markov wrote:
> > >>
> > >> > Elena, thanks for the patch!
> > >> > I've reviewed it and posted the comment to this JIRA.
> > >> >
> > >> > Regards,
> > >> > Mikhail
> > >> >
> > >> >
> > >> > On 1/30/07, Elena Semukhina < elena.semukhina@gmail.com> wrote:
> > >> >>
> > >> >> The DRLVM smoke test classloader.StressLoader fails
> > intermittently.
> > >> >> Investigation revealed that java.net.URLClassLoader implementation
> > >> >> does
> > >> >> not
> > >> >> take care of synchronizing on access to its IdentityHashMap field.
> > I
> > >> >> suggested a patch to fix the issue at
> > >> >> https://issues.apache.org/jira/browse/HARMONY-2982.
> > >> >>
> > >> >> Could anyone please take a look and review the patch?
> > >> >>
> > >> >> --
> > >> >> Thanks,
> > >> >> Elena
> > >> >>
> > >> >>
> > >>
> > >>
> > >
> > >
> >
>
>
Re: [classlib][net] java.net.URLClassLoader implementation is not thread-safe
Posted by Elena Semukhina <el...@gmail.com>.
On 2/8/07, Tim Ellison <t....@gmail.com> wrote:
>
> Elena Semukhina wrote:
> > On 1/30/07, Geir Magnusson Jr. <ge...@pobox.com> wrote:
> >>
> >> I have a comment, that I'll bring out to the dev list -
> >>
> >> Is there any problem with some other thread adding the entry for
> >> currentURL while the if (jf==null) block is doing it's work? That's
> >> one difference I can see between sync-ing the block, vs just syncing
> >> the map.
> >>
> >> I don't know if it matters, though.
> >
> >
> > This remark is quite relevant. Each thread which gets jf == null for
> some
> > URL does the same job on openning connection, getting jar file and
> putting
> > it to a map. Block synchronization ensures that this job is done once
> for
> > each jar file.
>
> In general we would allow the concurrent update of a cache, but in this
> case it sounds like the cost of a cache miss is very high, so that it
> may be worth synchronizing.
>
> > Moreover, I ran the classloader.StressLoader test on an unpatched build
> and
> > see stable hang on a Windows 2CPUx4core machine. Threads are deadlocked
> > while doing the same job somewhere in
> >
> org/apache/harmony/luni/internal/net/www/protocol/jar/JarURLConnection.findJarFile().
> >
> > The patch resolves the issue.
>
> Does that imply that there is a synchronization problem 'lower down'?
I'm not sure that we have a "lower" synchronization problem here. The
deadlock happens somewhere in internal classes when many threads try to find
the same file while it needs to be found once. Synchronization on an upper
level helps avoiding this.
Thanks,
Elena
Regards,
> Tim
>
> > Mikhail, could you please add some more comments to
> > https://issues.apache.org/jira/browse/HARMONY-2982?
> >
> > Elena
> >
> >
> > geir
> >>
> >> On Jan 30, 2007, at 7:48 AM, Mikhail Markov wrote:
> >>
> >> > Elena, thanks for the patch!
> >> > I've reviewed it and posted the comment to this JIRA.
> >> >
> >> > Regards,
> >> > Mikhail
> >> >
> >> >
> >> > On 1/30/07, Elena Semukhina <el...@gmail.com> wrote:
> >> >>
> >> >> The DRLVM smoke test classloader.StressLoader fails intermittently.
> >> >> Investigation revealed that java.net.URLClassLoader implementation
> >> >> does
> >> >> not
> >> >> take care of synchronizing on access to its IdentityHashMap field. I
> >> >> suggested a patch to fix the issue at
> >> >> https://issues.apache.org/jira/browse/HARMONY-2982.
> >> >>
> >> >> Could anyone please take a look and review the patch?
> >> >>
> >> >> --
> >> >> Thanks,
> >> >> Elena
> >> >>
> >> >>
> >>
> >>
> >
> >
>
Re: [classlib][net] java.net.URLClassLoader implementation is not
thread-safe
Posted by Tim Ellison <t....@gmail.com>.
Elena Semukhina wrote:
> On 1/30/07, Geir Magnusson Jr. <ge...@pobox.com> wrote:
>>
>> I have a comment, that I'll bring out to the dev list -
>>
>> Is there any problem with some other thread adding the entry for
>> currentURL while the if (jf==null) block is doing it's work? That's
>> one difference I can see between sync-ing the block, vs just syncing
>> the map.
>>
>> I don't know if it matters, though.
>
>
> This remark is quite relevant. Each thread which gets jf == null for some
> URL does the same job on openning connection, getting jar file and putting
> it to a map. Block synchronization ensures that this job is done once for
> each jar file.
In general we would allow the concurrent update of a cache, but in this
case it sounds like the cost of a cache miss is very high, so that it
may be worth synchronizing.
> Moreover, I ran the classloader.StressLoader test on an unpatched build and
> see stable hang on a Windows 2CPUx4core machine. Threads are deadlocked
> while doing the same job somewhere in
> org/apache/harmony/luni/internal/net/www/protocol/jar/JarURLConnection.findJarFile().
>
> The patch resolves the issue.
Does that imply that there is a synchronization problem 'lower down'?
Regards,
Tim
> Mikhail, could you please add some more comments to
> https://issues.apache.org/jira/browse/HARMONY-2982?
>
> Elena
>
>
> geir
>>
>> On Jan 30, 2007, at 7:48 AM, Mikhail Markov wrote:
>>
>> > Elena, thanks for the patch!
>> > I've reviewed it and posted the comment to this JIRA.
>> >
>> > Regards,
>> > Mikhail
>> >
>> >
>> > On 1/30/07, Elena Semukhina <el...@gmail.com> wrote:
>> >>
>> >> The DRLVM smoke test classloader.StressLoader fails intermittently.
>> >> Investigation revealed that java.net.URLClassLoader implementation
>> >> does
>> >> not
>> >> take care of synchronizing on access to its IdentityHashMap field. I
>> >> suggested a patch to fix the issue at
>> >> https://issues.apache.org/jira/browse/HARMONY-2982.
>> >>
>> >> Could anyone please take a look and review the patch?
>> >>
>> >> --
>> >> Thanks,
>> >> Elena
>> >>
>> >>
>>
>>
>
>