Skip to content

Commit

Permalink
fix: avoid possible datarace in startCLISession (#6453)
Browse files Browse the repository at this point in the history
It was previously possible for stderr to still be being written to while
we try and get the logs for it.

Signed-off-by: Justin Chadwell <me@jedevc.com>
  • Loading branch information
jedevc authored Jan 24, 2024
1 parent 1903eba commit 6843245
Showing 1 changed file with 19 additions and 2 deletions.
21 changes: 19 additions & 2 deletions internal/engineconn/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func startCLISession(ctx context.Context, binPath string, cfg *Config) (_ Engine
// threads within this process are trying to provision it at the same time.
var proc *exec.Cmd
var stdout io.ReadCloser
var stderrBuf *bytes.Buffer
var stderrBuf *safeBuffer
var childStdin io.WriteCloser

if cfg.LogOutput != nil {
Expand Down Expand Up @@ -132,7 +132,7 @@ func startCLISession(ctx context.Context, binPath string, cfg *Config) (_ Engine
// of this function so we can return it in the error if something
// goes wrong here. Otherwise the only error ends up being EOF and
// the user has to enable log output to see anything.
stderrBuf = bytes.NewBuffer(nil)
stderrBuf = &safeBuffer{}
discardableBuf := &discardableWriter{w: stderrBuf}
go io.Copy(io.MultiWriter(cfg.LogOutput, discardableBuf), stderrPipe)
defer discardableBuf.Discard()
Expand Down Expand Up @@ -246,3 +246,20 @@ func (w *discardableWriter) Discard() {
defer w.mu.Unlock()
w.w = io.Discard
}

type safeBuffer struct {
bu bytes.Buffer
mu sync.Mutex
}

func (s *safeBuffer) Write(p []byte) (n int, err error) {
s.mu.Lock()
defer s.mu.Unlock()
return s.bu.Write(p)
}

func (s *safeBuffer) String() string {
s.mu.Lock()
defer s.mu.Unlock()
return s.bu.String()
}

0 comments on commit 6843245

Please sign in to comment.