Cleanup resources #246
Merged
Cleanup resources #246
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@cheatfate can I get some 👀 on the cancelation logic that I added, I'm not 100% sure I've got it right. |
cheatfate
reviewed
Jun 28, 2020
cheatfate
reviewed
Jun 28, 2020
cheatfate
reviewed
Jun 28, 2020
cheatfate
reviewed
Jun 28, 2020
cheatfate
reviewed
Jun 28, 2020
cheatfate
reviewed
Jun 28, 2020
cheatfate
reviewed
Jun 28, 2020
| trace "deleted channel" | ||
| finally: | ||
| trace "stopping mplex main loop", oid = m.oid | ||
| defer: |
There was a problem hiding this comment.
defer inside of try block you are trying to break it really, finally here is much more natural from my point of view.
There was a problem hiding this comment.
With all things being equal, I think defer is cleaner than a bunch of nested try/finally, but it is a matter of preference. Hopefully, defer wont break this time tho 😄
There was a problem hiding this comment.
fwiw, defer has been proposed for removal from the language and has proven buggy before - it will not get as much attention on the compiler side.. finally might be the safer option overall
cheatfate
reviewed
Jun 28, 2020
|
I can take care of the defers in a separate PR, going to move this one forward as it resolves a lot of our current issues. |
dryajov
added a commit
that referenced
this pull request
Jul 2, 2020
* consolidate reading in lpstream * remove debug echo * tune log level * add channel cleanup and cancelation handling * cancelation handling * cancelation handling * cancelation handling * cancelation handling * cleanup and cancelation handling * cancelation handling * cancelation * tests * rename isConnected to connected * remove testing trace * comment out debug stacktraces * explicit raises
dryajov
added a commit
that referenced
this pull request
Jul 7, 2020
* consolidate reading in lpstream * remove debug echo * tune log level * add channel cleanup and cancelation handling * cancelation handling * cancelation handling * cancelation handling * cancelation handling * cleanup and cancelation handling * cancelation handling * cancelation * tests * rename isConnected to connected * remove testing trace * comment out debug stacktraces * explicit raises
dryajov
added a commit
that referenced
this pull request
Jul 8, 2020
* use var semantics to optimize table access * wip... lvalues don't work properly sadly... * big publish refactor, replenish and balance * fix internal tests * use g.peers for fanout (todo: don't include flood peers) * exclude non gossip from fanout * internal test fixes * fix flood tests * fix test's trypublish * test interop fixes * make sure to not remove peers from gossip table * restore old replenishFanout * cleanups * Cleanup resources (#246) * consolidate reading in lpstream * remove debug echo * tune log level * add channel cleanup and cancelation handling * cancelation handling * cancelation handling * cancelation handling * cancelation handling * cleanup and cancelation handling * cancelation handling * cancelation * tests * rename isConnected to connected * remove testing trace * comment out debug stacktraces * explicit raises * restore trace vs debug in gossip * improve fanout replenish behavior further * cleanup stale peers more eaguerly * synchronize connection cleanup and small refactor * close client first and call parent second * disconnect failed peers on publish * check for publish result * fix tests * fix tests * always call close Co-authored-by: Giovanni Petrantoni <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves resource cleanup and reduces the number of
LPChannel's in use as well as addsCancelationsupport.