[PR #134] [CLOSED] Fix goroutine and resource leak in terminal exec timeout handling #182

New Issue

2026-06-06T22:09:35-04:00

yindo commented

2026-06-06 22:09:35 -04:00

📋 Pull Request Information

Original PR: https://github.com/vxcontrol/pentagi/pull/134
Author: @Vaibhavee89
Created: 2/23/2026
Status: ❌ Closed

Base: master ← Head: fix/goroutine-leak-issue-108

📝 Commits (3)

5b356dc Fix goroutine and resource leak in terminal exec timeout handling
dbb4051 Add comprehensive tests for goroutine leak fix
42fb981 fix: add explicit resp.Close() before grace period wait

📊 Changes

4 files changed (+407 additions, -4 deletions)

View changed files

📝 backend/go.mod (+1 -0)
📝 backend/pkg/tools/terminal.go (+17 -4)
➕ backend/pkg/tools/terminal_leak_test.go (+217 -0)
➕ backend/pkg/tools/terminal_load_test.go (+172 -0)

📄 Description

Summary

Fixes #108

This PR addresses the goroutine and resource leak in the getExecResult() method in backend/pkg/tools/terminal.go. The issue occurred when command execution timed out - the io.Copy goroutine would remain blocked even after context cancellation, leading to memory bloat in long-running systems.

Changes Made

Replaced done channel with buffered error channel (errChan): The goroutine now sends the io.Copy error result through errChan instead of just closing a channel
Proper goroutine completion handling: On timeout, the code now waits for the goroutine to complete with a grace period (defaultExtraExecTimeout)
Improved error handling: Both normal completion and timeout scenarios are properly handled with correct error propagation

Technical Details

The fix ensures that:

Normal case: The goroutine completes and sends its error (or nil) via errChan
Timeout case: After context timeout, we wait up to defaultExtraExecTimeout (5 seconds) for the goroutine to finish cleanly
Resources are properly released in both scenarios, preventing goroutine leaks

Test Plan

Code compiles successfully
Manual testing with timeout scenarios (requires Go environment)
Integration testing with Docker container execution
Verify no goroutine leaks under load

Impact

This fix prevents:

Memory bloat in long-running systems
Resource accumulation from leaked goroutines
Potential performance degradation over time

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/vxcontrol/pentagi/pull/134 **Author:** [@Vaibhavee89](https://github.com/Vaibhavee89) **Created:** 2/23/2026 **Status:** ❌ Closed **Base:** `master` ← **Head:** `fix/goroutine-leak-issue-108` --- ### 📝 Commits (3) - [`5b356dc`](https://github.com/vxcontrol/pentagi/commit/5b356dcafe8f7dd03cd79411bc9b0e8fc84c05c3) Fix goroutine and resource leak in terminal exec timeout handling - [`dbb4051`](https://github.com/vxcontrol/pentagi/commit/dbb405144bc3092d07d4b2881508f560f8a35ff3) Add comprehensive tests for goroutine leak fix - [`42fb981`](https://github.com/vxcontrol/pentagi/commit/42fb981fbf6b80a89daf955dfbc63e5f9276efee) fix: add explicit resp.Close() before grace period wait ### 📊 Changes **4 files changed** (+407 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `backend/go.mod` (+1 -0) 📝 `backend/pkg/tools/terminal.go` (+17 -4) ➕ `backend/pkg/tools/terminal_leak_test.go` (+217 -0) ➕ `backend/pkg/tools/terminal_load_test.go` (+172 -0) </details> ### 📄 Description ## Summary Fixes #108 This PR addresses the goroutine and resource leak in the `getExecResult()` method in `backend/pkg/tools/terminal.go`. The issue occurred when command execution timed out - the `io.Copy` goroutine would remain blocked even after context cancellation, leading to memory bloat in long-running systems. ## Changes Made - **Replaced `done` channel with buffered error channel (`errChan`)**: The goroutine now sends the `io.Copy` error result through `errChan` instead of just closing a channel - **Proper goroutine completion handling**: On timeout, the code now waits for the goroutine to complete with a grace period (`defaultExtraExecTimeout`) - **Improved error handling**: Both normal completion and timeout scenarios are properly handled with correct error propagation ## Technical Details The fix ensures that: 1. **Normal case**: The goroutine completes and sends its error (or nil) via `errChan` 2. **Timeout case**: After context timeout, we wait up to `defaultExtraExecTimeout` (5 seconds) for the goroutine to finish cleanly 3. **Resources are properly released** in both scenarios, preventing goroutine leaks ## Test Plan - [x] Code compiles successfully - [x] Manual testing with timeout scenarios (requires Go environment) - [x] Integration testing with Docker container execution - [ ] Verify no goroutine leaks under load ## Impact This fix prevents: - Memory bloat in long-running systems - Resource accumulation from leaked goroutines - Potential performance degradation over time --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

yindo added the pull-request label 2026-06-06 22:09:35 -04:00

yindo closed this issue

2026-06-06 22:09:35 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vxcontrol/pentagi#182