• Kirill Smelkov's avatar
    X zodb/fs1: Don't set .file = nil on Close · a5d0e83f
    Kirill Smelkov authored
    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.
    a5d0e83f
filestorage.go 15 KB