Hacktoberfest 2026：メンテナが10月に向けて印を付けた、オープンで初心者向けの issue。 Hacktoberfest の issue を見る

bufio.CachedConn: data race on the cached buffer between Read/ReaderReplaceable and Close

オープン
  • 何か問題がありますか？
#112 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
55/100
issue の種類
バグ
明瞭さ
おおむね明確
活発さ
活発
技術スタック
go
領域
networking

調査の方向性

Start in common/bufio/cache.go with CachedConn.Read, ReaderReplaceable, Close, and the corresponding CachedPacketConn methods. Reproduce the reported concurrent paths with the race detector, including the routing and DNS-over-TCP entry points named in the issue. Done means the cache has one consistent ownership path, without races or double release.

索引モデルが issue の本文から書いたものです。

説明

sing v0.8.10 (via sing-box v1.13.12), go 1.26, -race

CachedConn.buffer is read and cleared without synchronization in Read and ReaderReplaceable, while Close (usually called from another goroutine) reads and clears it too. Close guards itself with taken, but Read / ReaderReplaceable don't check or set taken:

// common/bufio/cache.go
func (c *CachedConn) Read(p []byte) (n int, err error) {
	if c.buffer != nil {              // read
		...
		c.buffer.DecRef()
		c.buffer.Release()
		c.buffer = nil                // write
	}
	return c.Conn.Read(p)
}

func (c *CachedConn) ReaderReplaceable() bool {
	return c.buffer == nil            // read
}

func (c *CachedConn) Close() error {
	if c.taken.CompareAndSwap(false, true) {
		if buffer := c.buffer; buffer != nil {   // read (races with Read)
			c.buffer = nil
			buffer.DecRef()
			buffer.Release()          // can release a buffer Read already released
		}
	}
	return c.Conn.Close()
}

CachedPacketConn.ReadPacket / Close have the same pattern.

Besides the race report, the worst case is the pooled buffer being released twice, so it can come back from buf.New to another connection while still referenced.

Where it shows up in sing-box 1.13.12 (both found with -race):

  1. Routing a sniffed TCP connection: route.(*Router).routeConnection wraps it in bufio.NewCachedConn; ConnectionManager.connectionCopy then calls ReaderReplaceable (via N.UnwrapCountReader in bufio.CopyWithCounters) on one goroutine, while the other copy direction calls Close (route/conn.go:262 vs :271).
  2. Hijacked DNS over TCP: protocol/dns.HandleStreamDNSRequest reads the next query from the conn (handle.go:24) while the exchange goroutine of the previous query calls conn.Close() on error (handle.go:46).

When the wrapper can be unwrapped before copying, ReadCached (which does use taken) avoids it, but callers such as a ConnectionTracker that wraps the conn force the unsynchronized Read path.

Suggested fix: make every consumer of the cache take it through the same atomic flag, e.g.

func (c *CachedConn) Read(p []byte) (n int, err error) {
	if !c.taken.Load() {
		c.mu.Lock() // or take the buffer once with CompareAndSwap and keep it reader-local
		...
	}
	return c.Conn.Read(p)
}

or have Read take the buffer once (CompareAndSwap(false, true)), move it into a reader-owned field, and never touch it from Close after that. ReaderReplaceable would then read an atomic. Happy to send a PR if the approach sounds right.

主要言語
Go
スター
131
フォーク
116
平均マージ
1時間 42分
マージ済み PR（30日）
1

環境構築

このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

SagerNet/sing のほかの issue

SagerNet/sing の issue をすべて見る

似ている issue

Go の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。