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

ClientDelegatingFuture falsely given null response #10581

Closed
sancar opened this issue May 15, 2017 · 0 comments
Closed

ClientDelegatingFuture falsely given null response #10581

sancar opened this issue May 15, 2017 · 0 comments

Comments

@sancar
Copy link
Member

@sancar sancar commented May 15, 2017

When two callbacks are registered as callback to ClientDelegatingFuture,
if one is wants to deserialize the response while the other is not,
we are falsely giving null response to one waiting a deserialized value.

Issue can be seen with following test in 3.8 and 3.7

Nearcache internally registers a non-deserializing callback.
Users callback wants to deserialize the object.
User ends up with null response.

 Config config = new XmlConfigBuilder().build();
        Hazelcast.newHazelcastInstance(config);

        ClientConfig clientConfig = new ClientConfig();

        NearCacheConfig nearCacheConfig = new NearCacheConfig();
        nearCacheConfig.setName("map");
        EvictionConfig evictionConfig = new EvictionConfig();
        evictionConfig.setSize(10000);
        evictionConfig.setEvictionPolicy(EvictionPolicy.LRU);
        nearCacheConfig.setEvictionConfig(evictionConfig);
        nearCacheConfig.setInvalidateOnChange(false);

        clientConfig.addNearCacheConfig(nearCacheConfig);
        HazelcastInstance client = HazelcastClient.newHazelcastClient(clientConfig);

        IMap<Object, AtomicInteger> clientMap = client.getMap("map");

        for (int i = 0; i < 1000; i++) {
            clientMap.put(i, new AtomicInteger(i));

            ICompletableFuture future = clientMap.getAsync(i);
            final int finalI = i;
            future.andThen(new ExecutionCallback<Object>() {
                @Override
                public void onResponse(Object response) {
                    if (response == null) {
                        System.out.println("NULL " + finalI);
                    }
                }

                @Override
                public void onFailure(Throwable t) {
                }
            });
        }


@sancar sancar added this to the 3.8.3 milestone May 15, 2017
@sancar sancar self-assigned this May 15, 2017
sancar added a commit to sancar/hazelcast that referenced this issue May 15, 2017
sancar added a commit to sancar/hazelcast that referenced this issue May 15, 2017
sancar added a commit to sancar/hazelcast that referenced this issue May 16, 2017
The old implementation was trying to deserialize only ones and every delegating future was using same deserialized value.
With this change, every callback and get method deserializes the response again.
Initial motivation is to simplify and get rid of lock. Not sure about performance implications:
waiting for lock vs using more cpu but parallel work.
AbstractInvocationFuture seems also deserializes multiple times for each callback so I followed the same logic.

fixes hazelcast#10581
fixes hazelcast#8127
sancar added a commit to sancar/hazelcast that referenced this issue May 16, 2017
The old implementation was trying to deserialize only ones and every delegating future was using same deserialized value.
With this change, every callback and get method deserializes the response again.
Initial motivation is to simplify and get rid of lock. Not sure about performance implications:
waiting for lock vs using more cpu but parallel work.
AbstractInvocationFuture seems also deserializes multiple times for each callback so I followed the same logic.

fixes hazelcast#10581
fixes hazelcast#8127
sancar added a commit to sancar/hazelcast that referenced this issue May 16, 2017
The old implementation was trying to deserialize only ones and every delegating future was using same deserialized value.
With this change, every callback and get method deserializes the response again.
Initial motivation is to simplify and get rid of lock. Not sure about performance implications:
waiting for lock vs using more cpu but parallel work.
AbstractInvocationFuture seems also deserializes multiple times for each callback so I followed the same logic.

fixes hazelcast#10581
fixes hazelcast#8127
sancar added a commit to sancar/hazelcast that referenced this issue May 16, 2017
The old implementation was trying to deserialize only ones and every delegating future was using same deserialized value.
With this change, every callback and get method deserializes the response again.
Initial motivation is to simplify and get rid of lock. Not sure about performance implications:
waiting for lock vs using more cpu but parallel work.
AbstractInvocationFuture seems also deserializes multiple times for each callback so I followed the same logic.

fixes hazelcast#10581
fixes hazelcast#8127
sancar added a commit to sancar/hazelcast that referenced this issue May 16, 2017
The old implementation was trying to deserialize only ones and every delegating future was using same deserialized value.
With this change, every callback and get method deserializes the response again.
Initial motivation is to simplify and get rid of lock. Not sure about performance implications:
waiting for lock vs using more cpu but parallel work.
AbstractInvocationFuture seems also deserializes multiple times for each callback so I followed the same logic.

fixes hazelcast#10581
fixes hazelcast#8127
sancar added a commit to sancar/hazelcast that referenced this issue May 16, 2017
The old implementation was trying to deserialize only ones and every delegating future was using same deserialized value.
With this change, every callback and get method deserializes the response again.
Initial motivation is to simplify and get rid of lock. Not sure about performance implications:
waiting for lock vs using more cpu but parallel work.
AbstractInvocationFuture seems also deserializes multiple times for each callback so I followed the same logic.

fixes hazelcast#10581
fixes hazelcast#8127
sancar added a commit to sancar/hazelcast that referenced this issue May 16, 2017
The old implementation was trying to deserialize only ones and every delegating future was using same deserialized value.
With this change, every callback and get method deserializes the response again.
Initial motivation is to simplify and get rid of lock. Not sure about performance implications:
waiting for lock vs using more cpu but parallel work.
AbstractInvocationFuture seems also deserializes multiple times for each callback so I followed the same logic.

fixes hazelcast#10581
fixes hazelcast#8127
sancar added a commit to sancar/hazelcast that referenced this issue Jun 2, 2017
The old implementation was trying to deserialize only ones and every delegating future was using same deserialized value.
With this change, every callback and get method deserializes the response again.
Initial motivation is to simplify and get rid of lock. Not sure about performance implications:
waiting for lock vs using more cpu but parallel work.
AbstractInvocationFuture seems also deserializes multiple times for each callback so I followed the same logic.

fixes hazelcast#10581
fixes hazelcast#8127
sancar added a commit to sancar/hazelcast that referenced this issue Jun 2, 2017
The old implementation was trying to deserialize only ones and every delegating future was using same deserialized value.
With this change, every callback and get method deserializes the response again.
Initial motivation is to simplify and get rid of lock. Not sure about performance implications:
waiting for lock vs using more cpu but parallel work.
AbstractInvocationFuture seems also deserializes multiple times for each callback so I followed the same logic.

fixes hazelcast#10581
fixes hazelcast#8127
sancar added a commit to sancar/hazelcast that referenced this issue Jun 6, 2017
The old implementation was trying to deserialize only ones and every delegating future was using same deserialized value.
With this change, every callback and get method deserializes the response again.
Initial motivation is to simplify and get rid of lock. Not sure about performance implications:
waiting for lock vs using more cpu but parallel work.
AbstractInvocationFuture seems also deserializes multiple times for each callback so I followed the same logic.

fixes hazelcast#10581
fixes hazelcast#8127
sancar added a commit to sancar/hazelcast that referenced this issue Jun 6, 2017
The old implementation was trying to deserialize only ones and every delegating future was using same deserialized value.
With this change, every callback and get method deserializes the response again.
Initial motivation is to simplify and get rid of lock. Not sure about performance implications:
waiting for lock vs using more cpu but parallel work.
AbstractInvocationFuture seems also deserializes multiple times for each callback so I followed the same logic.

fixes hazelcast#10581
fixes hazelcast#8127
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.