[
https://issues.apache.org/jira/browse/RATIS-1923?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Tsz-wo Sze updated RATIS-1923:
------------------------------
Description:
- The update methods in AtomicReference run in a loop and may call the update
function multiple time. The old values created by the update function may be
discarded without cleaning up. In NettyClientStreamRpc.Connection the connect
method is not side-effect-free. Although it passes a [MemoizedSupplier to
getAndUpdate|https://github.com/apache/ratis/blob/47e5b6aa148cfe777fb3b7742154fe6b417ea42a/ratis-netty/src/main/java/org/apache/ratis/netty/client/NettyClientStreamRpc.java#L187-L188],
it still is possible that a value is supplied but then discarded in the
AtomicReference loop.
- --Similarly, the computeIfAbsent in ConcurrentHashMap also runs in a loop
and may call the mapping function multiple times. DataStreamMap is implemented
using a ConcurrentHashMap. However, [the mapping function is not
side-effect-free|https://github.com/apache/ratis/blob/47e5b6aa148cfe777fb3b7742154fe6b417ea42a/ratis-netty/src/main/java/org/apache/ratis/netty/server/DataStreamManagement.java#L281-L289]
although it uses a MemoizedSupplier.--
(Update: [~tanxinyu] pointed out that this is not a problem. Thanks! The
computeIfAbsent method does not run in a loop as described in the
[javadoc|https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentHashMap.html#computeIfAbsent-K-java.util.function.Function-];
see also the comments below.)
was:
- The update methods in AtomicReference run in a loop and may call the update
function multiple time. The old values created by the update function may be
discarded without cleaning up. In NettyClientStreamRpc.Connection the connect
method is not side-effect-free. Although it passes a [MemoizedSupplier to
getAndUpdate|https://github.com/apache/ratis/blob/47e5b6aa148cfe777fb3b7742154fe6b417ea42a/ratis-netty/src/main/java/org/apache/ratis/netty/client/NettyClientStreamRpc.java#L187-L188],
it still is possible that a value is supplied but then discarded in the
AtomicReference loop.
- Similarly, the computeIfAbsent in ConcurrentHashMap also runs in a loop and
may call the mapping function multiple times. DataStreamMap is implemented
using a ConcurrentHashMap. However, [the mapping function is not
side-effect-free|https://github.com/apache/ratis/blob/47e5b6aa148cfe777fb3b7742154fe6b417ea42a/ratis-netty/src/main/java/org/apache/ratis/netty/server/DataStreamManagement.java#L281-L289]
although it uses a MemoizedSupplier.
> Netty: atomic operations require side-effect-free functions
> -----------------------------------------------------------
>
> Key: RATIS-1923
> URL: https://issues.apache.org/jira/browse/RATIS-1923
> Project: Ratis
> Issue Type: Bug
> Components: Netty
> Reporter: Tsz-wo Sze
> Assignee: Tsz-wo Sze
> Priority: Major
> Fix For: 3.0.0
>
> Attachments: for-loop.jpg, screenshot-1.png
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> - The update methods in AtomicReference run in a loop and may call the update
> function multiple time. The old values created by the update function may be
> discarded without cleaning up. In NettyClientStreamRpc.Connection the connect
> method is not side-effect-free. Although it passes a [MemoizedSupplier to
> getAndUpdate|https://github.com/apache/ratis/blob/47e5b6aa148cfe777fb3b7742154fe6b417ea42a/ratis-netty/src/main/java/org/apache/ratis/netty/client/NettyClientStreamRpc.java#L187-L188],
> it still is possible that a value is supplied but then discarded in the
> AtomicReference loop.
> - --Similarly, the computeIfAbsent in ConcurrentHashMap also runs in a loop
> and may call the mapping function multiple times. DataStreamMap is
> implemented using a ConcurrentHashMap. However, [the mapping function is not
> side-effect-free|https://github.com/apache/ratis/blob/47e5b6aa148cfe777fb3b7742154fe6b417ea42a/ratis-netty/src/main/java/org/apache/ratis/netty/server/DataStreamManagement.java#L281-L289]
> although it uses a MemoizedSupplier.--
> (Update: [~tanxinyu] pointed out that this is not a problem. Thanks! The
> computeIfAbsent method does not run in a loop as described in the
> [javadoc|https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentHashMap.html#computeIfAbsent-K-java.util.function.Function-];
> see also the comments below.)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
