Skip to content

Commit 5e9c407

Browse files
committed
fix: prevent git progress writer race reading stageNumber
Since the progress writer runs in a goroutine, it can call the write function after the git operation has exited and while the next stage is starting, resulting in a race. This fixes the race by using a second variable and closing the writer early. An alternative fix is to rewrite the progress writer to be synchronous.
1 parent 3f054f6 commit 5e9c407

File tree

1 file changed

+16
-4
lines changed

1 file changed

+16
-4
lines changed

envbuilder.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ func Run(ctx context.Context, opts options.Options) error {
116116
newColor(color.FgCyan).Sprintf(cloneOpts.Path),
117117
)
118118

119-
w := git.ProgressWriter(func(line string) { opts.Logger(log.LevelInfo, "#%d: %s", stageNumber, line) })
119+
stageNum := stageNumber
120+
w := git.ProgressWriter(func(line string) { opts.Logger(log.LevelInfo, "#%d: %s", stageNum, line) })
120121
defer w.Close()
121122
cloneOpts.Progress = w
122123

@@ -132,6 +133,8 @@ func Run(ctx context.Context, opts options.Options) error {
132133
opts.Logger(log.LevelError, "Falling back to the default image...")
133134
}
134135

136+
_ = w.Close()
137+
135138
// Always clone the repo in remote repo build mode into a location that
136139
// we control that isn't affected by the users changes.
137140
if opts.RemoteRepoBuildMode {
@@ -146,7 +149,8 @@ func Run(ctx context.Context, opts options.Options) error {
146149
newColor(color.FgCyan).Sprintf(cloneOpts.Path),
147150
)
148151

149-
w := git.ProgressWriter(func(line string) { opts.Logger(log.LevelInfo, "#%d: %s", stageNumber, line) })
152+
stageNum := stageNumber
153+
w := git.ProgressWriter(func(line string) { opts.Logger(log.LevelInfo, "#%d: %s", stageNum, line) })
150154
defer w.Close()
151155
cloneOpts.Progress = w
152156

@@ -158,6 +162,8 @@ func Run(ctx context.Context, opts options.Options) error {
158162
opts.Logger(log.LevelError, "Failed to clone repository for remote repo mode: %s", fallbackErr.Error())
159163
opts.Logger(log.LevelError, "Falling back to the default image...")
160164
}
165+
166+
_ = w.Close()
161167
}
162168
}
163169

@@ -893,7 +899,8 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error)
893899
newColor(color.FgCyan).Sprintf(cloneOpts.Path),
894900
)
895901

896-
w := git.ProgressWriter(func(line string) { opts.Logger(log.LevelInfo, "#%d: %s", stageNumber, line) })
902+
stageNum := stageNumber
903+
w := git.ProgressWriter(func(line string) { opts.Logger(log.LevelInfo, "#%d: %s", stageNum, line) })
897904
defer w.Close()
898905
cloneOpts.Progress = w
899906

@@ -908,6 +915,8 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error)
908915
opts.Logger(log.LevelError, "Failed to clone repository: %s", fallbackErr.Error())
909916
opts.Logger(log.LevelError, "Falling back to the default image...")
910917
}
918+
919+
_ = w.Close()
911920
} else {
912921
cloneOpts, err := git.CloneOptionsFromOptions(opts)
913922
if err != nil {
@@ -920,7 +929,8 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error)
920929
newColor(color.FgCyan).Sprintf(cloneOpts.Path),
921930
)
922931

923-
w := git.ProgressWriter(func(line string) { opts.Logger(log.LevelInfo, "#%d: %s", stageNumber, line) })
932+
stageNum := stageNumber
933+
w := git.ProgressWriter(func(line string) { opts.Logger(log.LevelInfo, "#%d: %s", stageNum, line) })
924934
defer w.Close()
925935
cloneOpts.Progress = w
926936

@@ -932,6 +942,8 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error)
932942
opts.Logger(log.LevelError, "Failed to clone repository for remote repo mode: %s", fallbackErr.Error())
933943
opts.Logger(log.LevelError, "Falling back to the default image...")
934944
}
945+
946+
_ = w.Close()
935947
}
936948
}
937949

0 commit comments

Comments
 (0)