(kudu) branch master updated: [Java] Fix concurrent problem while traversing the traces of KuduRpc.

[alexey]

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new f03de689b [Java] Fix concurrent problem while traversing the traces of 
KuduRpc.
f03de689b is described below

commit f03de689b2e138f4eb3a2501a4d037f90b00b2ae
Author: 宋家成 <songjiach...@thinkingdata.cn>
AuthorDate: Tue Feb 20 14:56:27 2024 +0800

    [Java] Fix concurrent problem while traversing the traces of KuduRpc.
    
    When the RPC request which is sent by java client times out, the
    RpcTimeoutTask is called by HashedWheelTimer. During the running
    of this task, KuduRpc#toString() is called. This method will traverse
    the traces(a synchronizedList) without any protection, which leads to
    exception in timeout task and then the callback will not be triggered.
    
    The exception message is as follow:
    
    An exception was thrown by TimerTask.
    java.util.ConcurrentModificationException: null
            at java.util.ArrayList$Itr.checkForComodification
            at java.util.ArrayList$Itr.next
            at org.apache.kudu.client.RpcTraceFrame
               .getHumanReadableSummaryStringForTraces
            at org.apache.kudu.client.KuduRpc.toString
            at java.lang.String.valueOf
            at java.lang.StringBuilder.append
            at org.apache.kudu.client.KuduRpc$RpcTimeoutTask.run
            at org.apache.kudu.shaded.io.netty.util
               .HashedWheelTimer$HashedWheelTimeout.expire
    
    So make a copy of the traces and then traverse the copy. This method
    is mostly called when some exceptions are raised, so the influence on
    performance should be acceptable.
    
    Change-Id: I1a642d93d01ebcfa05b01fe263023b5580d2542b
    Reviewed-on: http://gerrit.cloudera.org:8080/21041
    Reviewed-by: Alexey Serbin <ale...@apache.org>
    Tested-by: Alexey Serbin <ale...@apache.org>
---
 .../src/main/java/org/apache/kudu/client/KuduRpc.java        | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java 
b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
index 3aac537bc..a80a4a05c 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
@@ -403,11 +403,19 @@ public abstract class KuduRpc<R> {
     buf.append(", ").append(timeoutTracker);
     // Cheating a bit, we're not actually logging but we'll augment the 
information provided by
     // this method if DEBUG is enabled.
+    //
+    // Lock the traces array and get a copy of it before traverse the traces. 
This method is
+    // mostly called when some exceptions are raised, so the influence of 
performance should be
+    // acceptable.
+    List<RpcTraceFrame> tracesCopy;
+    synchronized (traces) {
+      tracesCopy = new ArrayList<>(traces);
+    }
     if (LOG.isDebugEnabled()) {
-      buf.append(", 
").append(RpcTraceFrame.getHumanReadableStringForTraces(traces));
+      buf.append(", 
").append(RpcTraceFrame.getHumanReadableStringForTraces(tracesCopy));
       buf.append(", deferred=").append(deferred);
     } else {
-      buf.append(", 
").append(RpcTraceFrame.getHumanReadableSummaryStringForTraces(traces));
+      buf.append(", 
").append(RpcTraceFrame.getHumanReadableSummaryStringForTraces(tracesCopy));
     }
     buf.append(')');
     return buf.toString();