[インデックス 10760] ファイルの概要

このコミットは、以前のコミット CL 5414048 (ハッシュ f6b994f33cf4) を元に戻す（revert）ものです。元のコミットは、HTTPサーバーがパニックを起こした際にスタックトレースが確実に表示されるように、接続を閉じる処理の順序を変更しようとしましたが、この変更がビルドを壊す（"breaks build"）という問題を引き起こしたため、元の状態に戻されました。

コミット

コミットハッシュ: 2c6d3eaf78c9314fe49a550e765def95463179e8
作者: Russ Cox rsc@golang.org
日付: 2011年12月13日火曜日 17:08:18 -0500

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/2c6d3eaf78c9314fe49a550e765def95463179e8

元コミット内容

undo CL 5414048 / f6b994f33cf4

breaks build

««« original CL description
http: close connection after printing panic stack trace
In a testing situation, it's possible for a local http
server to panic and the test exit without the stack trace
ever being printed.
Fixes #2480.

R=rsc, bradfitz
CC=golang-dev
https://golang.org/cl/5414048

»»»

R=bradfitz
CC=golang-dev
https://golang.org/cl/5482061

このコミットは、以前に適用されたコミット CL 5414048 を元に戻すために作成されました。元のコミットの目的は、Goのnet/httpパッケージで実装されたHTTPサーバーがパニック（予期せぬエラー）を起こした際に、そのスタックトレースが確実にログに出力されるようにすることでした。特にテスト環境において、サーバーがパニックした直後にテストプロセスが終了してしまうと、スタックトレースがコンソールに表示される前に接続が閉じられ、情報が失われる可能性がありました。これを解決するため、元のコミットでは接続を閉じる処理（c.rwc.Close()）をスタックトレースの出力（log.Print(buf.String())）よりも後に移動させました。

しかし、この「修正」が「ビルドを壊す」（"breaks build"）という予期せぬ副作用を引き起こしたため、このコミットによってその変更が元に戻されることになりました。具体的にどのようなビルドエラーが発生したのかはコミットメッセージからは不明ですが、元の変更がGoのビルドシステムや他の部分と互換性がなかったか、あるいは新たなバグを導入した可能性が考えられます。

前提知識の解説

このコミットの理解には、以下のGo言語およびネットワークプログラミングに関する知識が役立ちます。

Go言語のpanicとrecover:
- panicは、Goプログラムが回復不能なエラーに遭遇した際に、通常の実行フローを中断してスタックを巻き戻すメカニズムです。Goでは、致命的なエラーやプログラマーの想定外の状況でpanicが使用されます。
- deferステートメントは、関数がリターンする直前（またはpanicによってスタックが巻き戻される際）に実行される関数をスケジュールします。
- recoverは、deferされた関数内でpanicから回復するために使用されます。recoverが呼び出されると、panicの値が返され、プログラムの実行を再開できます。このコミットでは、defer内でrecoverを使ってパニックを捕捉し、エラー情報をログに出力しています。
net/httpパッケージ:
- Goの標準ライブラリに含まれるHTTPクライアントおよびサーバーの実装を提供するパッケージです。
- http.ServerはHTTPリクエストを処理するためのサーバーを提供します。
- http.conn構造体は、個々のHTTP接続を表し、クライアントとの間でデータの読み書きを行います。c.rwcは、この接続に関連付けられたio.ReadWriteCloserインターフェース（通常はTCP接続）を指します。
debug.Stack():
- runtime/debugパッケージに含まれる関数で、現在のゴルーチンのスタックトレースをバイトスライスとして返します。これは、パニック発生時やデバッグ時にプログラムの実行パスを追跡するために非常に有用です。
log.Print():
- logパッケージに含まれる関数で、標準エラー出力（または設定された出力先）にメッセージをログとして出力します。
CL (Change List):
- Goプロジェクトでは、Gerritというコードレビューシステムが使用されており、CLは「Change List」の略で、単一の変更セット（コミット）を指します。https://golang.org/cl/のURLは、Gerrit上の特定の変更へのリンクです。

技術的詳細

このコミットの技術的詳細は、src/pkg/net/http/server.goファイルの(*conn).serve()メソッド内のdeferブロックに焦点を当てています。

(*conn).serve()メソッドは、個々のHTTP接続を処理するゴルーチン内で実行されます。このメソッドの冒頭には、以下のようなdefer関数が設定されています。

func (c *conn) serve() {
	defer func() {
		if err := recover(); err != nil {
			// ... パニック処理 ...
		}
		// ... その他のクリーンアップ ...
	}()
	// ... 実際のHTTPリクエスト処理 ...
}

このdefer関数は、serve()メソッドが正常に終了するか、またはpanicによって中断された場合に実行されます。recover()がnilでない値を返した場合、それはserve()メソッド内でパニックが発生したことを意味します。

