Skip to content

Commit 02fc78b

Browse files
jul-shjblebrun
andauthored
fix: deadlock and improve debugging experience (#3570)
# Summary Fixes #3571. Fixes #3569. Shoutout to @jblebrun who wrote most of the code in this PR and debugged this issue together with me. ## Testing Process ... ## Checklist - [ ] Review the contributing [guidelines](./../CONTRIBUTING.md) - [ ] Add a reference to related issues in the PR description. - [ ] Update documentation if applicable. - [ ] Add unit tests if applicable. - [ ] Add changes to the [CHANGELOG](./../CHANGELOG.md) if applicable. Signed-off-by: Juliette Pretot <[email protected]> Co-authored-by: Jason LeBrun <[email protected]>
1 parent 4534a0b commit 02fc78b

File tree

1 file changed

+77
-22
lines changed

1 file changed

+77
-22
lines changed

internal/builders/docker/pkg/builder.go

+77-22
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ package pkg
2323
// Docker image.
2424

2525
import (
26+
"bufio"
2627
"crypto/sha256"
2728
"encoding/hex"
2829
"encoding/json"
@@ -36,6 +37,7 @@ import (
3637
"path"
3738
"path/filepath"
3839
"strings"
40+
"sync"
3941

4042
intoto "github.com/in-toto/in-toto-golang/in_toto"
4143
slsa1 "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/v1"
@@ -476,34 +478,87 @@ func (c *GitClient) checkoutGitCommit() error {
476478
return nil
477479
}
478480

479-
// saveToTempFile creates a tempfile in `/tmp` and writes the content of the
480-
// given readers to that file.
481-
func saveToTempFile(verbose bool, readers ...io.Reader) ([]string, error) {
482-
var files []string
483-
for _, reader := range readers {
484-
bytes, err := io.ReadAll(reader)
485-
if err != nil {
486-
return files, err
487-
}
481+
type tempFileResult struct {
482+
File *os.File
483+
Err error
484+
}
485+
486+
// A helper function used by saveToTempFile to process one individual file.
487+
// This should be called in a goroutine, and the channels passed in should be owned by the caller,
488+
// and remain open until the goroutine completes.
489+
func saveOneTempFile(verbose bool, reader io.Reader, fileChannel chan tempFileResult, printChannel chan string) {
490+
var allBytes []byte
491+
scanner := bufio.NewScanner(reader)
492+
for scanner.Scan() {
493+
bytes := scanner.Bytes()
494+
allBytes = append(allBytes, bytes...)
495+
allBytes = append(allBytes, '\n')
488496

489497
if verbose {
490-
if len(bytes) > 0 {
491-
fmt.Print("\n\n>>>>>>>>>>>>>> output from command <<<<<<<<<<<<<<\n")
492-
fmt.Printf("%s", bytes)
493-
fmt.Print("=================================================\n\n\n")
494-
}
498+
printChannel <- string(bytes)
495499
}
500+
}
496501

497-
tmpfile, err := os.CreateTemp("", "log-*.txt")
498-
if err != nil {
499-
return files, fmt.Errorf("couldn't create tempfile: %v", err)
500-
}
502+
tmpfile, err := os.CreateTemp("", "log-*.txt")
503+
if err != nil {
504+
fileChannel <- tempFileResult{Err: err}
505+
return
506+
}
507+
defer tmpfile.Close()
508+
509+
if _, err := tmpfile.Write(allBytes); err != nil {
510+
fileChannel <- tempFileResult{Err: fmt.Errorf("couldn't write bytes to tempfile: %v", err)}
511+
} else {
512+
fileChannel <- tempFileResult{File: tmpfile}
513+
}
514+
}
515+
516+
// saveToTempFile creates a tempfile in `/tmp` and writes the content of the
517+
// given readers to that file.
518+
// It processes all provided readers concurrently.
519+
func saveToTempFile(verbose bool, readers ...io.Reader) ([]string, error) {
520+
if verbose {
521+
fmt.Print("\n\n>>>>>>>>>>>>>> output from command <<<<<<<<<<<<<<\n")
522+
}
523+
var wg sync.WaitGroup
524+
// We need to make sure the fileChannel has enough buffere space to hold everything,
525+
// since it won't be processed until the very end.
526+
fileChannel := make(chan tempFileResult, len(readers))
527+
printChannel := make(chan string)
501528

502-
if _, err := tmpfile.Write(bytes); err != nil {
503-
tmpfile.Close()
504-
return files, fmt.Errorf("couldn't write bytes to tempfile: %v", err)
529+
// Start a goroutine to process each Reader concurrently.
530+
for _, reader := range readers {
531+
wg.Add(1)
532+
go func(reader io.Reader) {
533+
defer wg.Done()
534+
saveOneTempFile(verbose, reader, fileChannel, printChannel)
535+
}(reader)
536+
}
537+
538+
// Close the channel once all goroutines have finished.
539+
// We do the waiting in a goroutine so that we can move on
540+
// to the lines below that handle printing the values passed on
541+
// the print channel.
542+
go func() {
543+
wg.Wait()
544+
close(printChannel)
545+
close(fileChannel)
546+
}()
547+
548+
// Print the output as it comes in.
549+
// Once all of the provided readers have been closed, the WaitGroup
550+
// will move to the Done state, completing this loop and allowing us to
551+
// move to the filename collection below.
552+
for line := range printChannel {
553+
fmt.Println(line)
554+
}
555+
556+
var files []string
557+
for result := range fileChannel {
558+
if result.Err != nil {
559+
return nil, result.Err
505560
}
506-
files = append(files, tmpfile.Name())
561+
files = append(files, result.File.Name())
507562
}
508563

509564
return files, nil

0 commit comments

Comments
 (0)