emitter

emitter copied to clipboard

Correct me if I'm wrong, but it seems that the below example that's currently on the README.md has a small chance of panicking.

done := e.Emit("broadcast", "the", "event", "with", "timeout")

select {
case <-done:
	// so the sending is done
case <-time.After(timeout):
	// time is out, let's discard emitting
	close(done)
}

If timeout happens at the same time when Emit closes the done channel, the done channel can be closed twice, which will cause a panic. To illustrate the point, the below code causes a panic reproducibly.

e := emitter.Emitter{}
ch := e.On("broadcast")

done := e.Emit("broadcast", "the", "event", "with", "timeout")

go func() {
	time.Sleep(200 * time.Millisecond)
	fmt.Println("this happens second")
	<-ch
}()

select {
case <-done:
case <-time.After(100 * time.Millisecond):
	fmt.Println("this happens first")
	time.Sleep(200 * time.Millisecond)
	fmt.Println("this happens third")
	close(done)
}

Hey @Gaboose, good catch! Would you like to send a PR? 🙂

Hi @olebedev! Maybe if I find a free minute or two :) but it's not straight forward.

I think fixing this would require for Emit to return something other than a chan struct{}, because closing a channel on the receiver's side will always have this race condition, I think. But Emit could return a struct with two channels.

type EmitOp struct {
    done chan struct{}
    cancelled chan struct{}
}

func (op EmitOp) Done() <-chan struct{} { return op.done }
func (op EmitOp) Cancel() { close(op.cancelled) )

So it would be an API breaking change. What do you think? Maybe there's some other better solution?

An alternative for this could be an EmitCtx type api

e := emitter.Emitter{}
ch := e.On("broadcast")

ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancel()
done := e.EmitCtx(ctx, "broadcast", "the", "event", "with", "timeout")

select {
case <-done:
    fmt.Println("Event sent")
case <-ctx.Done():
    fmt.Println("Event send timed out")
}