Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

An infinity loop in the method java.util.concurrent.ConcurrentMap#computeIfPresent #11816

Closed
QIvan opened this issue Nov 17, 2017 · 4 comments · Fixed by #11957
Closed

An infinity loop in the method java.util.concurrent.ConcurrentMap#computeIfPresent #11816

QIvan opened this issue Nov 17, 2017 · 4 comments · Fixed by #11957
Assignees
Labels
Source: Community PR or issue was opened by a community user Team: Core Type: Defect
Milestone

Comments

@QIvan
Copy link

QIvan commented Nov 17, 2017

Hi! This code have an infinity loop

import com.hazelcast.config.Config;
import com.hazelcast.core.Hazelcast;
import com.hazelcast.core.HazelcastInstance;
import com.hazelcast.core.IMap;
import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.ToString;

import java.io.Serializable;
import java.math.BigDecimal;
import java.util.concurrent.ConcurrentHashMap;


public class Starter {

    public static final String ACCOUNTS = "accounts";

    public static void main(String[] args) {
        Config cfg = new Config();                                            // 1  
        HazelcastInstance instance = Hazelcast.newHazelcastInstance(cfg);     // 2
        IMap<Long, Account> mapCustomers = instance.getMap(ACCOUNTS);         // 3 
//        Map<Long, Account> mapCustomers = new ConcurrentHashMap<>();        // 4

        System.out.println("init");
        mapCustomers.put(1L, new Account(1L, "my"));
        System.out.println("compute");
        mapCustomers.computeIfPresent(1L, (id, account) -> {
            account.setValue(BigDecimal.TEN);
            return account;
        });
        System.out.println("done");

        mapCustomers.values().forEach(System.out::println);
    }

    @Data
    @EqualsAndHashCode
    @ToString
    public static class Account implements Serializable {
        private final long id;
        private final String name;
        private BigDecimal value = BigDecimal.ZERO;
    }

}

a output will be like this

init
compute

But if we comment lines 1-3 and uncomment line 4 the output will be

init
compute
done
Starter.Account(id=1, name=my, value=10)

as expected.

  1. Hazelcast version: 3.9
  2. Cluster size: any
  3. Number of the clients: any
  4. Version of Java:
$ java -version 
openjdk version "1.8.0_111"
OpenJDK Runtime Environment (build 1.8.0_111-b16)
OpenJDK 64-Bit Server VM (build 25.111-b16, mixed mode)
  1. Operating system: any
  2. Logs and stack traces: -
  3. Detailed description of the steps to reproduce your issue: run the code =)
    8,9 - no available
@QIvan
Copy link
Author

QIvan commented Nov 17, 2017

There is this behavior because in

com.hazelcast.map.impl.operation.ReplaceIfSameOperation#run

we try to replace with a new value and before check that no modifications was between to apply the lambda and an attempt to replace. But the lambda has modified reference already and this check always will be failed because in

com.hazelcast.map.impl.recordstore.DefaultRecordStore#replace(com.hazelcast.nio.serialization.Data, java.lang.Object, java.lang.Object)

we compare the value by a key in the local map with a record in a cluster and there value by the same key has not modified yet.

@QIvan
Copy link
Author

QIvan commented Nov 17, 2017

I know that a Hazelcast's target is Java 6, but in Java 8 this is a bug. The method computeIfAbsent has javadoc

     * If the value for the specified key is present and non-null, attempts to
     * compute a new mapping given the key and its current mapped value.

@mmedenjak mmedenjak added this to the 3.10 milestone Nov 18, 2017
@taburet taburet self-assigned this Dec 8, 2017
taburet added a commit that referenced this issue Dec 12, 2017
Additionally, the test case for ConcurrentMap.computeIfPresent-like
behavior is provided to ensure we are supporting it properly in its
current restricted form.

Closes: #11816
@taburet
Copy link
Contributor

taburet commented Dec 12, 2017

@QIvan Unfortunately, the issue can't be fixed at this moment. The root cause of the problem is that Hazelcast maps expect values to have value type semantics, while standard Java implementations expect reference type semantics. Hazelcast is still at Java 6, so we can't override computeIfPresent to provide our own implementation to fix this.

I have enhanced IMap javadoc to make things more clear about value types vs reference types and created a separate issue #11958 to provide our own computeIfPresent implementation in Hazelcast 4.0, which will be based on a more recent Java version.

You may workaround the issue by returning a new value instance from remappingFunction instead of modifying the existing value in-place. If you think there is more to this issue, feel free to reopen. If you are experiencing other problems, please open a new issue.

@mmedenjak mmedenjak added the Source: Community PR or issue was opened by a community user label Jan 28, 2020
@mmedenjak
Copy link
Contributor

Fixed in 4.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Source: Community PR or issue was opened by a community user Team: Core Type: Defect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants