Keyboard shortcuts

Press ← or → to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

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

このコミットは、Go言語の time パッケージ内の TestOverflowRuntimeTimer というテストの不安定性（flakiness）を解消することを目的としています。このテストは、Goランタイムのタイマー機構、特にタイマーのオーバーフロー検出ロジックを検証するためのものでしたが、テスト自体の実装に問題があり、スケジューラの飢餓状態を引き起こすことで、タイマーのバグではないにもかかわらずテストが失敗するという誤検出が頻発していました。本コミットは、このテストの根本的な問題を修正するのではなく、テストの期待値を変更し、外部のタイムアウトに依存することで、テストの信頼性を向上させています。

コミット

commit 83c814066237bf6e8490d83643ba76513082a158
Author: Rob Pike <r@golang.org>
Date:   Thu Jun 12 11:44:55 2014 -0700

    time: avoid broken fix for buggy TestOverflowRuntimeTimer
    The test requires that timerproc runs, but busy loops and starves
    the scheduler so that, with high probability, timerproc doesn't run.
    Avoid the issue by expecting the test to succeed; if not, a major
    outside timeout will kill it and let us know.
    
    As you can see from the diffs, there have been several attempts to
    fix this with chicanery, but none has worked. Don't bother trying
    any more.
    
    Fixes #8136.
    
    LGTM=rsc
    R=rsc, josharian
    CC=golang-codereviews
    https://golang.org/cl/105140043

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

https://github.com/golang/go/commit/83c814066237bf6e8490d83643ba76513082a158

元コミット内容

time: バグのある TestOverflowRuntimeTimer の壊れた修正を回避する。 このテストは timerproc が実行されることを要求するが、ビジーループがスケジューラを飢餓状態にし、高い確率で timerproc が実行されない。 この問題を回避するため、テストが成功することを期待する。もし成功しない場合、大きな外部タイムアウトがテストを終了させ、我々に知らせるだろう。

差分からわかるように、この問題を修正するためにいくつかの試みが行われたが、どれも機能しなかった。これ以上試みる必要はない。

Fixes #8136.

変更の背景

TestOverflowRuntimeTimer は、Goランタイムのタイマー機能、特に runtime.addtimer 関数におけるオーバーフロー検出ロジックを検証するために設計されたテストです。しかし、このテストは長らく不安定であり、実際のタイマーのバグではなく、テスト自体の実装に起因する問題で頻繁に失敗していました。

具体的には、テスト内部で実行されるビジーループがGoスケジューラを飢餓状態に陥らせ、タイマー処理を担当する重要なゴルーチンである timerproc が適切に実行されないという問題がありました。これにより、タイマーが「スタックした」と誤って報告され、「overflow in addtimer」というエラーが発生していました。これは、タイマーのオーバーフローが実際に発生したわけではなく、timerproc がスケジュールされなかったためにタイマーイベントが処理されなかったことによる偽陽性でした。

この問題は Issue #8136 として報告されており、過去にも runtime.Gosched() や runtime.GC() を呼び出すなど、様々な試みが行われましたが、いずれもテストの不安定性を完全に解消するには至りませんでした。これらの試みは、スケジューラの動作を強制的に制御しようとするものでしたが、Goスケジューラの複雑な挙動と、テストが実行される環境（特にビルド環境）の多様性により、信頼性の高い解決策とはなりませんでした。

本コミットは、このテストの根本的なバグを修正するのではなく、テストの設計思想を変更することで、この不安定な挙動に対処しています。つまり、テストが内部でタイマーの完了を待つのではなく、外部のタイムアウト機構に依存することで、テストの信頼性を確保するというアプローチを採用しました。

前提知識の解説

Goランタイムスケジューラ

Goランタイムスケジューラは、Goプログラム内で実行されるゴルーチン（軽量スレッド）の実行を管理する重要なコンポーネントです。OSのスレッド上で複数のゴルーチンを多重化し、効率的な並行処理を実現します。スケジューラは、ゴルーチンをいつ、どのOSスレッドで実行するかを決定しますが、ビジーループやCPUを大量に消費する処理は、他のゴルーチン、特にシステムレベルのゴルーチン（例: timerproc）のスケジューリングを妨げ、飢餓状態を引き起こす可能性があります。

