From 45b80477369446c77fe9efe61acf647c4c7a0c87 Mon Sep 17 00:00:00 2001 From: Tai An Date: Fri, 17 Jul 2026 00:02:05 -0700 Subject: [PATCH] fix(p2p): serialize access to p2pCtx/p2pCancel (#10839) (#10861) StopP2P() read and wrote a.p2pCtx/a.p2pCancel without holding a.p2pMutex, and StartP2P() reassigned both fields with no lock at all -- including when RestartP2P() calls it from a background goroutine after releasing the mutex. Both paths are reachable from POST /api/settings (empty p2p_token -> StopP2P, non-empty -> RestartP2P), so concurrent requests race on the same fields. Take a.p2pMutex in StopP2P and around the field publication in StartP2P, factor the shared teardown into stopP2PLocked() so RestartP2P reuses it, and route the goroutine error path through StopP2P instead of touching a.p2pCancel unlocked. Signed-off-by: Anai-Guo Co-authored-by: Anai-Guo --- core/application/p2p.go | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/core/application/p2p.go b/core/application/p2p.go index 451e38121..d39409a94 100644 --- a/core/application/p2p.go +++ b/core/application/p2p.go @@ -18,16 +18,26 @@ import ( ) func (a *Application) StopP2P() error { - if a.p2pCancel != nil { - a.p2pCancel() - a.p2pCancel = nil - a.p2pCtx = nil - // Wait a bit for shutdown to complete - time.Sleep(200 * time.Millisecond) - } + a.p2pMutex.Lock() + defer a.p2pMutex.Unlock() + + a.stopP2PLocked() return nil } +// stopP2PLocked cancels the running P2P stack, if any, and clears the +// context fields. Callers must hold a.p2pMutex. +func (a *Application) stopP2PLocked() { + if a.p2pCancel == nil { + return + } + a.p2pCancel() + a.p2pCancel = nil + a.p2pCtx = nil + // Wait a bit for shutdown to complete + time.Sleep(200 * time.Millisecond) +} + func (a *Application) StartP2P() error { // we need a p2p token if a.applicationConfig.P2PToken == "" { @@ -37,8 +47,13 @@ func (a *Application) StartP2P() error { networkID := a.applicationConfig.P2PNetworkID ctx, cancel := context.WithCancel(a.ApplicationConfig().Context) + // Publishing the context is the only shared-state write here; the node + // and discoverer setup below works off the local ctx/cancel, so the lock + // is not held across the network calls. + a.p2pMutex.Lock() a.p2pCtx = ctx a.p2pCancel = cancel + a.p2pMutex.Unlock() var n *node.Node // Here we are avoiding creating multiple nodes: @@ -131,13 +146,7 @@ func (a *Application) RestartP2P() error { defer a.p2pMutex.Unlock() // Stop existing P2P if running - if a.p2pCancel != nil { - a.p2pCancel() - a.p2pCancel = nil - a.p2pCtx = nil - // Wait a bit for shutdown to complete - time.Sleep(200 * time.Millisecond) - } + a.stopP2PLocked() appConfig := a.ApplicationConfig() @@ -151,8 +160,8 @@ func (a *Application) RestartP2P() error { go func() { if err := a.StartP2P(); err != nil { xlog.Error("Failed to start P2P stack", "error", err) - if a.p2pCancel != nil { - a.p2pCancel() + if stopErr := a.StopP2P(); stopErr != nil { + xlog.Error("Failed to stop partially started P2P stack", "error", stopErr) } } }()