Skip to content

Conversation

@crazy-max
Copy link
Member

relates to moby/moby#50858

f := getFrontend(t, sb)

subModDir := t.TempDir()
defer os.RemoveAll(subModDir)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed, t.TempDir() is removed when test is completed.

Comment on lines -643 to -645
cl, err := client.New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the actual fix, correct? I was trying to find where the deadlock was, but I guess somewhere in the client.New constructor?

Or is it related to the grpc.DialContext ?

buildkit/client/client.go

Lines 154 to 157 in 9fcedf9

// ignore SA1019 NewClient has different behavior and needs to be tested
//nolint:staticcheck
conn, err := grpc.DialContext(ctx, address, gopts...)
if err != nil {

Copy link
Member Author

@crazy-max crazy-max Sep 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actual fix is cl is not closed so on line 645 defer cl.Close() would work but then cl.Prune later would be useless so we need to reuse the same client.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, gotcha; thanks!

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@crazy-max crazy-max merged commit 8c17bee into moby:master Sep 1, 2025
140 checks passed
@crazy-max crazy-max deleted the dockerfile-fix-test branch September 1, 2025 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants