From 154c19403ad2c32c5cb177efb61d9b24eeb0d5c4 Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Tue, 9 Jun 2026 10:41:05 -0300 Subject: [PATCH] fix(chisel): release a lock earlier to avoid a deadlock and clean stale tunnels immediately BE-13050 (#2815) --- api/chisel/service.go | 4 ++++ api/chisel/service_test.go | 18 ++++++++++++++++++ api/chisel/tunnel.go | 17 ++++++++++------- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/api/chisel/service.go b/api/chisel/service.go index 18d2359e39..e1b4e44499 100644 --- a/api/chisel/service.go +++ b/api/chisel/service.go @@ -306,6 +306,10 @@ func (service *Service) snapshotAndLog(endpointID portainer.EndpointID, tunnelPo Err(err). Msg("unable to snapshot Edge environment") + if service.dataStore.IsErrObjectNotFound(err) { + service.close(endpointID) + } + return false } diff --git a/api/chisel/service_test.go b/api/chisel/service_test.go index ec70435a85..fb923b8d90 100644 --- a/api/chisel/service_test.go +++ b/api/chisel/service_test.go @@ -187,6 +187,24 @@ func TestCheckTunnelsKeepsHasSnapshotFalseOnSnapshotFailure(t *testing.T) { require.False(t, s.activeTunnels[endpoint.ID].HasSnapshot, "HasSnapshot must stay false after failure") } +func TestCheckTunnelsClosesStaleEntryForDeletedEndpoint(t *testing.T) { + t.Parallel() + + _, store := datastore.MustNewTestStore(t, false, true) + + // Endpoint is not created in the store, simulates deletion while tunnel stays open. + s := NewService(store, nil, nil) + s.activeTunnels[1] = &portainer.TunnelDetails{ + Status: portainer.EdgeAgentManagementRequired, + Port: 50010, + LastActivity: time.Now(), + } + + s.checkTunnels() + + require.Nil(t, s.activeTunnels[1], "stale tunnel for deleted endpoint must be removed immediately") +} + func TestCheckTunnelsClosesIdleTunnelAndSnapshots(t *testing.T) { t.Parallel() diff --git a/api/chisel/tunnel.go b/api/chisel/tunnel.go index 712d691fea..7fe1dfcabe 100644 --- a/api/chisel/tunnel.go +++ b/api/chisel/tunnel.go @@ -82,17 +82,24 @@ func (s *Service) Open(endpoint *portainer.Endpoint) error { return nil } -// close removes the tunnel from the map so the agent will close it +// close removes the tunnel from the map so the agent will close it. +// The lock is released before cleaning up the chisel user and proxy to avoid +// blocking Config/Open callers while DeleteUser interacts with chisel internals. func (s *Service) close(endpointID portainer.EndpointID) { s.mu.Lock() - defer s.mu.Unlock() tun, ok := s.activeTunnels[endpointID] if !ok { + s.mu.Unlock() return } - if len(tun.Credentials) > 0 && s.chiselServer != nil { + delete(s.activeTunnels, endpointID) + cache.Del(endpointID) + + s.mu.Unlock() + + if s.chiselServer != nil { user, _, _ := strings.Cut(tun.Credentials, ":") s.chiselServer.DeleteUser(user) } @@ -100,10 +107,6 @@ func (s *Service) close(endpointID portainer.EndpointID) { if s.ProxyManager != nil { s.ProxyManager.DeleteEndpointProxy(endpointID) } - - delete(s.activeTunnels, endpointID) - - cache.Del(endpointID) } // Config returns the tunnel details needed for the agent to connect