resource: Fix multi-threaded image processing issue

When doing something like this with the same image from a partial used in, say, both the home page and the single page:

```bash
{{ with $img }}
{{ $big := .Fill "1024x512 top" }}
{{ $small := $big.Resize "512x" }}
{{ end }}
```

There would be timing issues making Hugo in some cases try to process the same image with the same instructions in parallel.

You would experience errors of type:

```bash
png: invalid format: not enough pixel data
```

This commit works around that by adding a mutex per image. This should also improve the performance, sligthly, as it avoids duplicate work.

The current workaround before this fix is to always operate on the original:

```bash
{{ with $img }}
{{ $big := .Fill "1024x512 top" }}
{{ $small := .Fill "512x256 top" }}
{{ end }}
```

Fixes #4404

resource/image.go

@@ -112,6 +112,9 @@ type Image struct {
 
 	copyToDestinationInit sync.Once
 
+	// Lock used when creating alternate versions of this image.
+	createMu sync.Mutex
+
 	imaging *Imaging
 
 	hash string

resource/image_cache.go

@@ -28,6 +28,7 @@ type imageCache struct {
 	absCacheDir   string
 	pathSpec      *helpers.PathSpec
 	mu            sync.RWMutex
+
 	store map[string]*Image
 }
 
@@ -69,6 +70,11 @@ func (c *imageCache) getOrCreate(
 	}
 
 	// Now look in the file cache.
+	// Multiple Go routines can invoke same operation on the same image, so
+	// we need to make sure this is serialized per source image.
+	parent.createMu.Lock()
+	defer parent.createMu.Unlock()
+
 	cacheFilename := filepath.Join(c.absCacheDir, key)
 
 	// The definition of this counter is not that we have processed that amount

resource/image_test.go

@@ -15,8 +15,12 @@ package resource
 
 import (
 	"fmt"
+	"math/rand"
+	"strconv"
 	"testing"
 
+	"sync"
+
 	"github.com/stretchr/testify/require"
 )
 
@@ -141,6 +145,51 @@ func TestImageTransformLongFilename(t *testing.T) {
 	assert.Equal("/a/_hu59e56ffff1bc1d8d122b1403d34e039f_90587_c876768085288f41211f768147ba2647.jpg", resized.RelPermalink())
 }
 
+func TestImageTransformConcurrent(t *testing.T) {
+
+	var wg sync.WaitGroup
+
+	assert := require.New(t)
+
+	spec := newTestResourceOsFs(assert)
+
+	image := fetchImageForSpec(spec, assert, "sunset.jpg")
+
+	for i := 0; i < 4; i++ {
+		wg.Add(1)
+		go func(id int) {
+			defer wg.Done()
+			for j := 0; j < 5; j++ {
+				img := image
+				for k := 0; k < 2; k++ {
+					r1, err := img.Resize(fmt.Sprintf("%dx", id-k))
+					if err != nil {
+						t.Fatal(err)
+					}
+
+					if r1.Width() != id-k {
+						t.Fatalf("Width: %d:%d", r1.Width(), j)
+					}
+
+					r2, err := r1.Resize(fmt.Sprintf("%dx", id-k-1))
+					if err != nil {
+						t.Fatal(err)
+					}
+
+					_, err = r2.decodeSource()
+					if err != nil {
+						t.Fatal("Err decode:", err)
+					}
+
+					img = r1
+				}
+			}
+		}(i + 20)
+	}
+
+	wg.Wait()
+}
+
 func TestDecodeImaging(t *testing.T) {
 	assert := require.New(t)
 	m := map[string]interface{}{
@@ -208,3 +257,22 @@ func TestImageWithMetadata(t *testing.T) {
 	assert.Equal("Sunset #1", resized.Name())
 
 }
+
+func BenchmarkResizeParallel(b *testing.B) {
+	assert := require.New(b)
+	img := fetchSunset(assert)
+
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			w := rand.Intn(10) + 10
+			resized, err := img.Resize(strconv.Itoa(w) + "x")
+			if err != nil {
+				b.Fatal(err)
+			}
+			_, err = resized.Resize(strconv.Itoa(w-1) + "x")
+			if err != nil {
+				b.Fatal(err)
+			}
+		}
+	})
+}

resource/testhelpers_test.go

@@ -6,8 +6,11 @@ import (
 
 	"image"
 	"io"
+	"io/ioutil"
 	"os"
 	"path"
+	"runtime"
+	"strings"
 
 	"github.com/gohugoio/hugo/helpers"
 	"github.com/gohugoio/hugo/hugofs"
@@ -45,17 +48,53 @@ func newTestResourceSpecForBaseURL(assert *require.Assertions, baseURL string) *
 	return spec
 }
 
+func newTestResourceOsFs(assert *require.Assertions) *Spec {
+	cfg := viper.New()
+	cfg.Set("baseURL", "https://example.com")
+
+	workDir, err := ioutil.TempDir("", "hugores")
+
+	if runtime.GOOS == "darwin" && !strings.HasPrefix(workDir, "/private") {
+		// To get the entry folder in line with the rest. This its a little bit
+		// mysterious, but so be it.
+		workDir = "/private" + workDir
+	}
+
+	contentDir := "base"
+	cfg.Set("workingDir", workDir)
+	cfg.Set("contentDir", contentDir)
+	cfg.Set("resourceDir", filepath.Join(workDir, "res"))
+
+	fs := hugofs.NewFrom(hugofs.Os, cfg)
+	fs.Destination = &afero.MemMapFs{}
+
+	s, err := helpers.NewPathSpec(fs, cfg)
+
+	assert.NoError(err)
+
+	spec, err := NewSpec(s, media.DefaultTypes)
+	assert.NoError(err)
+	return spec
+
+}
+
 func fetchSunset(assert *require.Assertions) *Image {
 	return fetchImage(assert, "sunset.jpg")
 }
 
 func fetchImage(assert *require.Assertions, name string) *Image {
+	spec := newTestResourceSpec(assert)
+	return fetchImageForSpec(spec, assert, name)
+}
+
+func fetchImageForSpec(spec *Spec, assert *require.Assertions, name string) *Image {
 	src, err := os.Open("testdata/" + name)
 	assert.NoError(err)
 
-	spec := newTestResourceSpec(assert)
+	workingDir := spec.Cfg.GetString("workingDir")
+	f := filepath.Join(workingDir, name)
 
-	out, err := spec.Fs.Source.Create("/b/" + name)
+	out, err := spec.Fs.Source.Create(f)
 	assert.NoError(err)
 	_, err = io.Copy(out, src)
 	out.Close()
@@ -66,11 +105,10 @@ func fetchImage(assert *require.Assertions, name string) *Image {
 		return path.Join("/a", s)
 	}
 
-	r, err := spec.NewResourceFromFilename(factory, "/public", "/b/"+name, name)
+	r, err := spec.NewResourceFromFilename(factory, "/public", f, name)
 	assert.NoError(err)
 	assert.IsType(&Image{}, r)
 	return r.(*Image)
-
 }
 
 func assertFileCache(assert *require.Assertions, fs *hugofs.Fs, filename string, width, height int) {