Skip to content

Commit 45b8047

Browse files
Anai-GuoAnai-Guo
andauthored
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 <antai12232931@anaiguo.com> Co-authored-by: Anai-Guo <antai12232931@anaiguo.com>
1 parent fd0d1b9 commit 45b8047

1 file changed

Lines changed: 25 additions & 16 deletions

File tree

core/application/p2p.go

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,26 @@ import (
1818
)
1919

2020
func (a *Application) StopP2P() error {
21-
if a.p2pCancel != nil {
22-
a.p2pCancel()
23-
a.p2pCancel = nil
24-
a.p2pCtx = nil
25-
// Wait a bit for shutdown to complete
26-
time.Sleep(200 * time.Millisecond)
27-
}
21+
a.p2pMutex.Lock()
22+
defer a.p2pMutex.Unlock()
23+
24+
a.stopP2PLocked()
2825
return nil
2926
}
3027

28+
// stopP2PLocked cancels the running P2P stack, if any, and clears the
29+
// context fields. Callers must hold a.p2pMutex.
30+
func (a *Application) stopP2PLocked() {
31+
if a.p2pCancel == nil {
32+
return
33+
}
34+
a.p2pCancel()
35+
a.p2pCancel = nil
36+
a.p2pCtx = nil
37+
// Wait a bit for shutdown to complete
38+
time.Sleep(200 * time.Millisecond)
39+
}
40+
3141
func (a *Application) StartP2P() error {
3242
// we need a p2p token
3343
if a.applicationConfig.P2PToken == "" {
@@ -37,8 +47,13 @@ func (a *Application) StartP2P() error {
3747
networkID := a.applicationConfig.P2PNetworkID
3848

3949
ctx, cancel := context.WithCancel(a.ApplicationConfig().Context)
50+
// Publishing the context is the only shared-state write here; the node
51+
// and discoverer setup below works off the local ctx/cancel, so the lock
52+
// is not held across the network calls.
53+
a.p2pMutex.Lock()
4054
a.p2pCtx = ctx
4155
a.p2pCancel = cancel
56+
a.p2pMutex.Unlock()
4257

4358
var n *node.Node
4459
// Here we are avoiding creating multiple nodes:
@@ -131,13 +146,7 @@ func (a *Application) RestartP2P() error {
131146
defer a.p2pMutex.Unlock()
132147

133148
// Stop existing P2P if running
134-
if a.p2pCancel != nil {
135-
a.p2pCancel()
136-
a.p2pCancel = nil
137-
a.p2pCtx = nil
138-
// Wait a bit for shutdown to complete
139-
time.Sleep(200 * time.Millisecond)
140-
}
149+
a.stopP2PLocked()
141150

142151
appConfig := a.ApplicationConfig()
143152

@@ -151,8 +160,8 @@ func (a *Application) RestartP2P() error {
151160
go func() {
152161
if err := a.StartP2P(); err != nil {
153162
xlog.Error("Failed to start P2P stack", "error", err)
154-
if a.p2pCancel != nil {
155-
a.p2pCancel()
163+
if stopErr := a.StopP2P(); stopErr != nil {
164+
xlog.Error("Failed to stop partially started P2P stack", "error", stopErr)
156165
}
157166
}
158167
}()

0 commit comments

Comments
 (0)