Skip to content

Commit 5a47abf

Browse files
djdvguseggert
authored andcommitted
fix: exec deadlock when emitter is not Typer intf
1 parent 4ade007 commit 5a47abf

File tree

2 files changed

+68
-20
lines changed

2 files changed

+68
-20
lines changed

executor.go

+34-20
Original file line numberDiff line numberDiff line change
@@ -50,32 +50,46 @@ func (x *executor) Execute(req *Request, re ResponseEmitter, env Environment) er
5050
return err
5151
}
5252
}
53+
maybeStartPostRun := func(formatters PostRunMap) <-chan error {
54+
var (
55+
postRun func(Response, ResponseEmitter) error
56+
postRunCh = make(chan error)
57+
)
5358

54-
// contains the error returned by PostRun
55-
errCh := make(chan error, 1)
56-
if cmd.PostRun != nil {
57-
if typer, ok := re.(interface {
58-
Type() PostRunType
59-
}); ok && cmd.PostRun[typer.Type()] != nil {
60-
var (
61-
res Response
62-
lower = re
63-
)
64-
65-
re, res = NewChanResponsePair(req)
59+
if postRun == nil {
60+
close(postRunCh)
61+
return postRunCh
62+
}
6663

67-
go func() {
68-
defer close(errCh)
69-
errCh <- lower.CloseWithError(cmd.PostRun[typer.Type()](res, lower))
70-
}()
64+
// check if we have a formatter for this emitter type
65+
typer, isTyper := re.(interface {
66+
Type() PostRunType
67+
})
68+
if isTyper &&
69+
formatters[typer.Type()] != nil {
70+
postRun = formatters[typer.Type()]
71+
} else {
72+
close(postRunCh)
73+
return postRunCh
7174
}
72-
} else {
73-
// not using this channel today
74-
close(errCh)
75+
76+
// redirect emitter to us
77+
// and start waiting for emissions
78+
var (
79+
postRes Response
80+
postEmitter = re
81+
)
82+
re, postRes = NewChanResponsePair(req)
83+
go func() {
84+
defer close(postRunCh)
85+
postRunCh <- postEmitter.CloseWithError(postRun(postRes, postEmitter))
86+
}()
87+
return postRunCh
7588
}
7689

90+
postRunCh := maybeStartPostRun(cmd.PostRun)
7791
runCloseErr := re.CloseWithError(cmd.Run(req, re, env))
78-
postCloseErr := <-errCh
92+
postCloseErr := <-postRunCh
7993
switch runCloseErr {
8094
case ErrClosingClosedEmitter, nil:
8195
default:

executor_test.go

+34
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,37 @@ func TestExecutorError(t *testing.T) {
8080
t.Fatalf("expected error message %q but got: %s", expErr, err)
8181
}
8282
}
83+
84+
func TestExecutorNotTyper(t *testing.T) {
85+
testCmd := &Command{
86+
Run: func(*Request, ResponseEmitter, Environment) error {
87+
return nil
88+
},
89+
PostRun: PostRunMap{
90+
CLI: func(response Response, emitter ResponseEmitter) error { return nil },
91+
},
92+
}
93+
testRoot := &Command{
94+
Subcommands: map[string]*Command{
95+
"test": testCmd,
96+
},
97+
}
98+
req, err := NewRequest(context.Background(), []string{"test"}, nil, nil, nil, testRoot)
99+
if err != nil {
100+
t.Fatal(err)
101+
}
102+
103+
emitter, resp := NewChanResponsePair(req)
104+
105+
x := NewExecutor(testRoot)
106+
107+
err = x.Execute(req, emitter, nil)
108+
if err != nil {
109+
t.Fatal(err)
110+
}
111+
112+
_, err = resp.Next()
113+
if err != io.EOF {
114+
t.Fatalf("expected EOF but got: %s", err)
115+
}
116+
}

0 commit comments

Comments
 (0)