パニックが捕捉された際、元のコミット（f6b994f33cf4）では、スタックトレースをログに出力する前にc.rwc.Close()（接続を閉じる処理）を移動させました。

元のコミット（f6b994f33cf4）での変更点:

 // ...
 		var buf bytes.Buffer
 		fmt.Fprintf(&buf, "http: panic serving %v: %v\\n", c.remoteAddr, err)
 		buf.Write(debug.Stack())
 		log.Print(buf.String())

-		if c.rwc != nil { // may be nil if connection hijacked
-			c.rwc.Close()
-		}
 	}()

 	if tlsConn, ok := c.rwc.(*tls.Conn); ok {

↓

 // ...
 		if err == nil {
 			return
 		}
+		if c.rwc != nil { // may be nil if connection hijacked
+			c.rwc.Close()
+		}
+
 		var buf bytes.Buffer
 		fmt.Fprintf(&buf, "http: panic serving %v: %v\\n", c.remoteAddr, err)
 		buf.Write(debug.Stack())
 		log.Print(buf.String())
 	}()

 	if tlsConn, ok := c.rwc.(*tls.Conn); ok {

この変更は、パニック発生時にスタックトレースがログに書き込まれる前に接続が閉じられることで、テスト環境などでスタックトレースが失われる問題を解決しようとしました。しかし、このコミット（2c6d3eaf78c9314fe49a550e765def95463179e8）は、この変更が「ビルドを壊す」という問題を引き起こしたため、その変更を元に戻しています。

「ビルドを壊す」という具体的な原因は明記されていませんが、考えられる可能性としては以下のようなものがあります。

競合状態の悪化: 接続を閉じるタイミングを変更したことで、他のゴルーチンやシステムリソースとの間で新たな競合状態が発生し、デッドロックやデータ破損を引き起こした。
リソースリーク: 接続を閉じる処理が早すぎた、あるいは特定の条件下で実行されなくなったことで、ファイルディスクリプタやネットワークリソースが適切に解放されず、ビルドプロセスやテスト実行中にリソース枯渇を引き起こした。
テストフレームワークとの非互換性: テストフレームワークが接続のライフサイクルに依存しており、接続が早期に閉じられたことでテストが失敗するようになった。
コンパイルエラー: 非常に稀ですが、コードの移動がGoコンパイラのバグを露呈させたか、あるいは特定の環境でのみ発生するコンパイルエラーを引き起こした。

このコミットは、問題のある変更を元に戻すことで、Goのビルドの安定性を回復させることを目的としています。

コアとなるコードの変更箇所

変更はsrc/pkg/net/http/server.goファイルにあります。

--- a/src/pkg/net/http/server.go
+++ b/src/pkg/net/http/server.go
@@ -569,14 +569,14 @@ func (c *conn) serve() {
 		if err == nil {
 			return
 		}
+		if c.rwc != nil { // may be nil if connection hijacked
+			c.rwc.Close()
+		}
+
 		var buf bytes.Buffer
 		fmt.Fprintf(&buf, "http: panic serving %v: %v\\n", c.remoteAddr, err)
 		buf.Write(debug.Stack())
 		log.Print(buf.String())
-
-		if c.rwc != nil { // may be nil if connection hijacked
-			c.rwc.Close()
-		}
 	}()

 	if tlsConn, ok := c.rwc.(*tls.Conn); ok {

コアとなるコードの解説

このdiffは、(*conn).serve()メソッド内のdefer関数ブロックにおけるコードの移動を示しています。

-で始まる行: 削除された行を示します。
+で始まる行: 追加された行を示します。

具体的には、以下のコードブロックが移動されています。

if c.rwc != nil { // may be nil if connection hijacked
	c.rwc.Close()
}

元のコミット（f6b994f33cf4）では、このc.rwc.Close()の呼び出しが、log.Print(buf.String())（スタックトレースの出力）の前に移動されました。しかし、このコミット（2c6d3eaf78c9314fe49a550e765def95463179e8）では、その変更が元に戻され、c.rwc.Close()の呼び出しが再びlog.Print(buf.String())の後に配置されています。

この変更は、パニック発生時の処理順序を元の状態に戻すことを意味します。つまり、パニックが捕捉された場合、まずスタックトレースがバッファに書き込まれ、ログに出力され、その後にネットワーク接続が閉じられるという順序に戻ります。これにより、以前の「ビルドを壊す」問題が解消されることが期待されます。

参考にした情報源リンク

Go言語公式ドキュメント: net/httpパッケージ (https://pkg.go.dev/net/http)
Go言語公式ドキュメント: runtime/debugパッケージ (https://pkg.go.dev/runtime/debug)
Go言語公式ドキュメント: logパッケージ (https://pkg.go.dev/log)
Go言語におけるpanicとrecoverのメカニズムに関する一般的な情報源 (例: Go by Example: Panics, Defer, and Recover)

comemo