Skip to content

Commit

Permalink
Prevent docker data race with version negotiation
Browse files Browse the repository at this point in the history
  • Loading branch information
orlangure committed May 18, 2021
1 parent fdc2a23 commit 38d01a2
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 3 deletions.
22 changes: 20 additions & 2 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"regexp"
"strconv"
"sync"
"time"

"github.com/docker/docker/api/types"
Expand All @@ -31,19 +32,30 @@ const (
type docker struct {
client *client.Client
log *zap.SugaredLogger

// This lock is used to protect docker client from concurrent connections
// with version negotiation. As of this moment, there is a data race in
// docker client when version negotiation is requested. This data race is
// not dangerous, but it triggers race detector alarms, so it should be
// avoided. Currently the client still has this issue, so this is an
// attempt to fix it locally by preventing concurrent connection using the
// same client (mostly when `Stop` is called with multiple containers).
//
// https://github.com/moby/moby/pull/42379
lock sync.Mutex
}

func (g *g) dockerConnect() (*docker, error) {
g.log.Info("connecting to docker engine")

cli, err := client.NewClientWithOpts(client.FromEnv)
cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())
if err != nil {
return nil, fmt.Errorf("%w: %s", ErrEnvClient, err)
}

g.log.Info("connected to docker engine")

return &docker{cli, g.log}, nil
return &docker{client: cli, log: g.log}, nil
}

func (d *docker) pullImage(ctx context.Context, image string) error {
Expand Down Expand Up @@ -128,6 +140,9 @@ func (d *docker) startContainer(ctx context.Context, image string, ports NamedPo
}

func (d *docker) waitForContainerNetwork(ctx context.Context, id string, ports NamedPorts) (*Container, error) {
d.lock.Lock()
defer d.lock.Unlock()

d.log.Infow("waiting for container network", "container", id)

tick := time.NewTicker(time.Millisecond * 250)
Expand Down Expand Up @@ -303,6 +318,9 @@ func (d *docker) readLogs(ctx context.Context, id string) (io.ReadCloser, error)
}

func (d *docker) stopContainer(ctx context.Context, id string) error {
d.lock.Lock()
defer d.lock.Unlock()

stopTimeout := defaultStopTimeout

err := d.client.ContainerStop(ctx, id, &stopTimeout)
Expand Down
2 changes: 1 addition & 1 deletion gnomock.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func newG(debug bool) (*g, error) {

l = l.With(zap.String("id", id.String()))

return &g{id, l.Sugar()}, nil
return &g{id: id, log: l.Sugar()}, nil
}

// g is a Gnomock operation wrapper, mostly for debug purposes.
Expand Down

0 comments on commit 38d01a2

Please # to comment.