Panic when PIndexImplType New function returns nil Dest, nil error

[couchbase]

Panic when PIndexImplType New function returns nil Dest, nil error

[tleyden]

Actually, it looks like in this case the RegisterPIndexImplType.Open function is returning nil, nil, nil. If OTOH it returns nil, nil, err then there is no panic.

[tleyden]

I guess I should probably post which CBGT commit I'm using, eh?

Looks like it's a fairly old commit: 7e6a6dc

I will see if I can reproduce on the current master branch.

[tleyden]

Reproduced on CBGT commit 17117fb

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x48 pc=0x41be42e]

goroutine 123 [running]:
github.com/couchbase/cbgt.(*DCPFeed).GetMetaData.func1(0x0, 0x0)
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/cbgt/feed_dcp.go:477 +0x12e
github.com/couchbase/cbgt.Timer.func1()
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/cbgt/misc.go:234 +0x25
github.com/rcrowley/go-metrics.(*StandardTimer).Time(0xc8205bc7b0, 0xc820440e60)
    /Users/tleyden/Development/sync_gateway/src/github.com/rcrowley/go-metrics/timer.go:212 +0x3d
github.com/couchbase/cbgt.Timer(0xc820518f00, 0x52a97a8, 0xc8205bc7b0, 0x0, 0x0)
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/cbgt/misc.go:235 +0xa7
github.com/couchbase/cbgt.(*DCPFeed).GetMetaData(0xc820074270, 0x2d, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/cbgt/feed_dcp.go:480 +0x123
github.com/couchbase/go-couchbase/cbdatasource.(*bucketDataSource).getVBucketMetaData(0xc820620900, 0x2d, 0x60, 0x4878dc0, 0x0, 0x0)
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/go-couchbase/cbdatasource/cbdatasource.go:1235 +0xa2
github.com/couchbase/go-couchbase/cbdatasource.(*bucketDataSource).sendStreamReq(0xc820620900, 0xc8203647e0, 0x2d, 0x0, 0x0)
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/go-couchbase/cbdatasource/cbdatasource.go:1275 +0x53
github.com/couchbase/go-couchbase/cbdatasource.(*bucketDataSource).refreshWorker(0xc820620900, 0xc8203647e0, 0xc820518d80, 0xc8200a6580, 0x40, 0x40, 0x0, 0x0)
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/go-couchbase/cbdatasource/cbdatasource.go:1011 +0x84e
github.com/couchbase/go-couchbase/cbdatasource.(*bucketDataSource).worker(0xc820620900, 0xc820414ba0, 0xf, 0xc82016d1a0, 0x0)
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/go-couchbase/cbdatasource/cbdatasource.go:957 +0x11ea
github.com/couchbase/go-couchbase/cbdatasource.(*bucketDataSource).workerStart.func1.1(0x2)
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/go-couchbase/cbdatasource/cbdatasource.go:681 +0x48
github.com/couchbase/go-couchbase/cbdatasource.ExponentialBackoffLoop(0xc8205bd2c0, 0x23, 0xc820591f38, 0x64, 0x3fc00000, 0x7d0)
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/go-couchbase/cbdatasource/cbdatasource.go:1518 +0x2b
github.com/couchbase/go-couchbase/cbdatasource.(*bucketDataSource).workerStart.func1(0xc820620900, 0xc820414ba0, 0xf, 0xc82016d1a0, 0x64, 0xc83fc00000, 0x7d0)
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/go-couchbase/cbdatasource/cbdatasource.go:682 +0x112
created by github.com/couchbase/go-couchbase/cbdatasource.(*bucketDataSource).workerStart
    /Users/tleyden/Development/sync_gateway/src/github.com/couchbase/go-couchbase/cbdatasource/cbdatasource.go:685 +0xb7

[steveyen]

Yep, I agree libraries shouldn't panic, and in this case cbgt is not invoking panic(), but it's a nil pointer dereference.

The feed_dcp line 477 is ...

value, lastSeq, err = dest.OpaqueGet(partition)

So, that means dest is nil.

From tracing the code paths, dest can be nil if the pindex implementation returns a nil for the Dest return value in the New or Open callbacks...

https://github.com/couchbase/cbgt/blob/master/pindex_impl.go#L39
https://github.com/couchbase/cbgt/blob/master/pindex_impl.go#L45

New func(indexType, indexParams, path string, restart func()) (
    PIndexImpl, Dest, error)

Open func(indexType, path string, restart func()) (
    PIndexImpl, Dest, error)

So, @tleyden, wondering if there are any codepaths in sync-gateway's pindex New/Open callbacks that might return a nil Dest and nil error?

[tleyden]

Yeah, it was only a codepath that came up while doing some intermediate changes, but it was returning a nil dest and a nil error.

[steveyen]

Thanks - I'm going to close this as won't fix, then, on the hope that the elite corps who implement new PIndex types (*) always return a non-nil Dest when nil error.

(*) - same folks deserve to receive "PIndex" or "cbgt" t-shirt. :-)

[tleyden]

What about detecting this situation and doing an explicit panic?

The value add is that instead of the meaningless error:

panic: runtime error: invalid memory address or nil pointer dereference

you could return:

panic: runtime error: you returned a nil Dest and nil error, and that's just something I can't deal with