fix: clean up main loop state on panic
- commit
- 8edd742134b070b013cf8a28345e89f4a414c543
- author
- brice · · 2026-05-15 12:33 UTC
- parent
- ee1755a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +30 -5
M internal/capi/stream_ops.go +5 -5
| @@ -222,13 +222,13 @@ func (s *streamOpsImpl) RunMainLoop(loopPtr unsafe.Pointer) error { | |||
| 222 | 222 | } | |
| 223 | 223 | s.runningLoops[loopPtr] = true | |
| 224 | 224 | s.mu.Unlock() | |
| 225 | + | defer func() { | |
| 226 | + | s.mu.Lock() | |
| 227 | + | delete(s.runningLoops, loopPtr) | |
| 228 | + | s.mu.Unlock() | |
| 229 | + | }() | |
| 225 | 230 | ||
| 226 | 231 | ret := pw_main_loop_run(loopPtr) | |
| 227 | - | ||
| 228 | - | s.mu.Lock() | |
| 229 | - | delete(s.runningLoops, loopPtr) | |
| 230 | - | s.mu.Unlock() | |
| 231 | - | ||
| 232 | 232 | if ret < 0 { | |
| 233 | 233 | return &PWError{Func: "pw_main_loop_run", Code: ret} | |
| 234 | 234 | } | |
| @@ -222,13 +222,13 @@ func (s *streamOpsImpl) RunMainLoop(loopPtr unsafe.Pointer) error { | |||||
| 222 | } | 222 | } | ||
| 223 | s.runningLoops[loopPtr] = true | 223 | s.runningLoops[loopPtr] = true | ||
| 224 | s.mu.Unlock() | 224 | s.mu.Unlock() | ||
| 225 | + | defer func() { | |||
| 226 | + | s.mu.Lock() | |||
| 227 | + | delete(s.runningLoops, loopPtr) | |||
| 228 | + | s.mu.Unlock() | |||
| 229 | + | }() | |||
| 225 | 230 | ||||
| 226 | ret := pw_main_loop_run(loopPtr) | 231 | ret := pw_main_loop_run(loopPtr) | ||
| 227 | - | ||||
| 228 | - | s.mu.Lock() | |||
| 229 | - | delete(s.runningLoops, loopPtr) | |||
| 230 | - | s.mu.Unlock() | |||
| 231 | - | ||||
| 232 | if ret < 0 { | 232 | if ret < 0 { | ||
| 233 | return &PWError{Func: "pw_main_loop_run", Code: ret} | 233 | return &PWError{Func: "pw_main_loop_run", Code: ret} | ||
| 234 | } | 234 | } | ||
M internal/capi/stream_ops_test.go +25 -0
| @@ -564,6 +564,31 @@ func TestRunMainLoopReturnsNilOnSuccess(t *testing.T) { | |||
| 564 | 564 | } | |
| 565 | 565 | } | |
| 566 | 566 | ||
| 567 | + | func TestRunMainLoopRemovesRunningLoopOnPanic(t *testing.T) { | |
| 568 | + | origRun := pw_main_loop_run | |
| 569 | + | t.Cleanup(func() { pw_main_loop_run = origRun }) | |
| 570 | + | ||
| 571 | + | pw_main_loop_run = func(loop unsafe.Pointer) int32 { | |
| 572 | + | panic("boom") | |
| 573 | + | } | |
| 574 | + | ||
| 575 | + | ops := &streamOpsImpl{} | |
| 576 | + | fakeLoop := opaquePtr() | |
| 577 | + | ||
| 578 | + | defer func() { | |
| 579 | + | if r := recover(); r == nil { | |
| 580 | + | t.Fatal("expected panic from pw_main_loop_run") | |
| 581 | + | } | |
| 582 | + | ops.mu.Lock() | |
| 583 | + | defer ops.mu.Unlock() | |
| 584 | + | if len(ops.runningLoops) != 0 { | |
| 585 | + | t.Fatalf("expected runningLoops to be cleaned up, got %d entries", len(ops.runningLoops)) | |
| 586 | + | } | |
| 587 | + | }() | |
| 588 | + | ||
| 589 | + | _ = ops.RunMainLoop(fakeLoop) | |
| 590 | + | } | |
| 591 | + | ||
| 567 | 592 | // TestCreatePlaybackStreamUsesPWLoopFromMainLoop verifies that CreatePlaybackStream | |
| 568 | 593 | // calls pw_main_loop_get_loop to obtain the pw_loop pointer and passes it to | |
| 569 | 594 | // pw_stream_new_simple (not the raw main loop pointer). | |
| @@ -564,6 +564,31 @@ func TestRunMainLoopReturnsNilOnSuccess(t *testing.T) { | |||||
| 564 | } | 564 | } | ||
| 565 | } | 565 | } | ||
| 566 | 566 | ||||
| 567 | + | func TestRunMainLoopRemovesRunningLoopOnPanic(t *testing.T) { | |||
| 568 | + | origRun := pw_main_loop_run | |||
| 569 | + | t.Cleanup(func() { pw_main_loop_run = origRun }) | |||
| 570 | + | ||||
| 571 | + | pw_main_loop_run = func(loop unsafe.Pointer) int32 { | |||
| 572 | + | panic("boom") | |||
| 573 | + | } | |||
| 574 | + | ||||
| 575 | + | ops := &streamOpsImpl{} | |||
| 576 | + | fakeLoop := opaquePtr() | |||
| 577 | + | ||||
| 578 | + | defer func() { | |||
| 579 | + | if r := recover(); r == nil { | |||
| 580 | + | t.Fatal("expected panic from pw_main_loop_run") | |||
| 581 | + | } | |||
| 582 | + | ops.mu.Lock() | |||
| 583 | + | defer ops.mu.Unlock() | |||
| 584 | + | if len(ops.runningLoops) != 0 { | |||
| 585 | + | t.Fatalf("expected runningLoops to be cleaned up, got %d entries", len(ops.runningLoops)) | |||
| 586 | + | } | |||
| 587 | + | }() | |||
| 588 | + | ||||
| 589 | + | _ = ops.RunMainLoop(fakeLoop) | |||
| 590 | + | } | |||
| 591 | + | ||||
| 567 | // TestCreatePlaybackStreamUsesPWLoopFromMainLoop verifies that CreatePlaybackStream | 592 | // TestCreatePlaybackStreamUsesPWLoopFromMainLoop verifies that CreatePlaybackStream | ||
| 568 | // calls pw_main_loop_get_loop to obtain the pw_loop pointer and passes it to | 593 | // calls pw_main_loop_get_loop to obtain the pw_loop pointer and passes it to | ||
| 569 | // pw_stream_new_simple (not the raw main loop pointer). | 594 | // pw_stream_new_simple (not the raw main loop pointer). | ||