Question about misleading "unclosed iterators" error

[justinfx]

Hi. I had a question about some confusing behaviour I saw after I updated a project to use BadgerDB instead of BoltDB. I had some crashes in my unittests and the error message seemed to be a bit misleading: https://gist.github.com/justinfx/37d2d2a3d44a9e8f97a1adad4bee824c

=== RUN   TestBadgerClose
--- FAIL: TestBadgerClose (0.63s)
	badger_close_test:62 I am a failed test
panic: runtime error: invalid memory address or nil pointer dereference
	panic: Unclosed iterator at time of Txn.Discard. [recovered]
	panic: Unclosed iterator at time of Txn.Discard.
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x7b3162]

The problem is that my defers are set up in a way that the Close would happen before the Update. I get that this is wrong. But the error message is talking about unclosed iterators. Is this a correct error? I kept looking for places where I didn't close an iterator, but finally figured out I was closing the db in the wrong order.

(I tried asking this first in the dgraph slack, but there doesn't seem to be anyone there)

[manishrjain]

Given that Go doesn't allow applications to directly control object destruction, it's hard to plan for what sort of error you'd run into if you call Update after db.Close.

I removed the panic that you're seeing, and still saw this stack trace which is also present in your logs. The nil pointer dereference gave it away for me. Not sure if there's something we can do.

2018/07/09 16:25:39 I am a failed test
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x70b722]

goroutine 1 [running]:
github.com/dgraph-io/badger/skl.(*Skiplist).IncrRef(...)
	/home/mrjn/go/src/github.com/dgraph-io/badger/skl/skl.go:86
github.com/dgraph-io/badger.(*DB).getMemTables(0xc420013500, 0x0, 0x0, 0x0, 0x0)
	/home/mrjn/go/src/github.com/dgraph-io/badger/db.go:472 +0xd2
github.com/dgraph-io/badger.(*Txn).NewIterator(0xc4254ee5a0, 0x0, 0x64, 0x0, 0x0)
	/home/mrjn/go/src/github.com/dgraph-io/badger/iterator.go:324 +0x68
main.main.func1.1(0xc4254ee5a0, 0x0, 0x0)
	/tmp/t/main.go:29 +0x68
github.com/dgraph-io/badger.(*DB).Update(0xc420013500, 0x81ada8, 0x0, 0x0)
	/home/mrjn/go/src/github.com/dgraph-io/badger/transaction.go:579 +0x84
main.main.func1(0xc420013500, 0xc420059e70)
	/tmp/t/main.go:26 +0x3b
main.main()
	/tmp/t/main.go:60 +0x323

[justinfx]

Thanks for the reply. I don't know alot about the underlying checks badger is doing, which is why I wondered if it was wrongly thinking iterators werent closed. Just was a red herring that made it more difficult to find the original bug.