fix(basic_host): set read deadline before multistream Close to prevent blocking

streamWrapper.Close() can block indefinitely when the remote peer is slow
or unresponsive during the multistream-select handshake completion.

The lazy multistream protocol negotiation defers reading the handshake
response until Close() is called. If the remote peer doesn't respond,
the read blocks forever, causing goroutine leaks.

This is particularly problematic for bitswap servers where taskWorkers
can get stuck trying to close streams after sending blocks.

The fix sets a read deadline (using DefaultNegotiationTimeout) before
calling the multistream Close(), ensuring the operation will time out
rather than block indefinitely.

Related: multiformats/go-multistream#47
Related: multiformats/go-multistream#48

Author: lidel

p2p/host/basic/basic_host.go

@@ -683,6 +683,11 @@ func (s *streamWrapper) Write(b []byte) (int, error) {
 }
 
 func (s *streamWrapper) Close() error {
+	// Set a read deadline to prevent Close() from blocking indefinitely
+	// waiting for the multistream-select handshake to complete.
+	// This can happen when the remote peer is slow or unresponsive.
+	// See: https://github.com/multiformats/go-multistream/issues/47
+	_ = s.Stream.SetReadDeadline(time.Now().Add(DefaultNegotiationTimeout))
 	return s.rw.Close()
 }
 

p2p/host/basic/basic_host_test.go

@@ -785,6 +785,61 @@ func TestNegotiationCancel(t *testing.T) {
 	}
 }
 
+// TestStreamCloseDoesNotHangOnUnresponsivePeer verifies that stream.Close()
+// returns within DefaultNegotiationTimeout even when the remote peer never
+// completes the multistream handshake. Without the read deadline fix in
+// streamWrapper.Close(), this would hang indefinitely.
+func TestStreamCloseDoesNotHangOnUnresponsivePeer(t *testing.T) {
+	ctx := t.Context()
+
+	h1, h2 := getHostPair(t)
+	defer h1.Close()
+	defer h2.Close()
+
+	const testProto = "/test/hang"
+
+	// Manually add protocol to peerstore so h1 thinks h2 supports it.
+	// This makes NewStream use lazy multistream (skipping negotiation until Close).
+	h1.Peerstore().AddProtocols(h2.ID(), testProto)
+
+	// h2 accepts streams at the network level but never responds to
+	// multistream protocol negotiation, simulating an unresponsive peer.
+	h2.Network().SetStreamHandler(func(s network.Stream) {
+		// Read incoming data but never write back - simulates unresponsive peer
+		buf := make([]byte, 1024)
+		for {
+			_, err := s.Read(buf)
+			if err != nil {
+				return
+			}
+		}
+	})
+
+	// Open stream to h2 - uses lazy multistream because protocol is "known"
+	s, err := h1.NewStream(ctx, h2.ID(), testProto)
+	require.NoError(t, err)
+
+	// Trigger the lazy handshake by writing data.
+	// The write succeeds (buffered), but the read handshake will block
+	// because h2 never sends a response.
+	_, err = s.Write([]byte("trigger handshake"))
+	require.NoError(t, err)
+
+	// Close() should return within DefaultNegotiationTimeout because the fix
+	// sets a read deadline before calling the underlying Close().
+	// Without the fix, this would hang indefinitely.
+	start := time.Now()
+	_ = s.Close()
+	elapsed := time.Since(start)
+
+	// Allow some margin for test execution overhead
+	maxExpected := DefaultNegotiationTimeout + 2*time.Second
+	require.Less(t, elapsed, maxExpected,
+		"Close() took %v, expected < %v (DefaultNegotiationTimeout + margin)", elapsed, maxExpected)
+
+	t.Logf("Close() returned in %v (limit: %v)", elapsed, maxExpected)
+}
+
 func waitForAddrChangeEvent(ctx context.Context, sub event.Subscription, t *testing.T) event.EvtLocalAddressesUpdated {
 	t.Helper()
 	for {