timerproc

timerproc は、Goランタイム内部で動作する特別なゴルーチンで、Goプログラム内で設定されたすべてのタイマー（time.After、time.NewTimer など）の管理と処理を担当します。タイマーの期限が来た際にチャネルへの送信を行うなど、時間ベースのイベントを正確に処理するために不可欠な存在です。timerproc が適切にスケジュールされないと、タイマーイベントが遅延したり、全く処理されなかったりする問題が発生します。

TestOverflowRuntimeTimer

このテストは、Goランタイムのタイマー実装が、非常に大きな時間値（オーバーフローを引き起こす可能性のある値）を扱った場合に正しく動作するかを検証するために書かれました。具体的には、runtime.addtimer 関数が内部的にどのようにタイマーを管理しているかをテストします。しかし、前述の通り、テスト内部のビジーループがスケジューラを妨害し、timerproc の実行を阻害することで、テストが誤って失敗するという問題がありました。

runtime.Gosched() と runtime.GC()

  • runtime.Gosched(): 現在のゴルーチンを一時停止し、Goスケジューラに他のゴルーチンを実行する機会を与える関数です。これにより、他のゴルーチンが実行される可能性が高まります。
  • runtime.GC(): Goランタイムのガベージコレクタを手動で実行する関数です。ガベージコレクションは、一時的にCPUリソースを消費しますが、メモリの解放や再利用を通じて、システム全体のパフォーマンスに影響を与えることがあります。

過去の修正試みでは、これらの関数をテスト内のビジーループ内で呼び出すことで、timerproc がスケジュールされる機会を増やそうとしました。しかし、runtime.Gosched() はスケジューラにヒントを与えるだけであり、必ずしも他のゴルーチンが即座に実行されることを保証するものではありません。また、runtime.GC() はガベージコレクションの実行を強制するものであり、スケジューリングの遅延を根本的に解決するものではありませんでした。特に、CPUがビジーな環境や、テストが実行される特定の条件下では、これらの試みは信頼性に欠けました。

技術的詳細

このコミットの技術的なアプローチは、TestOverflowRuntimeTimer の内部ロジックを簡素化し、テストの成功/失敗の判断を外部のタイムアウト機構に委ねるというものです。

以前の CheckRuntimeTimerOverflow 関数は、t.C からの受信をビジーループ内で試み、特定のタイムアウト期間内に受信できなかった場合にエラーを返すというロジックを持っていました。このビジーループは、runtime.Gosched() や runtime.GC() を呼び出すことで、timerproc が実行される機会を増やそうとしていましたが、これがスケジューラを飢餓状態に陥らせ、テストの不安定性の原因となっていました。

新しいアプローチでは、このビジーループとエラーハンドリングを完全に削除し、単に <-t.C というチャネル受信操作のみを行います。これにより、timerproc がタイマーイベントを処理し、t.C チャネルに値を送信するまで、テストはブロックされます。

もし timerproc が何らかの理由で実行されず、タイマーイベントが処理されない場合、テストは無限にブロックされることになります。しかし、Goのテストフレームワークには、テストが一定時間内に完了しない場合に強制的に終了させる外部タイムアウト機構（例: go test -timeout オプション）が存在します。このコミットは、この外部タイムアウトに依存することで、テストがハングアップした場合でも、最終的にはテストスイート全体が失敗し、問題が検出されるという考え方に基づいています。

この変更は、テストが自身の内部でスケジューリングの問題を解決しようとするのをやめ、より高レベルのテストインフラストラクチャにその責任を委ねるという、実用的な判断に基づいています。これにより、テストコードの複雑さが軽減され、テストの信頼性が向上します。

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

