yalue/merged_fs

Data race reading multiple files at the same time

Closed this issue · 3 comments

csmith commented

We're used merged_fs to serve files from disk and fall back to embedded versions if they don't exist.

It works great, but there's a data race which occasionally results in a fatal error:

fatal error: concurrent map writes
fatal error: concurrent map writes

goroutine 67 [running]:
runtime.throw(0xdc0eae, 0x15)
        /usr/lib/go/src/runtime/panic.go:1117 +0x72 fp=0xc00013ed48 sp=0xc00013ed18 pc=0x437f92
runtime.mapassign_faststr(0xcb4b00, 0xc000120e70, 0xc0006981c5, 0x7, 0x0)
        /usr/lib/go/src/runtime/map_faststr.go:291 +0x3d8 fp=0xc00013edb0 sp=0xc00013ed48 pc=0x4160b8
github.com/yalue/merged_fs.(*MergedFS).validatePathPrefix(0xc000120ea0, 0xc0006981c5, 0x27, 0x0, 0x0)
        /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:347 +0x118 fp=0xc00013eea8 sp=0xc00013edb0 pc=0xc1dbd8
github.com/yalue/merged_fs.(*MergedFS).Open(0xc000120ea0, 0xc0006981c5, 0x27, 0xc00013efb0, 0x4696e5, 0xc000582480, 0x200000003)
        /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:406 +0x106 fp=0xc00013ef68 sp=0xc00013eea8 pc=0xc1e1c6
net/http.ioFS.Open(0x12fa900, 0xc000120ea0, 0xc0006981c4, 0x28, 0x0, 0x20, 0x0, 0x0)
        /usr/lib/go/src/net/http/fs.go:760 +0x68 fp=0xc00013efc0 sp=0xc00013ef68 pc=0x7acb48
net/http.(*ioFS).Open(0xc0002cf720, 0xc0006981c4, 0x28, 0x0, 0x0, 0x20, 0x0)
        <autogenerated>:1 +0x5d fp=0xc00013f010 sp=0xc00013efc0 pc=0x81abbd
net/http.serveFile(0x1304f50, 0xc00022e288, 0xc00012bd00, 0x12fc520, 0xc0002cf720, 0xc0006981c4, 0x28, 0xc00022e201)
        /usr/lib/go/src/net/http/fs.go:597 +0x84 fp=0xc00013f1d8 sp=0xc00013f010 pc=0x7abcc4
[...]

goroutine 68 [running]:
runtime.throw(0xdc0eae, 0x15)
        /usr/lib/go/src/runtime/panic.go:1117 +0x72 fp=0xc0004c6d48 sp=0xc0004c6d18 pc=0x437f92
runtime.mapassign_faststr(0xcb4b00, 0xc000120e70, 0xc000736c45, 0x7, 0x0)
        /usr/lib/go/src/runtime/map_faststr.go:291 +0x3d8 fp=0xc0004c6db0 sp=0xc0004c6d48 pc=0x4160b8
github.com/yalue/merged_fs.(*MergedFS).validatePathPrefix(0xc000120ea0, 0xc000736c45, 0x26, 0x0, 0x0)
        /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:347 +0x118 fp=0xc0004c6ea8 sp=0xc0004c6db0 pc=0xc1dbd8
github.com/yalue/merged_fs.(*MergedFS).Open(0xc000120ea0, 0xc000736c45, 0x26, 0xc0004c6fb0, 0x4696e5, 0xc000282a80, 0x200000003)
        /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:406 +0x106 fp=0xc0004c6f68 sp=0xc0004c6ea8 pc=0xc1e1c6
net/http.ioFS.Open(0x12fa900, 0xc000120ea0, 0xc000736c44, 0x27, 0x0, 0x20, 0x0, 0x0)
        /usr/lib/go/src/net/http/fs.go:760 +0x68 fp=0xc0004c6fc0 sp=0xc0004c6f68 pc=0x7acb48
