Skip to content

Commit d9f1625

Browse files
Close split capture pipes on startup failure
Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
1 parent d6fb969 commit d9f1625

2 files changed

Lines changed: 135 additions & 21 deletions

File tree

internal/airplay/capture.go

Lines changed: 89 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,76 @@ func buildSplitGstVideoPipeline(source gstStage, beforeConvert, afterScale []gst
907907
return producer, consumer
908908
}
909909

910+
type waylandSplitPipes struct {
911+
rawFrames *os.File
912+
sourceOutput *os.File
913+
sourceStderr *os.File
914+
sourceErrOut *os.File
915+
stdout *os.File
916+
encoderOutput *os.File
917+
encoderStderr *os.File
918+
encoderErrOut *os.File
919+
}
920+
921+
func openWaylandSplitPipes(sourceCmd, encoderCmd *exec.Cmd) (*waylandSplitPipes, error) {
922+
pipes := &waylandSplitPipes{}
923+
var err error
924+
pipes.rawFrames, pipes.sourceOutput, err = os.Pipe()
925+
if err != nil {
926+
return nil, fmt.Errorf("capture serialization pipe: %w", err)
927+
}
928+
sourceCmd.Stdout = pipes.sourceOutput
929+
pipes.sourceStderr, pipes.sourceErrOut, err = os.Pipe()
930+
if err != nil {
931+
pipes.close()
932+
return nil, fmt.Errorf("capture stderr pipe: %w", err)
933+
}
934+
sourceCmd.Stderr = pipes.sourceErrOut
935+
encoderCmd.Stdin = pipes.rawFrames
936+
pipes.stdout, pipes.encoderOutput, err = os.Pipe()
937+
if err != nil {
938+
pipes.close()
939+
return nil, fmt.Errorf("encoder stdout pipe: %w", err)
940+
}
941+
encoderCmd.Stdout = pipes.encoderOutput
942+
pipes.encoderStderr, pipes.encoderErrOut, err = os.Pipe()
943+
if err != nil {
944+
pipes.close()
945+
return nil, fmt.Errorf("encoder stderr pipe: %w", err)
946+
}
947+
encoderCmd.Stderr = pipes.encoderErrOut
948+
return pipes, nil
949+
}
950+
951+
func (pipes *waylandSplitPipes) close() {
952+
if pipes == nil {
953+
return
954+
}
955+
for _, closer := range []io.Closer{
956+
pipes.rawFrames,
957+
pipes.sourceOutput,
958+
pipes.sourceStderr,
959+
pipes.sourceErrOut,
960+
pipes.stdout,
961+
pipes.encoderOutput,
962+
pipes.encoderStderr,
963+
pipes.encoderErrOut,
964+
} {
965+
if closer != nil {
966+
_ = closer.Close()
967+
}
968+
}
969+
}
970+
971+
func startWaylandEncoder(cmd *exec.Cmd, pipes *waylandSplitPipes) (<-chan error, error) {
972+
wait, err := startGStreamerCommand(cmd)
973+
if err != nil {
974+
pipes.close()
975+
return nil, err
976+
}
977+
return wait, nil
978+
}
979+
910980
func startPreparedWaylandCapture(ctx context.Context, cfg CaptureConfig, encoderParts encoderResult, nodeID uint32, pwFd *os.File, dbusConn *dbus.Conn, streamSize [2]int, timestampedOutput bool) (*ScreenCapture, error) {
911981
if pwFd == nil || dbusConn == nil {
912982
if pwFd != nil {
@@ -977,60 +1047,58 @@ func startPreparedWaylandCapture(ctx context.Context, cfg CaptureConfig, encoder
9771047

9781048
sourceCmd := exec.CommandContext(captureCtx, "gst-launch-1.0", sourceArgs...)
9791049
sourceCmd.ExtraFiles = []*os.File{pwFd}
980-
rawFrames, err := sourceCmd.StdoutPipe()
981-
if err != nil {
982-
cancel()
983-
_ = pwFd.Close()
984-
_ = dbusConn.Close()
985-
return nil, fmt.Errorf("capture serialization pipe: %w", err)
986-
}
987-
sourceStderr, _ := sourceCmd.StderrPipe()
988-
9891050
cmd := exec.CommandContext(captureCtx, "gst-launch-1.0", encoderArgs...)
990-
cmd.Stdin = rawFrames
991-
stdout, err := cmd.StdoutPipe()
1051+
pipes, err := openWaylandSplitPipes(sourceCmd, cmd)
9921052
if err != nil {
9931053
cancel()
994-
_ = rawFrames.Close()
9951054
_ = pwFd.Close()
9961055
_ = dbusConn.Close()
997-
return nil, fmt.Errorf("encoder stdout pipe: %w", err)
1056+
return nil, err
9981057
}
999-
encoderStderr, _ := cmd.StderrPipe()
10001058

1001-
encoderWait, err := startGStreamerCommand(cmd)
1059+
encoderWait, err := startWaylandEncoder(cmd, pipes)
10021060
if err != nil {
10031061
cancel()
1004-
_ = rawFrames.Close()
10051062
_ = pwFd.Close()
10061063
_ = dbusConn.Close()
10071064
return nil, fmt.Errorf("start encoder gst-launch: %w", err)
10081065
}
1066+
_ = pipes.rawFrames.Close()
1067+
_ = pipes.encoderOutput.Close()
1068+
_ = pipes.encoderErrOut.Close()
10091069
sourceWait, err := startGStreamerCommand(sourceCmd)
10101070
if err != nil {
10111071
cancel()
1012-
_ = rawFrames.Close()
1072+
pipes.close()
10131073
_ = pwFd.Close()
10141074
_ = dbusConn.Close()
10151075
<-encoderWait
10161076
return nil, fmt.Errorf("start capture gst-launch: %w", err)
10171077
}
1078+
_ = pipes.sourceOutput.Close()
1079+
_ = pipes.sourceErrOut.Close()
10181080
_ = pwFd.Close() // source child inherited it
10191081

1020-
go logStderr("GST-SOURCE", sourceStderr)
1021-
go logStderr("GST-ENCODER", encoderStderr)
1082+
go func() {
1083+
defer pipes.sourceStderr.Close()
1084+
logStderr("GST-SOURCE", pipes.sourceStderr)
1085+
}()
1086+
go func() {
1087+
defer pipes.encoderStderr.Close()
1088+
logStderr("GST-ENCODER", pipes.encoderStderr)
1089+
}()
10221090

10231091
capture := &ScreenCapture{
10241092
cmd: cmd,
10251093
sourceCmd: sourceCmd,
1026-
stdout: stdout,
1094+
stdout: pipes.stdout,
10271095
cancel: cancel,
10281096
pwNodeID: nodeID,
10291097
dbusConn: dbusConn,
10301098
waitCh: make(chan struct{}),
10311099
}
10321100
if timestampedOutput {
1033-
capture.frames = newRTPVideoAccessUnitReader(stdout, encoderParts.codec)
1101+
capture.frames = newRTPVideoAccessUnitReader(pipes.stdout, encoderParts.codec)
10341102
}
10351103
go func() {
10361104
type processResult struct {

internal/airplay/capture_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,52 @@ import (
1414
"github.com/godbus/dbus/v5"
1515
)
1616

17+
func TestWaylandEncoderStartFailureClosesEveryPipeDescriptor(t *testing.T) {
18+
before := openDescriptorCount(t)
19+
for i := 0; i < 20; i++ {
20+
source := exec.Command("true")
21+
encoder := exec.Command("/definitely/not/a/doubletake-test-command")
22+
pipes, err := openWaylandSplitPipes(source, encoder)
23+
if err != nil {
24+
t.Fatalf("openWaylandSplitPipes: %v", err)
25+
}
26+
if _, err := startWaylandEncoder(encoder, pipes); err == nil {
27+
t.Fatal("startWaylandEncoder unexpectedly succeeded")
28+
}
29+
}
30+
after := openDescriptorCount(t)
31+
if after > before {
32+
t.Fatalf("stderr setup failures leaked descriptors: before=%d after=%d", before, after)
33+
}
34+
}
35+
36+
func TestOpenWaylandSplitPipesCloseReleasesEveryDescriptor(t *testing.T) {
37+
before := openDescriptorCount(t)
38+
source := exec.Command("true")
39+
encoder := exec.Command("true")
40+
pipes, err := openWaylandSplitPipes(source, encoder)
41+
if err != nil {
42+
t.Fatalf("openWaylandSplitPipes: %v", err)
43+
}
44+
pipes.close()
45+
after := openDescriptorCount(t)
46+
if after > before {
47+
t.Fatalf("pipe close leaked descriptors: before=%d after=%d", before, after)
48+
}
49+
}
50+
51+
func openDescriptorCount(t *testing.T) int {
52+
t.Helper()
53+
entries, err := os.ReadDir("/proc/self/fd")
54+
if errors.Is(err, os.ErrNotExist) {
55+
t.Skip("/proc/self/fd is unavailable")
56+
}
57+
if err != nil {
58+
t.Fatalf("read /proc/self/fd: %v", err)
59+
}
60+
return len(entries)
61+
}
62+
1763
func TestCapturePreparationCloseReleasesUnstartedResources(t *testing.T) {
1864
portalFD, peerFD, err := os.Pipe()
1965
if err != nil {

0 commit comments

Comments
 (0)