- start two peers alice, bob
- make alice requests contact with bob
- they send messages
- they shutdown
- verify (and fix) no threads leaked
- verify messages
- add inbound connection handler to chatChannelHandler
- add Open and AcceptAllContactHandler to application
* add util to get onion address from private key
* PR comments: added test, using go-ricochet errors, function argument a privateKey
* remove unnecesary cast and variable
After the RequestOpenChannel changes, it's now possible to specify the
name and message of an outbound request as variables of the channel handler,
instead implementing an interface method to return them.
Also added the SendResponse method, which is necessary to respond to an
inbound request that was in the Pending state.
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.
After the last round of fixes in Do, there was still one major issue
making it impossible to use Do or Break safely: closing connections.
Connections can be closed spontaneously, and this causes Process to
return. At that moment any ongoing call to Do or Break has deadlocked;
nothing will ever read those channels again. To prevent this, we have to
ensure that Do or Break won't try to send to Process' channels once it
has stopped reading from them. Doing that without other races is tricky.
The solution here, briefly, is to acquire a mutex in Do and Break before
checking the `closed` boolean, and hold that mutex for the entire
operation (including any blocking channel operations). When Process is
closing down the connection, it uses a separate goroutine to acquire the
same mutex and change the boolean, while still handling channel reads.
Once the boolean has changed, the mutex guarantees that nothing will try
to send to these channels again.
I've tried to document the problems and solutions in the code, because
it is subtle in some places and this is definitely critical code.
There were several issues with the Do function that made it nearly
impossible to write safe code.
First, Do cannot be called recursively -- it will deadlock. There is
actually no way to implement a safe and recursive Do (or mutex) in Go,
because there is no primitive that will identify the current goroutine.
RequestOpenChannel used Do internally, which made it impossible to open
channels safely in many circumstances. That has been removed, so all
calls to RequestOpenChannel must be changed to happen under Do now.
Do now has more documentation and a new rule: no code exposed through
API can use Do, unless it has sole custody of the connection (such as
ProcessAuthAsClient).
Related to that problem, Do was impossible to call from inside handlers
(or anything else on the process goroutine) -- it would again just
deadlock. This is resolved by wrapping calls into user code to continue
handling invocations of Do (and only those) while the handler is
executing.
There is a third issue with connection close, but it will be addressed
in a separate commit
And finally, because it's impossible to timeout or interrupt a call to
Do, I also added a DoContext method that takes a go Context, which is
also passed through to the called function.
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.
Dan Ballard
Sarah Jamie Lewis
Dan Ballard
Sarah Jamie Lewis
Sarah Jamie Lewis
John Brooks