Undefined concurrent writes in BuildDockerfiles

[mtrmac]

Steps to reproduce the issue:
Code inspection only

buildah/imagebuildah/build.go

Line 65 in d8e9ca4

func BuildDockerfiles(ctx context.Context, store storage.Store, options define.BuildOptions, paths ...string) (id string, ref reference.Canonical, err error) {

defines return values id and ref, shared for the whole BuildDockerfiles function.

buildah/imagebuildah/build.go

Line 255 in d8e9ca4

builds.Go(func() error {

starts a new goroutine, and that writes to id and ref at

buildah/imagebuildah/build.go

Line 278 in d8e9ca4

id, ref = thisID, thisRef

without anything I can see that would prevent concurrent writes. That can have, in principle, unpredictable results (starting with getting id and ref values from different goroutines).

I suppose the idea is that on single-platform builds there is no race, and on multi-platform builds the fields are not used anyway. (Pedantically: Go is fairly imprecise about its memory model, so it’s unclear to me that “racy but unused writes” don’t trigger undefined behavior.) I can’t see that this is actually the case, nothing forces multi-platform builds to set the Manifest option which would cause id and ref to be overwritten.

Moving those writes under (renamed) instanceLock might be a one-line fix … but it seems worthwhile to move the per-platform goroutine into a completely separate function to make it impossible to similarly write to shared state without making it explicitly visible to the goroutine.

[flouthoc]

I think we can avoid sharing these variable inside goroutines. Created a refactor PR above: #4120