renproject / aw

A flexible P2P networking library for upgradable distributed systems.
MIT License
38 stars 18 forks source link

Use incoming message channel API #90

Closed ross-pure closed 2 years ago

ross-pure commented 2 years ago

This PR fixes a problem that can occur with the current receiver API. The problem is: if the receiver that is registered with the Peer (i.e. the function that the Peer will call when receiving a message of type wire.MsgTypeSend) ever calls Peer.Send (or similar functions), execution will block. This is because the event loop is currently calling the receiver, and so cannot process the send event, but the call to Peer.Send made by the receiver will block until that send event has been handled.

The solution implemented in this PR fixes the issue in most cases by changing the API to instead use a channel to which incoming messages are written. Specifically, if the package user has a message handling function

func handle(from id.Signatory, msg []byte) { ... }

then where previously this handler would be registered by calling

peer.Receive(handle)

after which handle would be called by the event loop whenever a direct message was received, using the API proposed in this PR would instead require the package user to handle the messages as they are read off the channel:

LOOP:
for {
    select {
    case <-ctx.Done():
        break LOOP

    case msg := <-peer.IncomingMessages:
        handle(msg.From, msg.Data)
    }
}

This fixes the main issue since the event loop is not blocked handling the incoming direct message and can therefore handle a call to Peer.Send or similar during the execution of handle. However, it is still possible for execution to block. This can happen if during a call to handle the incoming message channel buffer becomes full and the event loop is trying to write to it, in which case it is blocked, and then handle makes a call to Peer.Send which in turn will also block. The probability that this situation occurs can of course be reduced by increasing the event loop and incoming message channel buffer sizes, but it will always be possible. This PR is put forward as a incomplete but mostly sufficient solution that doesn't require large changes. A later fix can solve the problem entirely (hopefully without having to spawn go routines all the time).