Code Review Request for Bug #4802647
David Holmes
david.holmes at oracle.com
Fri Dec 23 21:55:47 UTC 2011
Hi Brandon,
This looks okay to me.
Just to clarify for other readers, the change to CopyOnWriteArrayList
doesn't use Objects.requireNonNull because this code has to sync up with
Doug Lea's jsr166 repository which runs on multiple version of the
platform (some without requireNonNull).
Also CopyOnWriteArraySet is fixed implicitly by the fix to
CopyOnWriteArrayList.
David
On 24/12/2011 3:30 AM, Brandon Passanisi wrote:
> Here's an updated webrev for review for 4802647, which also contains a
> fix for 7123424. 7123424, as noted below, is the newly filed bug
> regarding the same bug behavior for CopyOnWriteArrayList and
> CopyOnWriteArraySet. It seems best to combine the fix for both into one
> webrev, so this webrev for review is a reflection of this:
>
> Webrev URL:
> http://cr.openjdk.java.net/~bpassani/4802647_7123424/3/webrev/
>
> Thanks.
>
> On 12/21/2011 11:23 AM, Brandon Passanisi wrote:
>> Yes, my intent was "extends AbstractSet<E>" instead of "extends
>> NewAbstractCollection<E>". I have reflected this in the updated webrev
>> below. Here's the information:
>>
>> Webrev URL: http://cr.openjdk.java.net/~bpassani/4802647/2/webrev/
>> Bug URL: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4802647
>>
>> 1. In MOAT.java, I changed NewAbstractSet to extend AbstractSet<E>.
>>
>> 2. The changes in 1) resulted in finding out that AbstractSet has the
>> same bug behavior with removeAll(null).
>> AbstractSet.removeAll(Collection<?> c) was updated accordingly.
>>
>> 3. I filed bug 7123424 to account for the same bug behavior found in
>> CopyOnWriteArrayList and CopyOnWriteArraySet.
>>
>> 4. It was advised that I skip the failing behavior of
>> CopyOnWriteArrayList and CopyOnWriteArraySet for
>> removeAll(null)/retainAll(null) in MOAT.java and provide a comment
>> about how the skip needs to be removed once bug 7123424 is fixed. This
>> is the reason for the instanceof checks that were recently added and
>> the added comments.
>>
>> Thanks.
>>
>> On 12/21/2011 7:52 AM, Jason Mehrens wrote:
>>> > Date: Tue, 20 Dec 2011 10:12:02 +1000
>>> > From: david.holmes at oracle.com
>>> > To: brandon.passanisi at oracle.com
>>> > Subject: Re: Code Review Request for Bug #4802647
>>> > CC: core-libs-dev at openjdk.java.net
>>> >
>>> > Brandon,
>>> >
>>> > I don't see the purpose of NewAbstractSet. It is identical to
>>> > NewAbstractCollection.
>>>
>>> I would assume the intent was "extends AbstractSet<E>" instead of
>>> "extends NewAbstractCollection<E>".
>>>
>>> Jason
>>
>
More information about the core-libs-dev
mailing list