Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibly broken broadcast channel's Recv interface #3420

Open
pdyraga opened this issue Nov 28, 2022 · 0 comments
Open

Possibly broken broadcast channel's Recv interface #3420

pdyraga opened this issue Nov 28, 2022 · 0 comments

Comments

@pdyraga
Copy link
Member

pdyraga commented Nov 28, 2022

The broadcast channel Recv function expects the message handler as a callback:

func (c *channel) Recv(ctx context.Context, handler func(m net.Message)) {

However, in possibly all cases all the handler is doing is writing to a buffered channel. Example:

recvChan := make(chan net.Message, asyncReceiveBuffer)
handler := func(msg net.Message) {
recvChan <- msg
}
am.channel.Recv(recvCtx, handler)

The broadcast channel has no idea what is happening with the receiver. If it takes too long to receive a message, the receiver's handler goroutine gets blocked:

go func() {
for {
select {
case <-ctx.Done():
logger.Debug("context is done; removing message handler")
c.removeHandler(messageHandler)
return
case msg := <-messageHandler.channel:
// Go language specification says that if one or more of the
// communications in the select statement can proceed, a single
// one that will proceed is chosen via a uniform pseudo-random
// selection.
// Thus, it can happen this communication is called when ctx is
// already done. Since we guarantee in the network channel API
// that handler is not called after ctx is done (client code
// could e.g. perform come cleanup), we need to double-check
// the context state here.
if messageHandler.ctx.Err() != nil {
continue
}
handleWithRetransmissions(msg)
}
}
}()

Instead of using a callback, we could explore using a channel. We would avoid unnecessary hop and the broadcast channel could be dropping messages from a full channel if needed.

pdyraga added a commit that referenced this issue Nov 29, 2022
Left a TODO about goroutine that may possibly be blocked forever.

This shouldn't be a problem right now with how Recv consumers are
constructed but this may become a problem in future.

#3420 captures this
work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant