You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by "Zakharov, Vasily M" <va...@intel.com> on 2007/04/09 21:20:44 UTC
[jira][swing] Could someone please commit HARMONY-3454 ?
Hi,
Could someone please commit
http://issues.apache.org/jira/browse/HARMONY-3454 ?
It has patch available and it looks like the discussion over it is over.
Thank you!
Vasily Zakharov
Intel ESSD
RE: [jira][swing] Could someone please commit HARMONY-3454 ?
Posted by "Zakharov, Vasily M" <va...@intel.com>.
Sure, I'll make a new patch.
Thank you!
Vasily
-----Original Message-----
From: nbeyer@gmail.com [mailto:nbeyer@gmail.com] On Behalf Of Nathan
Beyer
Sent: Wednesday, April 11, 2007 2:41 AM
To: dev@harmony.apache.org
Subject: Re: [jira][swing] Could someone please commit HARMONY-3454 ?
I think I'm convinced of the catch Throwable for now, but let's put
some comments in the catch block to document that this is being done
for RI compatibility right now and that this could be tweaked in the
future.
As for the 'contentTypes' field, I'm fine with either changing it to
use a Hashtable or just putting synchronized blocks around the
get/put/etc invocations and using the field as the lock. Perhaps a
code comment about this would be nice as well.
I'll take assignment of the bug. Do you mind uploading a new patch, or
do you want me to just tweak what's there? Whatever you prefer is fine
with me.
-Nathan
On 4/10/07, Zakharov, Vasily M <va...@intel.com> wrote:
>
> > I guess if the RI is really capturing everything we can do it, but
I'd
> > prefer to go the way of a non-bug difference here, unless we have
some
> > applications that are depending on this behavior.
>
> Sure, we can make a NBD here. The question is, what would our behavior
> be?
> Catching just Exception is clearly wrong. If not catching Throwable we
> should
> at least catch Exception and LinkageError (with subclasses). Do you
> agree?
>
> Do you think we should file a separate NBD JIRA for this?
>
> > How do you know it's using a Hashtable?
>
> See the stack in the issue description, it's pretty clear.
> Also this is discussed in comments in
> http://issues.apache.org/jira/browse/HARMONY-3453
>
> > In this case, I would suggest that we just protect the data
structure
> > itself, such that only the actual read/write of the Map is
protected.
> > This makes sure that the data structure itself doesn't blow up with
a
> > spurious NPE (or the like) during a read/write race within the Map
> > itself. This is what a Hashtable would prevent because it makes each
> > method invocation atomic.
>
> Ok, I agree. We can really use Hashtable now as the code is
restructured
> and doesn't need null keys/values.
>
> Vasily
>
>
> -----Original Message-----
> From: nbeyer@gmail.com [mailto:nbeyer@gmail.com] On Behalf Of Nathan
> Beyer
> Sent: Wednesday, April 11, 2007 12:11 AM
> To: dev@harmony.apache.org
> Subject: Re: [jira][swing] Could someone please commit HARMONY-3454 ?
>
> On 4/10/07, Zakharov, Vasily M <va...@intel.com> wrote:
> >
> > Thanks for checking the patch, Nathan!
> >
> > 1.1. The problem is the wrapped fragment performs class loading, and
> > can easily produce LinkageError, ExceptionInInitializerError,
> > IllegalAccessError
> > etc. and all these must not disrupt the code operation.
> >
> > 1.2. It is a question of compatibility. RI seems to catch everything
> > at this point, including ThreadDeath. :(
>
> I guess if the RI is really capturing everything we can do it, but I'd
> prefer to go the way of a non-bug difference here, unless we have some
> applications that are depending on this behavior.
>
> >
> > 2.1. Adding synchronization is not a problem at all. But:
> >
> > 2.2. What is a point to make a tread-safe point in non-thread-safe
> > package?
> > If user can't and shouldn't rely on internal thread-safety and must
> > think
> > of synchronization himself, why waste performance (probably twice,
as
> > user
> > provides his own synchronization) to provide thread-safety in one
> > particular
> > place? Isn't it a drop in the ocean?
>
> The synchronization would be a minimal performance hit, especially
> considering it's protecting the integrity of the data structure.
>
> >
> > 2.3. The only reason I see for adding the synchronization here is RI
> > uses
> > Hashtable that is synchronized, and we probably would like to be
> > compatible
> > in this manner.
>
> How do you know it's using a Hashtable?
>
> If it is, then it backs up my understanding of the thread-safety
> guarantees of Swing. There aren't any guarantees for code that's
> expected to be executed exclusively in the UI thread and protected via
> isolation, but shared data that could be updated or queried in any
> thread needs to be protected for consistency.
>
> In this case, I would suggest that we just protect the data structure
> itself, such that only the actual read/write of the Map is protected.
> This makes sure that the data structure itself doesn't blow up with a
> spurious NPE (or the like) during a read/write race within the Map
> itself. This is what a Hashtable would prevent because it makes each
> method invocation atomic.
>
> -Nathan
>
> >
> > Vasily
> >
> >
> > -----Original Message-----
> > From: nbeyer@gmail.com [mailto:nbeyer@gmail.com] On Behalf Of Nathan
> > Beyer
> > Sent: Tuesday, April 10, 2007 8:42 AM
> > To: dev@harmony.apache.org
> > Subject: Re: [jira][swing] Could someone please commit HARMONY-3454
?
> >
> > I have a few comments about the patch -
> >
> > 1. I think the "catch(Throwable)" in JEditorPane should be change,
at
> > a minimum, to "catch(Exception)". We don't want to hide things like
> > OutOfMemoryErrors, which could happen anywhere.
> >
> > 2. There's no concurrency protection for the static Map,
contentTypes.
> > I realize that many parts of Swing and AWT aren't thread safe, but
> > this doesn't seem like one of them. Perhaps this wasn't properly
> > protected before, but we should add it now.
> >
> > -Nathan
> >
> > On 4/9/07, Zakharov, Vasily M <va...@intel.com> wrote:
> > > Hi,
> > >
> > > Could someone please commit
> > > http://issues.apache.org/jira/browse/HARMONY-3454 ?
> > >
> > > It has patch available and it looks like the discussion over it is
> > over.
> > >
> > > Thank you!
> > >
> > > Vasily Zakharov
> > > Intel ESSD
> > >
> >
>
Re: [jira][swing] Could someone please commit HARMONY-3454 ?
Posted by Nathan Beyer <nd...@apache.org>.
I think I'm convinced of the catch Throwable for now, but let's put
some comments in the catch block to document that this is being done
for RI compatibility right now and that this could be tweaked in the
future.
As for the 'contentTypes' field, I'm fine with either changing it to
use a Hashtable or just putting synchronized blocks around the
get/put/etc invocations and using the field as the lock. Perhaps a
code comment about this would be nice as well.
I'll take assignment of the bug. Do you mind uploading a new patch, or
do you want me to just tweak what's there? Whatever you prefer is fine
with me.
-Nathan
On 4/10/07, Zakharov, Vasily M <va...@intel.com> wrote:
>
> > I guess if the RI is really capturing everything we can do it, but I'd
> > prefer to go the way of a non-bug difference here, unless we have some
> > applications that are depending on this behavior.
>
> Sure, we can make a NBD here. The question is, what would our behavior
> be?
> Catching just Exception is clearly wrong. If not catching Throwable we
> should
> at least catch Exception and LinkageError (with subclasses). Do you
> agree?
>
> Do you think we should file a separate NBD JIRA for this?
>
> > How do you know it's using a Hashtable?
>
> See the stack in the issue description, it's pretty clear.
> Also this is discussed in comments in
> http://issues.apache.org/jira/browse/HARMONY-3453
>
> > In this case, I would suggest that we just protect the data structure
> > itself, such that only the actual read/write of the Map is protected.
> > This makes sure that the data structure itself doesn't blow up with a
> > spurious NPE (or the like) during a read/write race within the Map
> > itself. This is what a Hashtable would prevent because it makes each
> > method invocation atomic.
>
> Ok, I agree. We can really use Hashtable now as the code is restructured
> and doesn't need null keys/values.
>
> Vasily
>
>
> -----Original Message-----
> From: nbeyer@gmail.com [mailto:nbeyer@gmail.com] On Behalf Of Nathan
> Beyer
> Sent: Wednesday, April 11, 2007 12:11 AM
> To: dev@harmony.apache.org
> Subject: Re: [jira][swing] Could someone please commit HARMONY-3454 ?
>
> On 4/10/07, Zakharov, Vasily M <va...@intel.com> wrote:
> >
> > Thanks for checking the patch, Nathan!
> >
> > 1.1. The problem is the wrapped fragment performs class loading, and
> > can easily produce LinkageError, ExceptionInInitializerError,
> > IllegalAccessError
> > etc. and all these must not disrupt the code operation.
> >
> > 1.2. It is a question of compatibility. RI seems to catch everything
> > at this point, including ThreadDeath. :(
>
> I guess if the RI is really capturing everything we can do it, but I'd
> prefer to go the way of a non-bug difference here, unless we have some
> applications that are depending on this behavior.
>
> >
> > 2.1. Adding synchronization is not a problem at all. But:
> >
> > 2.2. What is a point to make a tread-safe point in non-thread-safe
> > package?
> > If user can't and shouldn't rely on internal thread-safety and must
> > think
> > of synchronization himself, why waste performance (probably twice, as
> > user
> > provides his own synchronization) to provide thread-safety in one
> > particular
> > place? Isn't it a drop in the ocean?
>
> The synchronization would be a minimal performance hit, especially
> considering it's protecting the integrity of the data structure.
>
> >
> > 2.3. The only reason I see for adding the synchronization here is RI
> > uses
> > Hashtable that is synchronized, and we probably would like to be
> > compatible
> > in this manner.
>
> How do you know it's using a Hashtable?
>
> If it is, then it backs up my understanding of the thread-safety
> guarantees of Swing. There aren't any guarantees for code that's
> expected to be executed exclusively in the UI thread and protected via
> isolation, but shared data that could be updated or queried in any
> thread needs to be protected for consistency.
>
> In this case, I would suggest that we just protect the data structure
> itself, such that only the actual read/write of the Map is protected.
> This makes sure that the data structure itself doesn't blow up with a
> spurious NPE (or the like) during a read/write race within the Map
> itself. This is what a Hashtable would prevent because it makes each
> method invocation atomic.
>
> -Nathan
>
> >
> > Vasily
> >
> >
> > -----Original Message-----
> > From: nbeyer@gmail.com [mailto:nbeyer@gmail.com] On Behalf Of Nathan
> > Beyer
> > Sent: Tuesday, April 10, 2007 8:42 AM
> > To: dev@harmony.apache.org
> > Subject: Re: [jira][swing] Could someone please commit HARMONY-3454 ?
> >
> > I have a few comments about the patch -
> >
> > 1. I think the "catch(Throwable)" in JEditorPane should be change, at
> > a minimum, to "catch(Exception)". We don't want to hide things like
> > OutOfMemoryErrors, which could happen anywhere.
> >
> > 2. There's no concurrency protection for the static Map, contentTypes.
> > I realize that many parts of Swing and AWT aren't thread safe, but
> > this doesn't seem like one of them. Perhaps this wasn't properly
> > protected before, but we should add it now.
> >
> > -Nathan
> >
> > On 4/9/07, Zakharov, Vasily M <va...@intel.com> wrote:
> > > Hi,
> > >
> > > Could someone please commit
> > > http://issues.apache.org/jira/browse/HARMONY-3454 ?
> > >
> > > It has patch available and it looks like the discussion over it is
> > over.
> > >
> > > Thank you!
> > >
> > > Vasily Zakharov
> > > Intel ESSD
> > >
> >
>
RE: [jira][swing] Could someone please commit HARMONY-3454 ?
Posted by "Zakharov, Vasily M" <va...@intel.com>.
> I guess if the RI is really capturing everything we can do it, but I'd
> prefer to go the way of a non-bug difference here, unless we have some
> applications that are depending on this behavior.
Sure, we can make a NBD here. The question is, what would our behavior
be?
Catching just Exception is clearly wrong. If not catching Throwable we
should
at least catch Exception and LinkageError (with subclasses). Do you
agree?
Do you think we should file a separate NBD JIRA for this?
> How do you know it's using a Hashtable?
See the stack in the issue description, it's pretty clear.
Also this is discussed in comments in
http://issues.apache.org/jira/browse/HARMONY-3453
> In this case, I would suggest that we just protect the data structure
> itself, such that only the actual read/write of the Map is protected.
> This makes sure that the data structure itself doesn't blow up with a
> spurious NPE (or the like) during a read/write race within the Map
> itself. This is what a Hashtable would prevent because it makes each
> method invocation atomic.
Ok, I agree. We can really use Hashtable now as the code is restructured
and doesn't need null keys/values.
Vasily
-----Original Message-----
From: nbeyer@gmail.com [mailto:nbeyer@gmail.com] On Behalf Of Nathan
Beyer
Sent: Wednesday, April 11, 2007 12:11 AM
To: dev@harmony.apache.org
Subject: Re: [jira][swing] Could someone please commit HARMONY-3454 ?
On 4/10/07, Zakharov, Vasily M <va...@intel.com> wrote:
>
> Thanks for checking the patch, Nathan!
>
> 1.1. The problem is the wrapped fragment performs class loading, and
> can easily produce LinkageError, ExceptionInInitializerError,
> IllegalAccessError
> etc. and all these must not disrupt the code operation.
>
> 1.2. It is a question of compatibility. RI seems to catch everything
> at this point, including ThreadDeath. :(
I guess if the RI is really capturing everything we can do it, but I'd
prefer to go the way of a non-bug difference here, unless we have some
applications that are depending on this behavior.
>
> 2.1. Adding synchronization is not a problem at all. But:
>
> 2.2. What is a point to make a tread-safe point in non-thread-safe
> package?
> If user can't and shouldn't rely on internal thread-safety and must
> think
> of synchronization himself, why waste performance (probably twice, as
> user
> provides his own synchronization) to provide thread-safety in one
> particular
> place? Isn't it a drop in the ocean?
The synchronization would be a minimal performance hit, especially
considering it's protecting the integrity of the data structure.
>
> 2.3. The only reason I see for adding the synchronization here is RI
> uses
> Hashtable that is synchronized, and we probably would like to be
> compatible
> in this manner.
How do you know it's using a Hashtable?
If it is, then it backs up my understanding of the thread-safety
guarantees of Swing. There aren't any guarantees for code that's
expected to be executed exclusively in the UI thread and protected via
isolation, but shared data that could be updated or queried in any
thread needs to be protected for consistency.
In this case, I would suggest that we just protect the data structure
itself, such that only the actual read/write of the Map is protected.
This makes sure that the data structure itself doesn't blow up with a
spurious NPE (or the like) during a read/write race within the Map
itself. This is what a Hashtable would prevent because it makes each
method invocation atomic.
-Nathan
>
> Vasily
>
>
> -----Original Message-----
> From: nbeyer@gmail.com [mailto:nbeyer@gmail.com] On Behalf Of Nathan
> Beyer
> Sent: Tuesday, April 10, 2007 8:42 AM
> To: dev@harmony.apache.org
> Subject: Re: [jira][swing] Could someone please commit HARMONY-3454 ?
>
> I have a few comments about the patch -
>
> 1. I think the "catch(Throwable)" in JEditorPane should be change, at
> a minimum, to "catch(Exception)". We don't want to hide things like
> OutOfMemoryErrors, which could happen anywhere.
>
> 2. There's no concurrency protection for the static Map, contentTypes.
> I realize that many parts of Swing and AWT aren't thread safe, but
> this doesn't seem like one of them. Perhaps this wasn't properly
> protected before, but we should add it now.
>
> -Nathan
>
> On 4/9/07, Zakharov, Vasily M <va...@intel.com> wrote:
> > Hi,
> >
> > Could someone please commit
> > http://issues.apache.org/jira/browse/HARMONY-3454 ?
> >
> > It has patch available and it looks like the discussion over it is
> over.
> >
> > Thank you!
> >
> > Vasily Zakharov
> > Intel ESSD
> >
>
Re: [jira][swing] Could someone please commit HARMONY-3454 ?
Posted by Nathan Beyer <nd...@apache.org>.
On 4/10/07, Zakharov, Vasily M <va...@intel.com> wrote:
>
> Thanks for checking the patch, Nathan!
>
> 1.1. The problem is the wrapped fragment performs class loading, and
> can easily produce LinkageError, ExceptionInInitializerError,
> IllegalAccessError
> etc. and all these must not disrupt the code operation.
>
> 1.2. It is a question of compatibility. RI seems to catch everything
> at this point, including ThreadDeath. :(
I guess if the RI is really capturing everything we can do it, but I'd
prefer to go the way of a non-bug difference here, unless we have some
applications that are depending on this behavior.
>
> 2.1. Adding synchronization is not a problem at all. But:
>
> 2.2. What is a point to make a tread-safe point in non-thread-safe
> package?
> If user can't and shouldn't rely on internal thread-safety and must
> think
> of synchronization himself, why waste performance (probably twice, as
> user
> provides his own synchronization) to provide thread-safety in one
> particular
> place? Isn't it a drop in the ocean?
The synchronization would be a minimal performance hit, especially
considering it's protecting the integrity of the data structure.
>
> 2.3. The only reason I see for adding the synchronization here is RI
> uses
> Hashtable that is synchronized, and we probably would like to be
> compatible
> in this manner.
How do you know it's using a Hashtable?
If it is, then it backs up my understanding of the thread-safety
guarantees of Swing. There aren't any guarantees for code that's
expected to be executed exclusively in the UI thread and protected via
isolation, but shared data that could be updated or queried in any
thread needs to be protected for consistency.
In this case, I would suggest that we just protect the data structure
itself, such that only the actual read/write of the Map is protected.
This makes sure that the data structure itself doesn't blow up with a
spurious NPE (or the like) during a read/write race within the Map
itself. This is what a Hashtable would prevent because it makes each
method invocation atomic.
-Nathan
>
> Vasily
>
>
> -----Original Message-----
> From: nbeyer@gmail.com [mailto:nbeyer@gmail.com] On Behalf Of Nathan
> Beyer
> Sent: Tuesday, April 10, 2007 8:42 AM
> To: dev@harmony.apache.org
> Subject: Re: [jira][swing] Could someone please commit HARMONY-3454 ?
>
> I have a few comments about the patch -
>
> 1. I think the "catch(Throwable)" in JEditorPane should be change, at
> a minimum, to "catch(Exception)". We don't want to hide things like
> OutOfMemoryErrors, which could happen anywhere.
>
> 2. There's no concurrency protection for the static Map, contentTypes.
> I realize that many parts of Swing and AWT aren't thread safe, but
> this doesn't seem like one of them. Perhaps this wasn't properly
> protected before, but we should add it now.
>
> -Nathan
>
> On 4/9/07, Zakharov, Vasily M <va...@intel.com> wrote:
> > Hi,
> >
> > Could someone please commit
> > http://issues.apache.org/jira/browse/HARMONY-3454 ?
> >
> > It has patch available and it looks like the discussion over it is
> over.
> >
> > Thank you!
> >
> > Vasily Zakharov
> > Intel ESSD
> >
>
RE: [jira][swing] Could someone please commit HARMONY-3454 ?
Posted by "Ivanov, Alexey A" <al...@intel.com>.
>-----Original Message-----
>From: nbeyer@gmail.com [mailto:nbeyer@gmail.com] On Behalf Of Nathan
Beyer
>Sent: Tuesday, April 10, 2007 8:42 AM
>To: dev@harmony.apache.org
>Subject: Re: [jira][swing] Could someone please commit HARMONY-3454 ?
>
>I have a few comments about the patch -
>
>1. I think the "catch(Throwable)" in JEditorPane should be change, at
>a minimum, to "catch(Exception)". We don't want to hide things like
>OutOfMemoryErrors, which could happen anywhere.
Good point.
>
>2. There's no concurrency protection for the static Map, contentTypes.
>I realize that many parts of Swing and AWT aren't thread safe, but
>this doesn't seem like one of them. Perhaps this wasn't properly
>protected before, but we should add it now.
AWT is supposed to be thread-safe whereas Swing is not. JavaDoc for
Swing and its tutorials state Swing is not thread-safe if the doc
doesn't clearly state that a method, class is thread-safe. And in all
Swing examples provided by Sun, GUI initialization is performed in Event
Dispatch Thread rather than main thread.
That's why there was no concurrency protection here, and that's why it
was not implemented now. Of course it may be added...
Regards,
Alexey.
--
Alexey A. Ivanov
Intel Enterprise Solutions Software Division
>
>-Nathan
>
>On 4/9/07, Zakharov, Vasily M <va...@intel.com> wrote:
>> Hi,
>>
>> Could someone please commit
>> http://issues.apache.org/jira/browse/HARMONY-3454 ?
>>
>> It has patch available and it looks like the discussion over it is
over.
>>
>> Thank you!
>>
>> Vasily Zakharov
>> Intel ESSD
>>
RE: [jira][swing] Could someone please commit HARMONY-3454 ?
Posted by "Zakharov, Vasily M" <va...@intel.com>.
Thanks for checking the patch, Nathan!
1.1. The problem is the wrapped fragment performs class loading, and
can easily produce LinkageError, ExceptionInInitializerError,
IllegalAccessError
etc. and all these must not disrupt the code operation.
1.2. It is a question of compatibility. RI seems to catch everything
at this point, including ThreadDeath. :(
2.1. Adding synchronization is not a problem at all. But:
2.2. What is a point to make a tread-safe point in non-thread-safe
package?
If user can't and shouldn't rely on internal thread-safety and must
think
of synchronization himself, why waste performance (probably twice, as
user
provides his own synchronization) to provide thread-safety in one
particular
place? Isn't it a drop in the ocean?
2.3. The only reason I see for adding the synchronization here is RI
uses
Hashtable that is synchronized, and we probably would like to be
compatible
in this manner.
Vasily
-----Original Message-----
From: nbeyer@gmail.com [mailto:nbeyer@gmail.com] On Behalf Of Nathan
Beyer
Sent: Tuesday, April 10, 2007 8:42 AM
To: dev@harmony.apache.org
Subject: Re: [jira][swing] Could someone please commit HARMONY-3454 ?
I have a few comments about the patch -
1. I think the "catch(Throwable)" in JEditorPane should be change, at
a minimum, to "catch(Exception)". We don't want to hide things like
OutOfMemoryErrors, which could happen anywhere.
2. There's no concurrency protection for the static Map, contentTypes.
I realize that many parts of Swing and AWT aren't thread safe, but
this doesn't seem like one of them. Perhaps this wasn't properly
protected before, but we should add it now.
-Nathan
On 4/9/07, Zakharov, Vasily M <va...@intel.com> wrote:
> Hi,
>
> Could someone please commit
> http://issues.apache.org/jira/browse/HARMONY-3454 ?
>
> It has patch available and it looks like the discussion over it is
over.
>
> Thank you!
>
> Vasily Zakharov
> Intel ESSD
>
Re: [jira][swing] Could someone please commit HARMONY-3454 ?
Posted by Nathan Beyer <nd...@apache.org>.
I have a few comments about the patch -
1. I think the "catch(Throwable)" in JEditorPane should be change, at
a minimum, to "catch(Exception)". We don't want to hide things like
OutOfMemoryErrors, which could happen anywhere.
2. There's no concurrency protection for the static Map, contentTypes.
I realize that many parts of Swing and AWT aren't thread safe, but
this doesn't seem like one of them. Perhaps this wasn't properly
protected before, but we should add it now.
-Nathan
On 4/9/07, Zakharov, Vasily M <va...@intel.com> wrote:
> Hi,
>
> Could someone please commit
> http://issues.apache.org/jira/browse/HARMONY-3454 ?
>
> It has patch available and it looks like the discussion over it is over.
>
> Thank you!
>
> Vasily Zakharov
> Intel ESSD
>