Skip to content
  • Daniel Martí's avatar
    db: fix iteration with badger · 3adf3359
    Daniel Martí authored
    There have been two attempts at a fix for the panic below, but I think
    both were wrong.
    
    	panic: runtime error: invalid memory address or nil pointer dereference
    	[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1f41e8a]
    
    	goroutine 191380 [running]:
    	github.com/dgraph-io/badger/v2.(*Iterator).Next(0xc003ebcf00)
    	    /go/pkg/mod/github.com/dgraph-io/badger/v2@v2.0.1-rc1.0.20200409094109-809725940698/iterator.go:554 +0x2a
    	gitlab.com/vocdoni/go-dvote/db.(*BadgerIterator).Next(0xc015e75d80, 0x2ff7900)
    	    /src/db/badger.go:125 +0x33
    	gitlab.com/vocdoni/go-dvote/vochain/scrutinizer.(*Scrutinizer).List(0xc007dd1aa0, 0x40, 0x0, 0x0, 0x2a4fb88, 0x2, 0xc01664d680, 0x4191100, 0xee2200)
    	    /src/vochain/scrutinizer/scrutinizer.go:173 +0xc0
    	gitlab.com/vocdoni/go-dvote/router.(*Router).getProcListResults(0xc020379200, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    	    /src/router/voteCallbacks.go:247 +0xe3
    	created by gitlab.com/vocdoni/go-dvote/router.(*Router).Route
    	    /src/router/router.go:277 +0x408
    
    First, d9b71dbd three days ago. It's not the right fix, because the call
    makes it all the way to badger.Iterator.Next. What's nil is the
    *badger.Iterator, not our wrapper.
    
    Second, 81cb2fff two days ago. It seems wrong, as it means that we no
    longer use the iterator API properly. To quote them:
    
    	Next would advance the iterator by one. Always check it.Valid()
    	after a Next() to ensure you have access to a valid it.Item().
    
    To quote upstream's iterator example, the right way to use an iterator
    is as follows:
    
    	for it.Rewind(); it.Valid(); it.Next() {
    		...
    	}
    
    Since we just do "for iter.Next() { ... }", let's split the logic above
    to clarify it:
    
    	it.Rewind()
    	for it.Valid() {
    		...
    		it.Next()
    	}
    
    That is, there were two things the original code was getting wrong.
    First, it wasn't calling it.Rewind before the first iteration. Second,
    it was calling it.Next before the first iteration. This could lead to
    
    To get the same logic, we add a new boolean field to track if we're at
    the first iteration or not, so that we can know whether to use Rewind or
    Next. We also revert the two previous commits mentioned above.
    
    For the sake of predictability, since the prefixed database doesn't
    implement an iterator, make it panic instead of silently returning nil.
    This shouldn't affect us.
    
    Fixes #228.
    3adf3359
Loading