fix refcounts during comms addEgress #3485
Merged
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
warner
force-pushed
the
3483-comms-add-egress-refcount
branch
from
0ade5dd to
417c50a
Compare
FUDCo
approved these changes
Jul 17, 2021
warner
force-pushed
the
3483-comms-add-egress-refcount
branch
from
417c50a to
2a7a702
Compare
warner
force-pushed
the
3240-better-gc-and-finalize
branch
from
1fee5ef to
c80e6c7
Compare
warner
force-pushed
the
3483-comms-add-egress-refcount
branch
from
2a7a702 to
2981a8a
Compare
To make it easier to read, so I don't mess up the polarity again. This should not cause any behavioral changes.
* expose state.getRefCounts() for debugging and tests
`addEgress` was calling `setReachable` with the wrong value for `isImportFromComms`. When we add an egress, the kernel is exporting an object into comms, and comms is exporting that to the downstream machine. So the downstream machine is *importing* it from comms (which means the `isReachable` flag causes the `reachable` refcount to be increased, something that only happens on imports, not on the export). This caused the object being exported through `addEgress` to have a zero refcount. If/when the downstream machine ever dropped it, the refcount would go negative, causing an error. fixes #3483
warner
force-pushed
the
3483-comms-add-egress-refcount
branch
from
2981a8a to
230b494
Compare
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.
The comms
addEgressfunction was callingsetReachablewith the wrong value forisImportFromComms. When we add an egress, the kernel is exporting an object into comms, and comms is exporting that to the downstream machine. So the downstream machine is importing it from comms (which means theisReachableflag causes thereachablerefcount to be increased, something that only happens on imports, not on the export).This caused the object being exported through
addEgressto have a zero refcount. If/when the downstream machine ever dropped it, the refcount would go negative, causing an error.fixes #3483
I recommend reviewing this one commit at a time. The second commit is a non-behavior-changing renaming which would distract from the important changes.