(celeborn) branch main updated: [CELEBORN-1430] TransportClientFactory should check whether handler is null when creating client

[ethanfeng]

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

ethanfeng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/celeborn.git


The following commit(s) were added to refs/heads/main by this push:
     new b8647947c [CELEBORN-1430] TransportClientFactory should check whether 
handler is null when creating client
b8647947c is described below

commit b8647947c1552a95d3aeb97e336430fdab1c9136
Author: SteNicholas <programg...@163.com>
AuthorDate: Thu May 16 17:57:41 2024 +0800

    [CELEBORN-1430] TransportClientFactory should check whether handler is null 
when creating client
    
    ### What changes were proposed in this pull request?
    
    `TransportClientFactory` checks whether `handler` is null when creating 
client.
    
    ### Why are the changes needed?
    
    There is a case that `cachedClient.isActive()` may return true and may 
return false when checked for the second time when another thread is closing 
the channel, which causes that the `handler` may be null. Therefore, 
`TransportClientFactory` should check whether handler is null when creating 
client.
    
    Backport https://github.com/apache/spark/pull/46506.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    GA.
    
    Closes #2517 from SteNicholas/CELEBORN-1430.
    
    Authored-by: SteNicholas <programg...@163.com>
    Signed-off-by: mingji <fengmingxiao....@alibaba-inc.com>
---
 .../celeborn/common/network/client/TransportClientFactory.java      | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git 
a/common/src/main/java/org/apache/celeborn/common/network/client/TransportClientFactory.java
 
b/common/src/main/java/org/apache/celeborn/common/network/client/TransportClientFactory.java
index a712965a4..f9943ea9a 100644
--- 
a/common/src/main/java/org/apache/celeborn/common/network/client/TransportClientFactory.java
+++ 
b/common/src/main/java/org/apache/celeborn/common/network/client/TransportClientFactory.java
@@ -154,8 +154,10 @@ public class TransportClientFactory implements Closeable {
       // this code was able to update things.
       TransportChannelHandler handler =
           
cachedClient.getChannel().pipeline().get(TransportChannelHandler.class);
-      synchronized (handler) {
-        handler.getResponseHandler().updateTimeOfLastRequest();
+      if (handler != null) {
+        synchronized (handler) {
+          handler.getResponseHandler().updateTimeOfLastRequest();
+        }
       }
 
       if (cachedClient.isActive()) {