There were a few related issues with ProcessAuthAsClient/Server that
could cause deadlocks or leak goroutines:
Break() could end up being called more than once, which is always a
deadlock. This is fixed by using a sync.Once.
RequestOpenChannel was called without Do, and was called before
Process in the same goroutine, which would have deadlocked if Do was
used.
Timing out the authentication attempt wouldn't directly abort
Process(); it would only exit if the connection was closed somewhere
else.
It may be a good idea to change some of this to guarantee that
ProcessAuthAsClient returns either an authenticated connection or closes
the connection, but I'll leave that as a separate task for the moment.
This fixes a quirk where it would've been difficult to tell which of
several channels of the same type+direction is the one you just created.
It's also a fairly common pattern to want to interact with a channel
right after opening it; for example, a chat channel is opened and can
immediately send messages before getting the peer response. It's
convenient to not have to do a separate lookup.
RequestOpenChannel is the primary API to open a new outbound channel. It
was written to take a connection.Handler and use OnOpenChannelRequest
to get a channels.Handler to represent the new channel, which is the
same path that inbound channels will take.
Going through the global OnOpenChannelRequest method makes this much
less flexible and prevents passing parameters to the new channel handler
during creation. This also requires users of the API to know/find the
connection handler, or worse, to boilerplate one into existence for their
channel creation.
Instead, I think this function should take a channels.Handler directly,
so that the caller gets full control over the handler for their new
channel.
As part of that change, I've also moved the authentication logic in
AutoConnectionHandler to be contained entirely within
{In,Out}boundConnectionHandler.
Sarah Jamie Lewis
Sarah Jamie Lewis
John Brooks
Sarah Jamie Lewis