From efa24a44a0bb05a6e72eef5c45091fa3f8a3a310 Mon Sep 17 00:00:00 2001 From: Erik Sipsma Date: Thu, 7 Apr 2022 17:23:54 -0700 Subject: [PATCH] client: fix deadlock when connect to buildkitd fails Before this change, if BUILDKIT_HOST was set to an invalid value that resulted in the connection of the grpc client to the server to fail, then deadlock occured waiting for the `eventsCh` to be closed. This happened because the call to the buildkit client's Build method returned an error before the provided callback was executed, which is where the `eventsCh` gets closed. This change places the creation of `eventsCh` inside the Build callback, which avoids the increment of the WaitGroup and thus the deadlock in this error path. Signed-off-by: Erik Sipsma --- client/client.go | 12 ++++++------ tests/plan.bats | 10 ++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/client/client.go b/client/client.go index f24be978..8efb7023 100644 --- a/client/client.go +++ b/client/client.go @@ -189,12 +189,6 @@ func (c *Client) buildfn(ctx context.Context, pctx *plancontext.Context, fn DoFu wg.Done() } - // Catch solver's events - // Closed manually - eventsCh := make(chan *bk.SolveStatus) - wg.Add(1) - go catchOutput(eventsCh) - // Catch build events // Closed by buildkit buildCh := make(chan *bk.SolveStatus) @@ -202,6 +196,12 @@ func (c *Client) buildfn(ctx context.Context, pctx *plancontext.Context, fn DoFu go catchOutput(buildCh) resp, err := c.c.Build(ctx, opts, "", func(ctx context.Context, gw bkgw.Client) (*bkgw.Result, error) { + // Catch solver's events + // Closed by solver.Stop + eventsCh := make(chan *bk.SolveStatus) + wg.Add(1) + go catchOutput(eventsCh) + s := solver.New(solver.Opts{ Control: c.c, Gateway: gw, diff --git a/tests/plan.bats b/tests/plan.bats index 79f9880d..25c77106 100644 --- a/tests/plan.bats +++ b/tests/plan.bats @@ -248,3 +248,13 @@ setup() { assert_failure assert_output --partial "no match for platform in manifest" } + +@test "plan/do: invalid BUILDKIT_HOST results in error" { + cd "$TESTDIR" + + # ip address is in a reserved range that should be unroutable + export BUILDKIT_HOST=tcp://192.0.2.1:1234 + run timeout 30 "$DAGGER" "do" -p ./plan/do/actions.cue test + assert_failure + assert_output --partial "Unavailable: connection error" +}