このコミットでは、主に以下の2つのファイルが変更されています。

  1. src/pkg/time/internal_test.go

    • CheckRuntimeTimerOverflow 関数のシグネチャが func CheckRuntimeTimerOverflow() error から func CheckRuntimeTimerOverflow() に変更され、エラーを返さなくなりました。
    • 関数内部のビジーループ（for { select { ... } }）と、それに伴うタイムアウト処理、runtime.Gosched() や runtime.GC() の呼び出しが削除されました。
    • 代わりに、<-t.C という単一のチャネル受信操作のみが残されました。
    • import セクションから errors と runtime パッケージのインポートが削除されました。

  2. src/pkg/time/sleep_test.go

    • TestOverflowRuntimeTimer 関数内で CheckRuntimeTimerOverflow() の呼び出し方が変更されました。以前はエラーをチェックして t.Fatalf を呼び出していましたが、新しい CheckRuntimeTimerOverflow がエラーを返さないため、単に CheckRuntimeTimerOverflow() を呼び出すだけになりました。
    • テストがハングアップする可能性についてのコメントが追加されました。

差分スニペット

diff --git a/src/pkg/time/internal_test.go b/src/pkg/time/internal_test.go
index 2243d3668d..f09d30507f 100644
--- a/src/pkg/time/internal_test.go
+++ b/src/pkg/time/internal_test.go
@@ -4,11 +4,6 @@
 
 package time
 
