Repository: libp2p/go-libp2p
Pull request #2722 (2024-03-02 20:27:06)

Small code improvements
Here are a few small changes.
Happy to do 3 PRs if you want a single one per change.

The only actual code change that is reachable is that I have now interlocked the Locks in the stream `spawnControlMessageReader` function. 
This could be an issue if another goroutine were to lock the stream lock and wait on being able to lock on the reader lock before releasing the stream lock, but if that were the case, it would also mean that the previous code didn't get the guarantee no other function was able to "do reader.ReadMsg" as the comment says.
This is the reason why we should Lock the stream mutex before unlocking the reader one, IMO.


Patch:
diff --git a/p2p/discovery/backoff/backoffcache.go b/p2p/discovery/backoff/backoffcache.go
index 82c07241..c8f11802 100644
--- a/p2p/discovery/backoff/backoffcache.go
+++ b/p2p/discovery/backoff/backoffcache.go
@@ -82,14 +82,6 @@ func (c realClock) Now() time.Time {
 	return time.Now()
 }
 
-// withClock lets you override the default time.Now() call. Useful for tests.
-func withClock(c clock) BackoffDiscoveryOption {
-	return func(b *BackoffDiscovery) error {
-		b.clock = c
-		return nil
-	}
-}
-
 type backoffCache struct {
 	// strat is assigned on creation and not written to
 	strat BackoffStrategy
diff --git a/p2p/host/resource-manager/scope.go b/p2p/host/resource-manager/scope.go
index 60089c3a..bb70c100 100644
--- a/p2p/host/resource-manager/scope.go
+++ b/p2p/host/resource-manager/scope.go
@@ -118,12 +118,10 @@ func (rc *resources) checkMemory(rsvp int64, prio uint8) error {
 	threshold, mulOk := mulInt64WithOverflow(1+int64(prio), limit)
 	if !mulOk {
 		thresholdBig := big.NewInt(limit)
-		thresholdBig = thresholdBig.Mul(thresholdBig, big.NewInt(1+int64(prio)))
+		thresholdBig.Mul(thresholdBig, big.NewInt(1+int64(prio)))
 		thresholdBig.Rsh(thresholdBig, 8) // Divide 256
-		if !thresholdBig.IsInt64() {
-			// Shouldn't happen since the threshold can only be <= limit
-			threshold = limit
-		}
+		// necessarily a Int64 since we multiplied a int64 != MaxInt64 with
+		// a uint8+1 (max 255+1 = 256) and divided by 256
 		threshold = thresholdBig.Int64()
 	} else {
 		threshold = threshold / 256
diff --git a/p2p/protocol/ping/ping.go b/p2p/protocol/ping/ping.go
index 6ff5c3fb..1c782290 100644
--- a/p2p/protocol/ping/ping.go
+++ b/p2p/protocol/ping/ping.go
@@ -158,11 +158,10 @@ func Ping(ctx context.Context, h host.Host, p peer.ID) <-chan Result {
 			}
 		}
 	}()
-	go func() {
+	context.AfterFunc(ctx, func() {
 		// forces the ping to abort.
-		<-ctx.Done()
 		s.Reset()
-	}()
+	})
 
 	return out
 }
diff --git a/p2p/transport/webrtc/stream.go b/p2p/transport/webrtc/stream.go
index 45641321..135940e4 100644
--- a/p2p/transport/webrtc/stream.go
+++ b/p2p/transport/webrtc/stream.go
@@ -243,17 +243,15 @@ func (s *stream) spawnControlMessageReader() {
 			s.setDataChannelReadDeadline(time.Now().Add(-1 * time.Hour))
 
 			s.readerMx.Lock()
-			// We have the lock any readers blocked on reader.ReadMsg have exited.
+			// We have the lock: any readers blocked on reader.ReadMsg have exited.
+			s.mx.Lock()
+			defer s.mx.Unlock()
 			// From this point onwards only this goroutine will do reader.ReadMsg.
-
-			//lint:ignore SA2001 we just want to ensure any exising readers have exited.
+			// We just wanted to ensure any exising readers have exited.
 			// Read calls from this point onwards will exit immediately on checking
 			// s.readState
 			s.readerMx.Unlock()
 
-			s.mx.Lock()
-			defer s.mx.Unlock()
-
 			if s.nextMessage != nil {
 				s.processIncomingFlag(s.nextMessage.Flag)
 				s.nextMessage = nil


Test patch:
diff --git a/p2p/discovery/backoff/backoffcache_test.go b/p2p/discovery/backoff/backoffcache_test.go
index fe528cca..6679415a 100644
--- a/p2p/discovery/backoff/backoffcache_test.go
+++ b/p2p/discovery/backoff/backoffcache_test.go
@@ -80,6 +80,14 @@ func assertNumPeersWithLimit(t *testing.T, ctx context.Context, d discovery.Disc
 	}
 }
 
+// withClock lets you override the default time.Now() call. Useful for tests.
+func withClock(c clock) BackoffDiscoveryOption {
+	return func(b *BackoffDiscovery) error {
+		b.clock = c
+		return nil
+	}
+}
+
 func TestBackoffDiscoverySingleBackoff(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()