net/http.(*ioFS).Open(0xc0002cf720, 0xc000736c44, 0x27, 0x0, 0x0, 0x20, 0x0)
        <autogenerated>:1 +0x5d fp=0xc0004c7010 sp=0xc0004c6fc0 pc=0x81abbd
net/http.serveFile(0x1304f50, 0xc0007130f8, 0xc0000b6600, 0x12fc520, 0xc0002cf720, 0xc000736c44, 0x27, 0xc000713001)
        /usr/lib/go/src/net/http/fs.go:597 +0x84 fp=0xc0004c71d8 sp=0xc0004c7010 pc=0x7abcc4
net/http.(*fileHandler).ServeHTTP(0xc0002cf730, 0x1304f50, 0xc0007130f8, 0xc0000b6600)
        /usr/lib/go/src/net/http/fs.go:848 +0x9c fp=0xc0004c7228 sp=0xc0004c71d8 pc=0x7ad39c

Running with the race detector enabled shows these two warnings about a concurrent read/write in validatePathPrefix, and a concurrent write/write which is the same as the fatal error above.

==================
WARNING: DATA RACE
Write at 0x00c000219230 by goroutine 24:
  runtime.mapassign_faststr()
      /usr/lib/go/src/runtime/map_faststr.go:202 +0x0
  github.com/yalue/merged_fs.(*MergedFS).validatePathPrefix()
      /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:347 +0x184
  github.com/yalue/merged_fs.(*MergedFS).Open()
      /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:406 +0x13a
  net/http.ioFS.Open()
      /usr/lib/go/src/net/http/fs.go:760 +0x84
  net/http.(*ioFS).Open()
      <autogenerated>:1 +0x8b
  net/http.serveFile()
      /usr/lib/go/src/net/http/fs.go:597 +0xdd
[...]

Previous read at 0x00c000219230 by goroutine 37:
  runtime.mapaccess1_faststr()
      /usr/lib/go/src/runtime/map_faststr.go:12 +0x0
  github.com/yalue/merged_fs.(*MergedFS).validatePathPrefix()
      /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:316 +0xa4
  github.com/yalue/merged_fs.(*MergedFS).Open()
      /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:406 +0x13a
  net/http.ioFS.Open()
      /usr/lib/go/src/net/http/fs.go:760 +0x84
  net/http.(*ioFS).Open()
      <autogenerated>:1 +0x8b
  net/http.serveFile()
      /usr/lib/go/src/net/http/fs.go:597 +0xdd
[...]
==================

and

==================
WARNING: DATA RACE
Write at 0x00c0008be128 by goroutine 24:
  github.com/yalue/merged_fs.(*MergedFS).validatePathPrefix()
      /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:347 +0x19c
  github.com/yalue/merged_fs.(*MergedFS).Open()
      /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:406 +0x13a
  net/http.ioFS.Open()
      /usr/lib/go/src/net/http/fs.go:760 +0x84
  net/http.(*ioFS).Open()
      <autogenerated>:1 +0x8b
  net/http.serveFile()
      /usr/lib/go/src/net/http/fs.go:597 +0xdd
[...]

Previous write at 0x00c0008be128 by goroutine 37:
  github.com/yalue/merged_fs.(*MergedFS).validatePathPrefix()
      /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:347 +0x19c
  github.com/yalue/merged_fs.(*MergedFS).Open()
      /home/chris/go/pkg/mod/github.com/yalue/merged_fs@v1.0.3/merged_fs.go:406 +0x13a
  net/http.ioFS.Open()
      /usr/lib/go/src/net/http/fs.go:760 +0x84
  net/http.(*ioFS).Open()
      <autogenerated>:1 +0x8b
  net/http.serveFile()
      /usr/lib/go/src/net/http/fs.go:597 +0xdd
[...]
==================
yalue commented

Good catch, this should be easy to fix, and I should push an update shortly.

yalue commented

Should be fixed with commit 7b96c58, v1.0.4 .

csmith commented

Awesome, thanks for the quick fix :)