AcceptStream still sets a finalizer that closes the stream mid-transfer — silent data loss on accepted streams

[jisung-02]

Session.AcceptStream() registers a finalizer on the *Stream wrapper it returns:

wrapper := &Stream{stream: stream}
runtime.SetFinalizer(wrapper, func(s *Stream) {
    s.Close()
})

OpenStream() has this same finalizer disabled (commented out with a NOTE(x): disabled finalizer remark), but AcceptStream() still has it active.

When an accepted stream is consumed with io.Copy / io.CopyBuffer, the copy dispatches to WriteTo promoted from the embedded *stream, and during that call nothing keeps the outer *Stream wrapper alive. GC collects it, the finalizer closes the stream mid-transfer, and PSH frames that arrive afterwards are dropped on recvLoop's closed-stream path. The receiver just comes up short — no error anywhere. The usual relay boilerplate is exactly this condition:

stream, _ := session.AcceptStream()
go io.Copy(dst, stream)

Repro

Public API only, real TCP loopback. Drop this into an empty module, go get github.com/xtaci/smux@master, then go test -run TestAcceptedStreamDataLossUnderGC -v -count=3 .. debug.SetGCPercent(1) is only there to make GC timing deterministic.

package repro

import (
      "io"
      "net"
      "runtime/debug"
      "testing"
      "time"

      "github.com/xtaci/smux"
)

func TestAcceptedStreamDataLossUnderGC(t *testing.T) {
      old := debug.SetGCPercent(1)
      defer debug.SetGCPercent(old)

      ln, _ := net.Listen("tcp", "localhost:0")
      defer ln.Close()

      const (
              chunkSize  = 64
              chunkCount = 200_000
              want       = chunkSize * chunkCount
      )

      received := make(chan int64, 1)
      accepted := make(chan error, 1)

      var srvConn net.Conn
      var srvSess *smux.Session
      go func() {
              srvConn, _ = ln.Accept()
              srvSess, _ = smux.Server(srvConn, nil)
              ss, err := srvSess.AcceptStream()
              accepted <- err
              if err != nil {
                      return
              }
              n, _ := io.Copy(io.Discard, ss)
              received <- n
      }()

      conn, err := net.Dial("tcp", ln.Addr().String())
      if err != nil {
              t.Fatal(err)
      }
      defer conn.Close()
      sess, err := smux.Client(conn, nil)
      if err != nil {
              t.Fatal(err)
      }
      defer sess.Close()
      cs, err := sess.OpenStream()
      if err != nil {
              t.Fatal(err)
      }
      if err := <-accepted; err != nil {
              t.Fatal(err)
      }

      buf := make([]byte, chunkSize)
      for i := 0; i < chunkCount; i++ {
              if _, err := cs.Write(buf); err != nil {
                      t.Fatalf("write chunk %d: %v", i, err)
              }
      }
      cs.Close()

      select {
      case n := <-received:
              if srvSess != nil {
                      srvSess.Close()
              }
              if srvConn != nil {
                      srvConn.Close()
              }
              if n != int64(want) {
                      t.Fatalf("sent %d bytes, receiver got %d (missing %d)", want, n, int64(want)-n)
              }
      case <-time.After(30 * time.Second):
              t.Fatal("timed out waiting for server-side copy")
      }
}

Fails every run on current master; the loss point moves around with GC timing:

--- FAIL: TestAcceptedStreamDataLossUnderGC  sent 12800000 bytes, receiver got 68224
--- FAIL: TestAcceptedStreamDataLossUnderGC  sent 12800000 bytes, receiver got 82176
--- FAIL: TestAcceptedStreamDataLossUnderGC  sent 12800000 bytes, receiver got 81600

This is not an artifact of touching GC settings: with default GOGC and nothing but a goroutine doing ordinary allocations alongside, a 64MB transfer through the same relay pattern lost data in 5 out of 5 runs — one run delivered 23488 bytes out of 67108864. I can post that standalone program too if useful.

Fix

Disable the finalizer in AcceptStream() the same way it's already disabled in OpenStream(). With that change the repro passes under the same GC pressure (verified locally). Streams the caller never closes still get cleaned up by Session.Close() walking s.streams, the same trade-off already accepted for OpenStream.

I'd like to send a PR for this with a regression test included.

Environment: go1.25.7 darwin/arm64, smux @ current master