From dd722be08f492c4d60834b85395d9b121e7d5765 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Wed, 15 Jan 2025 23:37:53 +0000 Subject: [PATCH 1/2] xdsclient: release lock before attempting to close underlying transport --- xds/internal/xdsclient/client_refcounted.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/xds/internal/xdsclient/client_refcounted.go b/xds/internal/xdsclient/client_refcounted.go index 5a256c1bfada..3d5ef1503ce9 100644 --- a/xds/internal/xdsclient/client_refcounted.go +++ b/xds/internal/xdsclient/client_refcounted.go @@ -40,19 +40,23 @@ var ( func clientRefCountedClose(name string) { clientsMu.Lock() - defer clientsMu.Unlock() - client, ok := clients[name] if !ok { logger.Errorf("Attempt to close a non-existent xDS client with name %s", name) + clientsMu.Unlock() return } if client.decrRef() != 0 { + clientsMu.Unlock() return } + delete(clients, name) + clientsMu.Unlock() + + // This attempts to close the transport to the management server and could + // take a while to complete. So, call it without holding the lock. client.clientImpl.close() xdsClientImplCloseHook(name) - delete(clients, name) } From 3fdb7c5531664c6b97565d92550e9e2d72dc6e80 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Thu, 16 Jan 2025 02:13:23 +0000 Subject: [PATCH 2/2] improve comment --- xds/internal/xdsclient/client_refcounted.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xds/internal/xdsclient/client_refcounted.go b/xds/internal/xdsclient/client_refcounted.go index 3d5ef1503ce9..f5fc76d8a75c 100644 --- a/xds/internal/xdsclient/client_refcounted.go +++ b/xds/internal/xdsclient/client_refcounted.go @@ -54,7 +54,8 @@ func clientRefCountedClose(name string) { clientsMu.Unlock() // This attempts to close the transport to the management server and could - // take a while to complete. So, call it without holding the lock. + // theoretically call back into the xdsclient package again and deadlock. + // Hence, this needs to be called without holding the lock. client.clientImpl.close() xdsClientImplCloseHook(name)