deviceconnect: connection-scoped cleanup for call instances#1
Open
kavya-chennoju wants to merge 1 commit intoericvh:feature/deviceconnectfsfrom
Open
Conversation
Owner
|
Embarrassed to say you are ahead of my in actually testing this out. Normal 9p semantics are that the session should be garbage collected when the session gets closed (usually when the fd itself gets garbage collected) -- so it shouldn't need an explicit close command in the ctl file, but I'm not sure the garbage collection is built correctly in the current file server. |
Reading clone allocates a numbered call directory under functions/<fn>/ that nothing reaps. Tie the call dir's lifetime to the 9P connection that allocated it: a per-connection registry of allocated dirs is drained from a ConnClosed hook on a small Fsrv wrapper (dcSrv). When a client unmounts, crashes, or drops its TCP connection, every call dir it allocated goes away with it — the same idiom Plan 9 uses for /net/tcp and /srv. No client-side cleanup ritual needed; the CLI just mounts and unmounts. Includes a regression test that allocates dirs, drops the allocating connection, and asserts a fresh client can no longer walk into them.
kavya-chennoju
force-pushed
the
feature/deviceconnectfs-close-ctl
branch
from
ca02ccb to
20d565f
Compare
kavya-chennoju
changed the title
Author
|
Ah 🤦♀️ , that makes clean up easier didn't realize there were already lifecycle hooks, wired them up. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Reading
cloneallocates a numbered call directory underfunctions/<fn>/that nothing reaps — every invocation leaks one numbered subdir for the
lifetime of the server.
Tie each call dir's lifetime to the 9P connection that allocated it:
a per-connection registry on
DCFSis drained from aConnClosedhookon a small
Fsrvwrapper (dcSrv). When a client unmounts, crashes, ordrops its TCP connection, every call dir it allocated goes away with it
— the same idiom Plan 9 uses for
/net/tcpand/srv.No client-side cleanup ritual required; the CLI just mounts and unmounts.
Test plan
go test ./p/srv/examples/deviceconnect/ ./cmd/go9p-deviceconnect/ -race -count=20(160 runs, no flakes)TestDeviceConnect_DisconnectCleansUpCallsallocates dirs, drops the allocating connection, asserts a fresh client cannot walk into themfuncCloneFile.Readand (b) theConnClosedbody — both produce the expected failure