Skip to content

Commit fa6c12d

Browse files
znullCopilot
andcommitted
Fix memoryWatchStage.Wait() to always call stopWatching()
Wait() returned the inner stage's error immediately without calling stopWatching(), which meant the memory watcher goroutine was never cancelled when the stage exited with an error (e.g., from being killed due to memory limit). This prevented the observer from logging peak memory usage on kill. Fix: always call stopWatching() before returning, regardless of whether the inner stage returned an error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a4bdbd1 commit fa6c12d

2 files changed

Lines changed: 10 additions & 4 deletions

File tree

pipe/memorylimit.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,11 +297,9 @@ func (m *memoryWatchStage) monitor(ctx context.Context) {
297297
}
298298

299299
func (m *memoryWatchStage) Wait() error {
300-
if err := m.stage.Wait(); err != nil {
301-
return err
302-
}
300+
err := m.stage.Wait()
303301
m.stopWatching()
304-
return nil
302+
return err
305303
}
306304

307305
func (m *memoryWatchStage) GetRSSAnon(ctx context.Context) (uint64, error) {

pipe/memorylimit_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,14 @@ func TestMemoryLimitWithObserverTreeMem(t *testing.T) {
129129
require.ErrorContains(t, err, "memory limit exceeded")
130130
}
131131

132+
func TestMemoryLimitWithObserverLogsPeakOnKill(t *testing.T) {
133+
t.Parallel()
134+
msg, err := testMemoryLimitWithObserver(t, 400, 10_000_000, pipe.Command("less"))
135+
assert.Contains(t, msg, "exceeded allowed memory")
136+
assert.Contains(t, msg, "peak memory usage")
137+
require.ErrorContains(t, err, "memory limit exceeded")
138+
}
139+
132140
func testMemoryLimit(t *testing.T, mbs int, limit uint64, stage pipe.Stage) (string, error) {
133141
ctx := context.Background()
134142

0 commit comments

Comments
 (0)