[Bug]: P2P replicator suffers from concurrent map race, nil iterator panic, and dropped retries on network timeout #5199

Summary

An audit of the P2P replicator subsystem (internal/db/p2p/replicator.go) revealed 4 distinct race conditions and crash bugs in replication error handling and concurrency:

1. Concurrent map read/write race in pushLogToReplicators (causes Go runtime panic)
2. Nil iterator dereference panic in retryReplicators (causes runtime panic)
3. Replication retries silently dropped on network request timeout (leads to silent data sync loss)
4. Missing mutex synchronization in handleCompletedReplicatorRetry (leads to peerstore state race conditions)

Bug 1: Concurrent Map Read/Write Race in pushLogToReplicators

Location: internal/db/p2p/replicator.go:384-426

pushLogToReplicators acquires p.repMu, grabs the inner map reference reps, and immediately releases the lock:

p.repMu.Lock()
reps, exists := p.replicators[lg.CollectionID]
p.repMu.Unlock() // lock released!

if exists {
    for peerID := range reps { // iterating live map reference without repMu..
        go func() { ... }()
    }
}

Concurrently, updateReplicators (internal/db/p2p/p2p.go:340) mutates p.replicators[collectionID] when peers join/leave or on transaction commit callbacks. Iterating reps while another goroutine writes to it causes an unrecoverable Go runtime panic:

fatal error: concurrent map read and map write

Proposed Fix: Copy peer IDs into a local slice under p.repMu.Lock() before iterating:

p.repMu.Lock()
reps, exists := p.replicators[lg.CollectionID]
var peerIDs []string
if exists {
    peerIDs = make([]string, 0, len(reps))
    for peerID := range reps {
        peerIDs = append(peerIDs, peerID)
    }
}
p.repMu.Unlock()

Bug 2: Nil Iterator Dereference Panic in retryReplicators

Location: internal/db/p2p/replicator.go:605-620

When Iterator() fails with any error other than corekv.ErrDBClosed (e.g. database error, context cancellation), the code logs the error but omits the return statement:

iter, err := p.db.Multistore().Peerstore().Iterator(ctx, corekv.IterOptions{
    Prefix: []byte(keys.REPLICATOR_RETRY_ID),
})
if err != nil {
    if errors.Is(err, corekv.ErrDBClosed) {
        return
    }
    log.ErrorContextE(ctx, "Failed iterate replicator retry ID keys", err)
    // bug: missing return here
}
defer closeQueryResults(iter)
for {
    hasNext, err := iter.Next() // iter is nil!

iter.Next() immediately panics with a nil pointer dereference:

panic: runtime error: invalid memory address or nil pointer dereference

Proposed Fix: Add return immediately after logging the error.

Bug 3: Replication Retries Dropped on Network Timeout

Location: internal/db/p2p/replicator.go:400-422

pushLogToReplicators creates a context with timeout for network requests:

ctx, cancel := context.WithTimeout(p.ctx, networkRequestTimeout)
defer cancel()
if _, err := p.replicatorProtocol.SendRequest(ctx, pushLogReq, peerID); err != nil {
    if !lg.IsRetry {
        err = p.handleReplicatorFailure(ctx, peerID, lg.DocID) // passes expired ctx
    }
}

Inside handleReplicatorFailure:

if ctx.Err() != nil {
    return ctx.Err() // immediately aborts
}

When SendRequest times out (context.DeadlineExceeded), handleReplicatorFailure is called with that already-expired context. It immediately aborts without writing the retry record to the peerstore. The most common replication failure scenario (network timeouts) silently drops retries.

Proposed Fix: Pass the long-lived node context p.ctx to handleReplicatorFailure.

Bug 4: Missing Mutex in handleCompletedReplicatorRetry

Location: internal/db/p2p/replicator.go:485-516

handleReplicatorFailure locks p.handleRetryMutex to serialize updates to peerstore retry records, but handleCompletedReplicatorRetry modifies those same retry records without acquiring p.handleRetryMutex. This leads to race conditions between concurrent push failures and retry completions.

Proposed Fix: Protect handleCompletedReplicatorRetry with p.handleRetryMutex.Lock() / defer Unlock().

Proposed Solution & Testing

We have a clean, tested fix addressing all 4 bugs in internal/db/p2p/replicator.go, accompanied by unit and race tests:

• TestPushLogToReplicators_ConcurrentMapAccess (verified clean under go test -race with 50 parallel readers and 50 parallel writers).
• TestHandleCompletedReplicatorRetry_ContextCanceled.
• Full CLI integration tests (go test ./cli/test/integration/p2p/...).

A PR will be opened shortly with the implementation and regression tests.