pipeline panic

[derekperkins]

This has panicked a couple times in the last hour or so running in prod

 panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x1ac3437]

goroutine 2571 [running]:
github.com/buraksezer/olric.(*DMapPipeline).execOnPartition(0xc000621800, {0x29e4ba8, 0xc001650540}, 0xc000c998e0?)
	/Users/derek/go/pkg/mod/github.com/buraksezer/olric@v0.5.1/pipeline.go:483 +0x317
github.com/buraksezer/olric.(*DMapPipeline).Exec.func1()
	/Users/derek/go/pkg/mod/github.com/buraksezer/olric@v0.5.1/pipeline.go:532 +0x6e
golang.org/x/sync/errgroup.(*Group).Go.func1()
	/Users/derek/go/pkg/mod/golang.org/x/sync@v0.1.0/errgroup/errgroup.go:75 +0x64

[buraksezer]

Hello @derekperkins,

I couldn't manage to reproduce the problem. It's very likely there is a nil item in the list of commands. pipe cannot be nil. I reviewed the code. I couldn't find any reason for nil commands. There can be a subtle bug in the command pool logic. Do you use Discard?

func (dp *DMapPipeline) execOnPartition(ctx context.Context, partID uint64) error {
	rc, err := dp.dm.clusterClient.clientByPartID(partID)
	if err != nil {
		return err
	}
	// There is no need to protect dp.commands map and its content.
	// It's already filled before running Exec, and it's now a read-only
	// data structure
	commands := dp.commands[partID]
	pipe := rc.Pipeline()

	for _, cmd := range commands {
		pipe.Do(ctx, cmd.Args()...) --> panics
	}

Could you share a code snippet to reproduce the problem?

[buraksezer]

Here is my hypothesis:

execOnPartition assumes that dp.commands is read-only during the execution. Neither dp.commands nor its content is protected by a mutex during the execution. If your code calls Exec and Discard simultaneously, execOnPartition may encounter nil items in dp.commands.

Discard calls dp.closedCancel() at line 548, if Exec is already passed the context check at line 503, they can start running parallel.

[derekperkins]

Yes, we're using Discard and reusing pipelines, but we're not calling Exec and Discard simultaneously on our own accord. This is simplified, but basically this is what we're doing.

defer pipeline.Discard()

pipeline.Exec(ctx)

for _, futureGet := range futureGets {
    futureGet.Result().Scan()
}

I'm wondering if there's a race condition when context is canceled...

[buraksezer]

I'm wondering if there's a race condition when context is canceled...

I believe the possible root cause is this.