Code Review Request for Bug #4802647

[chris hegarty]

On 23/12/2011 21:55, David Holmes wrote:
> Hi Brandon,
>
> This looks okay to me.

Thumbs up from me too.

-Chris.

> 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
>>>
>>