From 8bd797b6c6aa66a70171fc010794432938b0b7c4 Mon Sep 17 00:00:00 2001 From: wnqqnw19 Date: Thu, 4 Dec 2025 12:27:35 -0800 Subject: [PATCH] fix: resolve race conditions, add bounds checks, and remove redundant code --- eth/handler.go | 17 +++++++++++------ eth/tracers/tracker.go | 18 +++++++++++++++++- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/eth/handler.go b/eth/handler.go index ff970e2ba6..f9786f980a 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -316,6 +316,11 @@ func (h *handler) runEthPeer(peer *eth.Peer, handler eth.Handler) error { defer close(dead) // If we have any explicit peer required block hashes, request them + // Capture peer ID and logger to avoid race conditions in goroutines + peerID := peer.ID() + peerLogger := peer.Log() + peerAddr := peer.RemoteAddr() + peerName := peer.Name() for number, hash := range h.requiredBlocks { resCh := make(chan *eth.Response) @@ -323,7 +328,7 @@ func (h *handler) runEthPeer(peer *eth.Peer, handler eth.Handler) error { if err != nil { return err } - go func(number uint64, hash common.Hash, req *eth.Request) { + go func(number uint64, hash common.Hash, req *eth.Request, id string, logger log.Logger, addr string, name string) { // Ensure the request gets cancelled in case of error/drop defer req.Close() @@ -345,19 +350,19 @@ func (h *handler) runEthPeer(peer *eth.Peer, handler eth.Handler) error { return } if headers[0].Number.Uint64() != number || headers[0].Hash() != hash { - peer.Log().Info("Required block mismatch, dropping peer", "number", number, "hash", headers[0].Hash(), "want", hash) + logger.Info("Required block mismatch, dropping peer", "number", number, "hash", headers[0].Hash(), "want", hash) res.Done <- errors.New("required block mismatch") return } - peer.Log().Debug("Peer required block verified", "number", number, "hash", hash) + logger.Debug("Peer required block verified", "number", number, "hash", hash) res.Done <- nil case <-timeout.C: - peer.Log().Warn("Required block challenge timed out, dropping", "addr", peer.RemoteAddr(), "type", peer.Name()) - h.removePeer(peer.ID()) + logger.Warn("Required block challenge timed out, dropping", "addr", addr, "type", name) + h.removePeer(id) case <-dead: // Peer handler terminated, abort all goroutines } - }(number, hash, req) + }(number, hash, req, peerID, peerLogger, peerAddr, peerName) } // Handle incoming messages until the connection is torn down return handler(peer) diff --git a/eth/tracers/tracker.go b/eth/tracers/tracker.go index 136be37f5c..9a8a017448 100644 --- a/eth/tracers/tracker.go +++ b/eth/tracers/tracker.go @@ -52,10 +52,26 @@ func (t *stateTracker) releaseState(number uint64, release StateReleaseFunc) { t.lock.Lock() defer t.lock.Unlock() + // Validate that the state number is within the expected range + if number < t.oldest { + // This should not happen in normal operation, but handle gracefully + // by just appending the release function without updating the used array + t.releases = append(t.releases, release) + return + } + + // Calculate the index and ensure it's within bounds + index := int(number - t.oldest) + if index < 0 || index >= len(t.used) { + // State is outside the tracking window, just append the release function + t.releases = append(t.releases, release) + return + } + // Set the state as used, the corresponding flag is indexed by // the distance between the specified state and the oldest state // which is still using for trace. - t.used[int(number-t.oldest)] = true + t.used[index] = true // If the oldest state is used up, update the oldest marker by moving // it to the next state which is not used up.