ref: d8fdffb55268464d54558d6f9cd3874b612dc7c7
parent: 2851af0225cdf6c4e47058979cd22949ed6d1fc0
author: Bjørn Erik Pedersen <[email protected]>
date: Tue Feb 13 16:45:51 EST 2018
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
--- a/resource/image.go
+++ b/resource/image.go
@@ -112,6 +112,9 @@
copyToDestinationInit sync.Once
+ // Lock used when creating alternate versions of this image.
+ createMu sync.Mutex
+
imaging *Imaging
hash string
--- a/resource/image_cache.go
+++ b/resource/image_cache.go
@@ -28,7 +28,8 @@
absCacheDir string
pathSpec *helpers.PathSpec
mu sync.RWMutex
- store map[string]*Image
+
+ store map[string]*Image
}
func (c *imageCache) isInCache(key string) bool {@@ -69,6 +70,11 @@
}
// 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
--- a/resource/image_test.go
+++ b/resource/image_test.go
@@ -15,8 +15,12 @@
import (
"fmt"
+ "math/rand"
+ "strconv"
"testing"
+ "sync"
+
"github.com/stretchr/testify/require"
)
@@ -141,6 +145,51 @@
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{}{@@ -207,4 +256,23 @@
assert.NoError(err)
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)
+ }
+ }
+ })
}
--- a/resource/testhelpers_test.go
+++ b/resource/testhelpers_test.go
@@ -6,8 +6,11 @@
"image"
"io"
+ "io/ioutil"
"os"
"path"
+ "runtime"
+ "strings"
"github.com/gohugoio/hugo/helpers"
"github.com/gohugoio/hugo/hugofs"
@@ -45,17 +48,53 @@
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 @@
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) {