Commit a5d0e83f authored by Kirill Smelkov's avatar Kirill Smelkov

X zodb/fs1: Don't set .file = nil on Close

In case FileStorage will be still used we want to get proper "file is
already closed" error instead of crash.

A particular case of proper usage after Close is to spawn many
prefetches, including ones going after EOF, then after getting some
loads, close the storage as we are done processing.
By definition Prefetch ignores errors, but if FileStorage.file is nil,
e.g. os.File.Name() crashes and so instead of Prefetch ignoring the
error it will crash too.

For example here is how it can happen under neotest:

	Benchmarklink/fs1/zhash.go 1 1.7 µs/object	# null:00  oid=0..2127  nread=8540363  t=3.634083ms	# POLL·0 C1·2 C1E·74 C3·16 C6·0 C7·60
	Benchmarklink/fs1/zhash.go 1 2.2 µs/object	# null:00  oid=0..2127  nread=8540363  t=4.731554ms	# POLL·0 C1·3 C1E·74 C3·18 C6·0 C7·73
	Benchmarklink/fs1/zhash.go 1 1.7 µs/object	# null:00  oid=0..2127  nread=8540363  t=3.689686ms	# POLL·0 C1·17 C1E·70 C3·15 C6·1 C7·88
	Benchmarklink/fs1/zhash.go 1 1.7 µs/object	# null:00  oid=0..2127  nread=8540363  t=3.687432ms	# POLL·1 C1·4 C1E·71 C3·19 C6·0 C7·70
	Benchmarklink/fs1/zhash.go 1 2.3 µs/object	# null:00  oid=0..2127  nread=8540363  t=4.867104ms	# POLL·0 C1·11 C1E·71 C3·22 C6·0 C7·57
	Benchmarklink/fs1/zhash.go+prefetch128 1 6.9 µs/object	# null:00  oid=0..2127  nread=8540363  t=14.655538ms
	panic: runtime error: invalid memory address or nil pointer dereference
	[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x52da01]

	goroutine 2074 [running]:
	os.(*File).Name(...)
		/home/kirr/src/tools/go/go/src/os/file.go:47
	lab.nexedi.com/kirr/neo/go/xcommon/xio.Name(0x5ddb20, 0x0, 0xc420022187, 0x51)
		/home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/xcommon/xio/xio.go:67 +0x381
	lab.nexedi.com/kirr/neo/go/zodb/storage/fs1.(*DataHeader).err(0xc4200221e0, 0x6ddec0, 0x0, 0x5e6509, 0x4, 0x6dd580, 0xc420042130, 0xc420042130, 0x0)
		/home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/fs1/format.go:117 +0x42
	lab.nexedi.com/kirr/neo/go/zodb/storage/fs1.(*DataHeader).Load(0xc4200221e0, 0x6ddec0, 0x0, 0x7fc3b5, 0x0, 0x454530)
		/home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/fs1/format.go:471 +0xf5
	lab.nexedi.com/kirr/neo/go/zodb/storage/fs1.(*DataHeader).loadPrevRev(0xc4200221e0, 0x6ddec0, 0x0, 0x7fc3b5, 0xc4209d8288, 0xc420872d88)
		/home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/fs1/format.go:540 +0x79
	lab.nexedi.com/kirr/neo/go/zodb/storage/fs1.(*DataHeader).LoadPrevRev(0xc4200221e0, 0x6ddec0, 0x0, 0x5d0c60, 0xc4200221e0)
		/home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/fs1/format.go:526 +0x60
	lab.nexedi.com/kirr/neo/go/zodb/storage/fs1.(*FileStorage)._Load(0xc420090360, 0xc4200221e0, 0x3c4918e0f64d888, 0x7f6, 0x0, 0x0, 0x0, 0x0)
		/home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/fs1/filestorage.go:162 +0x48
	lab.nexedi.com/kirr/neo/go/zodb/storage/fs1.(*FileStorage).Load(0xc420090360, 0x6dfa80, 0xc42006a780, 0x3c4918e0f64d888, 0x7f6, 0x0, 0x0, 0x0, 0x0)
		/home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/fs1/filestorage.go:154 +0xb7
	lab.nexedi.com/kirr/neo/go/zodb.(*Cache).loadRCE(0xc4200101c0, 0x6dfa80, 0xc42006a780, 0xc4209e6900)
		/home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/cache.go:313 +0x89
	created by lab.nexedi.com/kirr/neo/go/zodb.(*Cache).Prefetch
		/home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/cache.go:205 +0x9f
	# POLL·0 C1·155 C1E·79 C3·31 C6·0 C7·42
	E: abnormal termination - stopping...

So don't set .file = nil after Close.

Not adding a comment explaining this as leaving things that still might
be used concurrently intact should be common practice.
parent a971231c
......@@ -503,7 +503,6 @@ func (fs *FileStorage) Close() error {
if err != nil {
return err
}
fs.file = nil
// TODO if opened !ro -> .saveIndex()
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment