Add alternative PollEvents() impl to prevent goroutine leaking

[andrievsky]

Add PollEventsWithContext(ctx context.Context) as an alternative to PollEvents() which prevents goroutine leaking

[maxatome]

Suggested change

	default:
	ch <- convertTermboxEvent(tb.PollEvent())
	case ch <- convertTermboxEvent(tb.PollEvent()):

is better, otherwise while you are waiting for ch input, cancelling the context is not efective.

[dolmen]

@maxatome The core problem is that tb.PollEvent() is blocking.
This suggestion would not fix that what you claim as it would just handle a cancel during the write on ch after an event has been blockingly read.

[andrievsky]

@maxatome The core problem is that tb.PollEvent() is blocking.
This suggestion would not fix that what you claim as it would just handle a cancel during the write on ch after an event has been blockingly read.

Yes, I had this in mind.

[andrievsky]

Do you have any suggestions how to make it better?

[maxatome]

Good point @dolmen :)

What about a specific goroutine to handle this blocking function?

Something like that (not tested in termui context):

func PollEventsWithContext(ctx context.Context) <-chan Event {
	// tb.PollEvent() is a blocking function, so dedicate a goroutine for it
	tbEvent := make(chan Event)
	go func() {
		defer close(tbEvent)
		for {
			ev := tb.PollEvent()
			if ev.Type == tb.EventInterrupt {
				return
			}
			tbEvent <- convertTermboxEvent(ev)
		}
	}()

	ch := make(chan Event)
	go func() {
		defer close(ch)
		ctxDone := ctx.Done()
		for {
			select {
			case ctxDone:
				// interrupt tb.PollEvent(), this call blocks until the
				// PollEvent function has successfully been interrupted
				tb.Interrupt()
				ctxDone = nil
			case ev, ok := <-tbEvent:
				// Successfully interrupted
				if !ok {
					return
				}
				ch <- ev
			}
		}
	}()
	return ch
}

[andrievsky]

Should works, however it looks like over usage of go-routines to me. I would like to suggest a simpler way to do it:

func PollEventsWithCancel() (<-chan Event, func()) {
	ch := make(chan Event)
	go func() {
		defer close(ch)
		for {
			ev := tb.PollEvent()
			if ev.Type == tb.EventInterrupt {
				return
			}
			ch <- convertTermboxEvent(ev)
		}
	}()
	return ch, func(){tb.Interrupt()}
}

[andrievsky]

Another concern is that the method should be a singleton. In other way behavior is unpredictable and doesn't really make sense.

[andrievsky]

Let me back to my pet proj after a while and see what can be done, any suggestions are welcome.