Infinite Loop in IMap for compute method

[Alykoff]

• Hazelcast: 3.8.9, 3.9.3, 3.10-BETA-1
• Java:

Java(TM) SE Runtime Environment (build 1.8.0_161-b12)
Java HotSpot(TM) 64-Bit Server VM (build 25.161-b12, mixed mode)

• Operating system: macOS High Sierra 10.13.3

When I run this code I have infinite loop (inside java.util.concurrent.ConcurrentMap#compute):

import com.hazelcast.config.Config;
import com.hazelcast.config.MapConfig;
import com.hazelcast.core.Hazelcast;
import com.hazelcast.core.IMap;
import java.io.Serializable;
import java.util.*;

public class Main {
    public static void main(String[] args) {
        IMap<String, List<StringWrapper>> store = Hazelcast.newHazelcastInstance(
                new Config().addMapConfig(new MapConfig("store"))
        ).getMap("store");
        store.compute("user", (k, value) -> {
            List<StringWrapper> newValues = Objects.isNull(value) ? new ArrayList<>() : value;
            newValues.add(new StringWrapper("user"));
            return newValues;
        });
        store.compute("user", (k, value) -> {
            // if use the next comment line everything is ok
            // List<StringWrapper> newValues = Objects.isNull(value) ? new ArrayList<>() : new ArrayList<>(value);
            List<StringWrapper> newValues = Objects.isNull(value) ? new ArrayList<>() : value;
            newValues.add(new StringWrapper("user"));
            return newValues;
        });

        System.out.println(store);
    }

    public static class StringWrapper implements Serializable {
        String value;

        public StringWrapper() {}

        public StringWrapper(String value) {
            this.value = value;
        }

        public String getValue() {
            return value;
        }

        public void setValue(String value) {
            this.value = value;
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;
            if (!super.equals(o)) return false;
            StringWrapper value = (StringWrapper) o;
            return Objects.equals(this.value, value.value);
        }

        @Override
        public int hashCode() {
            return Objects.hash(super.hashCode(), value);
        }
    }
}

Output:

Mar 10, 2018 9:55:39 PM com.hazelcast.instance.AddressPicker
INFO: [LOCAL] [dev] [3.9.3] Prefer IPv4 stack is true.
Mar 10, 2018 9:55:39 PM com.hazelcast.instance.AddressPicker
INFO: [LOCAL] [dev] [3.9.3] Picked [169.254.248.244]:5701, using socket ServerSocket[addr=/0:0:0:0:0:0:0:0,localport=5701], bind any local is true
Mar 10, 2018 9:55:39 PM com.hazelcast.system
INFO: [169.254.248.244]:5701 [dev] [3.9.3] Hazelcast 3.9.3 (20180216 - 539b124) starting at [169.254.248.244]:5701
Mar 10, 2018 9:55:39 PM com.hazelcast.system
INFO: [169.254.248.244]:5701 [dev] [3.9.3] Copyright (c) 2008-2018, Hazelcast, Inc. All Rights Reserved.
Mar 10, 2018 9:55:39 PM com.hazelcast.system
INFO: [169.254.248.244]:5701 [dev] [3.9.3] Configured Hazelcast Serialization version: 1
Mar 10, 2018 9:55:39 PM com.hazelcast.spi.impl.operationservice.impl.BackpressureRegulator
INFO: [169.254.248.244]:5701 [dev] [3.9.3] Backpressure is disabled
Mar 10, 2018 9:55:39 PM com.hazelcast.instance.Node
INFO: [169.254.248.244]:5701 [dev] [3.9.3] Creating MulticastJoiner
Mar 10, 2018 9:55:40 PM com.hazelcast.spi.impl.operationexecutor.impl.OperationExecutorImpl
INFO: [169.254.248.244]:5701 [dev] [3.9.3] Starting 8 partition threads and 5 generic threads (1 dedicated for priority tasks)
Mar 10, 2018 9:55:40 PM com.hazelcast.internal.diagnostics.Diagnostics
INFO: [169.254.248.244]:5701 [dev] [3.9.3] Diagnostics disabled. To enable add -Dhazelcast.diagnostics.enabled=true to the JVM arguments.
Mar 10, 2018 9:55:40 PM com.hazelcast.core.LifecycleService
INFO: [169.254.248.244]:5701 [dev] [3.9.3] [169.254.248.244]:5701 is STARTING
Mar 10, 2018 9:55:42 PM com.hazelcast.system
INFO: [169.254.248.244]:5701 [dev] [3.9.3] Cluster version set to 3.9
Mar 10, 2018 9:55:42 PM com.hazelcast.internal.cluster.ClusterService
INFO: [169.254.248.244]:5701 [dev] [3.9.3] 

Members {size:1, ver:1} [
	Member [169.254.248.244]:5701 - f9548e71-eee7-4abd-9d28-9cca4ef5db26 this
]

Mar 10, 2018 9:55:42 PM com.hazelcast.core.LifecycleService
INFO: [169.254.248.244]:5701 [dev] [3.9.3] [169.254.248.244]:5701 is STARTED
Mar 10, 2018 9:55:42 PM com.hazelcast.internal.partition.impl.PartitionStateManager
INFO: [169.254.248.244]:5701 [dev] [3.9.3] Initializing cluster partition table arrangement...

[gokhanoner]

@Alykoff I reproduced the issue based on your Stackoverflow example using HashSet & then checked the ArrayList version. There are 2 different problem:

1 - When using HashSet, the problem is how Java deserialize the HashSet/ArrayList (collections) & how compute method works. Inside compute method (since Hazelcast complied with Java 6 & there is no compute method to override, default implementation from ConcurrentMap called ), this block causes the infinite loop:

                    // replace
                    if (replace(key, oldValue, newValue)) {
                        // replaced as expected.
                        return newValue;
                    }

                    // some other value replaced old value. try again.
                    oldValue = get(key);

this replace method calls IMap replace method. IMap checks if the current value equal to the user-supplied value. But because of a Java Serialization optimization, the check fails. Please check HashSet.readObject method. You'll see that when deserializing the HashSet, since element size is known, it creates the inner HashMap with a capacity, overriding the capacity stored while object serialized:

        // Set the capacity according to the size and load factor ensuring that
        // the HashMap is at least 25% full but clamping to maximum capacity.
        capacity = (int) Math.min(size * Math.min(1 / loadFactor, 4.0f),
                HashMap.MAXIMUM_CAPACITY);

But your HashSet, created without an initial capacity, has a default capacity of 16, while the deserialized one has the initial capacity of 1. This changes the serialization, index 51 contains the current capacity & it seems JDK re-calculate it based on size when deserializing the object to minimize the size.

Please see below example:

        HazelcastInstance instance = Hazelcast.newHazelcastInstance();

        IMap<String, Collection<String>> store = instance.getMap("store");

        Collection<String> val = new HashSet<>();
        val.add("a");

        store.put("a", val);

        Collection<String> oldVal = store.get("a");

        byte[] dataOld = ((HazelcastInstanceProxy) hz).getSerializationService().toBytes(oldVal);
        byte[] dataNew = ((HazelcastInstanceProxy) hz).getSerializationService().toBytes(val);

        System.out.println(Arrays.equals(dataNew, dataOld));

This code prints false. But if you create the HashSet with the initial size 1, then both byte arrays are equal. And in your case, you won't get an infinite loop.

2 - When using ArrayList, or any other collection, there's another problem which you pointed above. Due to how compute method implemented in ConcurrentMap, when you assign the old value to the newValue & add a new element, you actually modify the oldValue thus causing replace method fail. But when you change the code to new ArrayList(value), now you're creating a new ArrayList & value collection is not modified. It's a best practice to wrap a collection before using it if you don't want to modify the original one. Same works for HashSet if you create with size 1 due to the first issue I explained.

So in your case, you should use

 Collection<String> newValues = Objects.isNull(value) ? new HashSet<>(1) : new HashSet<>(value);

or

Collection<String> newValues = Objects.isNull(value) ? new ArrayList<>() : new ArrayList<>(value);

That HashSet case seems to be a JDK issue, rather than an optimization. I don't know any of these cases can be solved/fixed in Hazelcast, unless Hazalcast overrides the HashXXX collection serialization & overrides the compute method.

[mdogan]

Related to #11816

[ahmetmircik]

closing this in favour of #11958