feat: replace mutex with state loop and channels in protocol #505
Conversation
rakshasa
requested review from
agaffney,
verbotenj and
wolf31o2
as code owners
|
This looks interesting, but how does it help? The stated purpose of this PR is to get rid of the state mutex, but was it causing a problem? |
|
The state of the protocol is conceptually separate from of the recv and send loops, and by separating it out it makes the control flow much clearer. This makes the code easier to read and future changes to state flow easier to implement. In addition the send loop was locking the state mutex for the full duration of the send loop block, including where it writes to the |
|
I'm not ignoring this. I just need some time to really look this over and understand it. In the meantime, could you rebase it against current |
rakshasa
force-pushed
the
feat/state-loop
branch
from
14a798d
to
7667cd1
Compare
There was a problem hiding this comment.
This looks great. It took me a little while to wrap my head around what was happening in some places, but everything (now) makes sense.
I'm not a super fan of the named anonymous functions embedded in larger functions, partly for function size reasons, partly because it reminds me a bit too much of Python 😅, but it's good as it is.
|
@rakshasa thank you for your contribution! |
No description provided.