RFR of JDk9 8164623: Doc typo in java/util/concurrent/ConcurrentLinkedDeque.java and ConcurrentLinkedQueue.java

[David Holmes]

Hi Hamlin,

On 23/08/2016 10:53 PM, Hamlin Li wrote:
>
> On 2016/8/23 17:10, David Holmes wrote:
>> Hi Hamlin,
>>
>> On 23/08/2016 6:55 PM, Hamlin Li wrote:
>>>
>>> Below doc is not correct, because for ConcurrentLinkedDeque and
>>> ConcurrentLinkedQueue, addAll is a atomic operation.
>>
>> No it is not! There is no bug here. Are you perhaps confusing
>> thread-safe with atomic?
> Hi David,
>
> Thank you for review. Please let me clarify.
> Although "public boolean addAll(Collection<? extends E> c)" is not
> protected by any lock or monitor, the implementation in both
> ConcurrentLinked*eque.java still "Atomically append the chain at the
> tail of this collection", so I still think addAll is a atomic method.

Yes you are correct, my apologies.

But as Martin states this is an implementation artifact not something 
that the specification wants to be guaranteeing.

Thanks,
David

> Even though it's not called atomic at this situation, the statement "For
> example, an iterator operating concurrently with an addAll operation
> might view only some of the added elements." is wrong, because either
> all objects in Collection c are viewed by iterator, or none of objects
> in Collection c are viewed by iterator.
>
> Thank you
> -Hamlin
>>
>> David
>> -----
>>
>>> "Additionally, the bulk operations addAll, removeAll, retainAll,
>>> containsAll, equals, and toArray are not guaranteed to be performed
>>> atomically. For example, an iterator operating concurrently with an
>>> addAll operation might view only some of the added elements."
>>> It should be modified as some thing like below:
>>> "Additionally, the bulk operations removeAll, retainAll, containsAll,
>>> equals, and toArray are not guaranteed to be performed atomically."
>>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8164623
>>> webrev: http://cr.openjdk.java.net/~mli/8164623/webrev.00/
>>>
>>> Thank you
>>> -Hamlin
>>>
>