-import (
-	"errors"
-	"runtime"
-)
-
 func init() {
 	// force US/Pacific for time zone tests
 	ForceUSPacificForTesting()
@@ -24,7 +19,7 @@ func empty(now int64, arg interface{}) {}
 //
 // This test has to be in internal_test.go since it fiddles with
 // unexported data structures.
-func CheckRuntimeTimerOverflow() error {
+func CheckRuntimeTimerOverflow() {
 	// We manually create a runtimeTimer to bypass the overflow
 	// detection logic in NewTimer: we're testing the underlying
 	// runtime.addtimer function.
@@ -35,30 +30,12 @@ func CheckRuntimeTimerOverflow() error {
 	}
 	startTimer(r)
 
-\ttimeout := 100 * Millisecond
-\tswitch runtime.GOOS {
-\t// Allow more time for gobuilder to succeed.
-\tcase "windows":
-\t\ttimeout = Second
-\tcase "plan9":
-\t\t// TODO(0intro): We don't know why it is needed.
-\t\ttimeout = 3 * Second
-\t}\n\n\t// Start a goroutine that should send on t.C before the timeout.
+\t// Start a goroutine that should send on t.C right away.
 \tt := NewTimer(1)
 
 \tdefer func() {
 \t\tstopTimer(r)
 \t\tstartTimer(r)
 \t}()
 
-\t// Try to receive from t.C before the timeout. It will succeed
-\t// iff the previous sleep was able to finish. We're forced to
-\t// spin and yield after trying to receive since we can't start
-\t// any more timers (they might hang due to the same bug we're
-\t// now testing).\n\tstop := Now().Add(timeout)
-\tfor {
-\t\tselect {
-\t\tcase <-t.C:
-\t\t\treturn nil // It worked!
-\t\tdefault:
-\t\t\tif Now().After(stop) {
-\t\t\t\treturn errors.New("runtime timer stuck: overflow in addtimer")
-\t\t\t}\n\t\t\t// Issue 6874. This test previously called runtime.Gosched to try to yield
-\t\t\t// to the goroutine servicing t, however the scheduler has a bias towards the
-\t\t\t// previously running goroutine in an idle system. Combined with high load due
-\t\t\t// to all CPUs busy running tests t's goroutine could be delayed beyond the
-\t\t\t// timeout window.\n\t\t\t//\n-\t\t\t// Calling runtime.GC() reduces the worst case lantency for scheduling t by 20x
-\t\t\t// under the current Go 1.3 scheduler.\n\t\t\truntime.GC()
-\t\t}\n\t}\n+\t// If the test fails, we will hang here until the timeout in the testing package
+\t// fires, which is 10 minutes. It would be nice to catch the problem sooner,
+\t// but there is no reliable way to guarantee that timerproc schedules without
+\t// doing something involving timerproc itself. Previous failed attempts have
+\t// tried calling runtime.Gosched and runtime.GC, but neither is reliable.
+\t// So we fall back to hope: We hope we don't hang here.
+\t<-t.C
 }\ndiff --git a/src/pkg/time/sleep_test.go b/src/pkg/time/sleep_test.go
index 03f8e732c9..d78490d444 100644
--- a/src/pkg/time/sleep_test.go
+++ b/src/pkg/time/sleep_test.go
@@ -387,7 +387,7 @@ func TestOverflowRuntimeTimer(t *testing.T) {
 \tif testing.Short() {
 \t\tt.Skip("skipping in short mode, see issue 6874")
 \t}\n-\tif err := CheckRuntimeTimerOverflow(); err != nil {
-\t\tt.Fatalf(err.Error())\n-\t}\n+\t// This may hang forever if timers are broken. See comment near
+\t// the end of CheckRuntimeTimerOverflow in internal_test.go.
+\tCheckRuntimeTimerOverflow()\
 }\n```

## コアとなるコードの解説

`CheckRuntimeTimerOverflow` 関数における最も重要な変更は、ビジーループとエラーを返すロジックの削除です。

**変更前:**
変更前のコードでは、`t.C` チャネルからの受信を `select` ステートメントと `for` ループを使ってポーリングしていました。このループは、`Now().After(stop)` でタイムアウトをチェックし、タイムアウトした場合は `errors.New("runtime timer stuck: overflow in addtimer")` を返していました。また、`default` ケースでは `runtime.GC()` を呼び出すことで、スケジューラにヒントを与え、`timerproc` が実行される機会を増やそうとしていました。

このアプローチの問題点は、テストが自身の内部でスケジューリングの問題を解決しようとしていた点にあります。ビジーループ自体がCPUを消費し、スケジューラに負荷をかけるため、かえって `timerproc` の実行を妨げる可能性がありました。また、`runtime.GC()` や `runtime.Gosched()` はスケジューリングの確実な保証を与えるものではなく、特定の環境や負荷条件下でテストが不安定になる原因となっていました。

**変更後:**
変更後のコードは、単に `<-t.C` というチャネル受信操作のみを行います。これにより、`CheckRuntimeTimerOverflow` 関数は、`t.C` チャネルに値が送信されるまで（つまり、タイマーが期限切れになり `timerproc` によって処理されるまで）ブロックされます。

この変更の背後にある考え方は、「テストがハングアップした場合、それはタイマーに問題があることを意味し、そのハングアップは外部のテストフレームワークのタイムアウトによって検出されるべきである」というものです。Goのテストフレームワーク（`go test` コマンド）には、テストが一定時間内に完了しない場合に強制的に終了させる機能があります。この機能に依存することで、テストコード自体は簡潔になり、スケジューリングの複雑さに起因する偽陽性を排除できます。

`TestOverflowRuntimeTimer` 関数における変更は、`CheckRuntimeTimerOverflow` がエラーを返さなくなったことに伴うものです。以前はエラーをチェックして `t.Fatalf` を呼び出していましたが、新しいバージョンでは単に `CheckRuntimeTimerOverflow()` を呼び出すだけになりました。これにより、テストの失敗は、`CheckRuntimeTimerOverflow` がハングアップし、外部タイムアウトによってテストプロセス全体が終了した場合にのみ発生するようになります。

このコミットは、テストの信頼性を向上させるための実用的なアプローチであり、Goランタイムのタイマー機構自体のバグを修正するものではなく、そのテスト方法を改善するものです。

## 関連リンク

*   **Go CL (Code Review):** [https://golang.org/cl/105140043](https://golang.org/cl/105140043)
*   **GitHub Issue #8136:** [https://github.com/golang/go/issues/8136](https://github.com/golang/go/issues/8136)

## 参考にした情報源リンク

*   **Google Web Search (for context on Issue #8136 and TestOverflowRuntimeTimer):** [https://vertexaisearch.cloud.google.com/grounding-api-redirect/AUZIYQFSVcBl_cgDzaLTEX_H2y-W5bu4hUjQyFibf-AExpgsgkEKCyhPl0BdaTTL_TCKd1Ivl4hj209DhxiPi8uNj02iToDFjm59FfVj7tRN0zW-fhJ1yptn-Masru3XZRIKEQ==](https://vertexaisearch.cloud.google.com/grounding-api-redirect/AUZIYQFSVcBl_cgDzaLTEX_H2y-W5bu4hUjQyFibf-AExpgsgkEKCyhPlBdaTTL_TCKd1Ivl4hj209DhxiPi8uNj02iToDFjm59FfVj7tRN0zW-fhJ1yptn-Masru3XZRIKEQ==)