diff --git a/pkg/capture/provider/network_capture_win.go b/pkg/capture/provider/network_capture_win.go index 9163f8c734..c11ef3393d 100644 --- a/pkg/capture/provider/network_capture_win.go +++ b/pkg/capture/provider/network_capture_win.go @@ -84,7 +84,7 @@ func (ncp *NetworkCaptureProvider) CaptureNetworkPacket(ctx context.Context, fil } if stopTrace { ncp.l.Info("Stopping netsh trace session before starting a new one") - _ = ncp.stopNetworkCapture(ctx) + _ = ncp.stopNetworkCapture() //nolint:contextcheck // stopNetworkCapture creates its own context } captureFileName := ncp.Filename.String() + ".etl" @@ -164,7 +164,8 @@ func (ncp *NetworkCaptureProvider) CaptureNetworkPacket(ctx context.Context, fil } ncp.l.Info("Stop netsh") - if err := ncp.stopNetworkCapture(ctx); err != nil { + // stopNetworkCapture creates its own context; parent ctx is expired here + if err := ncp.stopNetworkCapture(); err != nil { //nolint:contextcheck // stopNetworkCapture creates its own context ncp.l.Error("Failed to stop netsh trace by 'netsh trace stop', will kill the process", zap.Error(err)) _ = captureStartCmd.Process.Kill() return fmt.Errorf("netsh stop failed: Output: %s", err) @@ -204,10 +205,15 @@ func (ncp *NetworkCaptureProvider) needToStopTraceSession(ctx context.Context) ( return false, fmt.Errorf("cannot stop trace session because it's not created by Retina capture") } -func (ncp *NetworkCaptureProvider) stopNetworkCapture(ctx context.Context) error { +func (ncp *NetworkCaptureProvider) stopNetworkCapture() error { ncp.l.Info("Stopping netsh trace session") - command := exec.CommandContext(ctx, "netsh", "trace", "stop") + // Create independent context for cleanup. + // netsh trace stop completes in ~1s; 30s timeout provides ample safety margin. + stopCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + command := exec.CommandContext(stopCtx, "netsh", "trace", "stop") output, err := command.CombinedOutput() // ignore the error when stop the trace when no live trace session exists. if strings.Contains(string(output), "There is no trace session currently in progress") { diff --git a/pkg/capture/provider/network_capture_win_test.go b/pkg/capture/provider/network_capture_win_test.go index 87e3a03fa4..5e51d45a7a 100644 --- a/pkg/capture/provider/network_capture_win_test.go +++ b/pkg/capture/provider/network_capture_win_test.go @@ -6,7 +6,13 @@ package provider import ( + "context" "testing" + "time" + + "github.com/microsoft/retina/pkg/capture/file" + "github.com/microsoft/retina/pkg/log" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestValidateNetshFilter(t *testing.T) { @@ -114,3 +120,37 @@ func TestValidateNetshFilter(t *testing.T) { }) } } + +// TestStopNetworkCapture_ContextIndependence verifies stopNetworkCapture creates its own context +func TestStopNetworkCapture_ContextIndependence(t *testing.T) { + now := metav1.Now() + ncp := &NetworkCaptureProvider{ + NetworkCaptureProviderCommon: NetworkCaptureProviderCommon{ + TmpCaptureDir: t.TempDir(), + l: log.Logger().Named("test-capture"), + }, + Filename: file.CaptureFilename{ + CaptureName: "test-capture", + NodeHostname: "test-node", + StartTimestamp: &now, + }, + } + + // Create an expired context (simulating capture duration ending) + parentCtx, cancel := context.WithTimeout(context.Background(), 1*time.Nanosecond) + time.Sleep(10 * time.Millisecond) + defer cancel() + + if parentCtx.Err() == nil { + t.Fatal("Setup error: parent context should be expired") + } + + // Call StopNetworkCapture - should NOT return "context deadline exceeded" + err := ncp.stopNetworkCapture() + + if err != nil && err.Error() == "context deadline exceeded" { + t.Fatal("StopNetworkCapture returned 'context deadline exceeded' - bug reintroduced") + } + + t.Logf("StopNetworkCapture uses independent context (netsh error expected: %v)", err) +}