Hi Paul,
many thanks for confirmation. I'm going to fill the bug.
Cheers,
Jaromir
On Wed, Dec 9, 2015 at 1:21 PM, Paul Sandoz <paul.san...@oracle.com> wrote:
> Hi Jaromir,
>
> Thanks, you found a bug, seems to be a hangover from the Map default
> implementation.
>
> One fix might be:
>
> if (oldValue != null) {
> // something to remove
> if (remove(key, oldValue)) {
> // removed the old value as expected
> return null;
> }
>
> // some other value replaced old value. try again.
> oldValue = get(key);
> } else if (contains(key)) {
> // a mapping was added after obtaining the old value, try again
> oldValue = get(key);
> } else {
> // nothing to do. Leave things as they were.
> return null;
> }
>
> Although i am somewhat inclined to do away with the contains check (given
> nulls are not supported), so things are just left alone.
>
> Do you wanna report the issue here:
>
> http://bugreport.java.com/
>
> Thanks,
> Paul.
>
> > On 8 Dec 2015, at 11:12, Jaromir Hamala <jaromir.ham...@gmail.com>
> wrote:
> >
> > Hi,
> >
> > I stumbled upon an interesting issue with default implementation of
> > `compute(K key, BiFunction<? super K, ? super V, ? extends V>
> > remappingFunction)` in JDK8 `ConcurrentMap`.
> > According to its contract the default method implementation assumes map
> > implementations do not support null values.
> >
> > This is the begin of the default implementation:
> >
> > default V compute(K key, BiFunction<? super K, ? super V, ? extends V>
> > remappingFunction) {
> > Objects.requireNonNull(remappingFunction);
> > V oldValue = get(key);
> > for(;;) {
> > V newValue = remappingFunction.apply(key, oldValue);
> > if (newValue == null) {
> > // delete mapping
> > if (oldValue != null || containsKey(key)) {
> > // something to remove
> > if (remove(key, oldValue)) {
> > [...]
> >
> >
> > Let's say we have an empty map and 2 threads:
> > T1 is calling the `compute('foo', someFunction)`
> > T2 is concurrently calling calling `put('foo', 'bar');`
> >
> > so the T1 will get `oldValue = null`, but `containsKey()` will return
> > `true` - because T2 already created the mapping `foo -> bar`. Hence T1
> will
> > call `remove('foo', null)` !
> >
> > Contract of `remove()` says: `throws NullPointerException if the
> specified
> > key or value is null, and this map does not permit null keys or values
> > optional.` -> the T1 will throw NPE.
> > Is it a bug in default method impl or do I understand it wrong?
> >
> > Cheers,
> > Jaromir
> >
> > --
> > “Perfection is achieved, not when there is nothing more to add, but when
> > there is nothing left to take away.”
> > Antoine de Saint Exupéry
>
>
--
“Perfection is achieved, not when there is nothing more to add, but when
there is nothing left to take away.”
Antoine de Saint Exupéry
