Potential race/deadlock between broker error and AsyncClose

[eapache]

I was inspecting the consumer code as part of #474 and I realized that there is a potential deadlock condition in the very rare case where a broker returns an error (such as ErrOffsetOutOfRange) at the same time that the user calls AsyncClose (or just Close).

Both events result in a value being written to child.dying which has a buffer for only one event. If the AsyncClose write succeeds, then the responseFeeder write (triggered by the broker error) will block; since the goroutine responsible for reading from that channel is currently waiting on responseFeeder to unblock it, I believe this deadlocks.

cc @Shopify/kafka very unlikely (and entirely theoretical at this point) but I'd appreciate if somebody could verify my logic

[wvanbergen]

Your sequence of events seems plausible to me. Maybe things like this will be a lot easier to reason about using a sync.Once guard around the shutdown logic? That should also fix this I think.

[eapache]

?

the problem is that multiple things can legitimately write to dying, not that one of those writes is illegitimate

[wvanbergen]

Why would we ever want to write more than once to dying? We can only shut down a consumer once.

[wvanbergen]

Never mind; we're not just closeing the channel, but also writing errors to it.