Repository navigation
Conversation
mafredri
force-pushed
the
mafredri/context-ctx-cleanup
branch
from
November 3, 2026 15:16
91b1642 to
29aa068
Compare
Nothing checked that test cleanup cancels a mock context the caller never canceled with ErrMockClockTestEnded, or that it leaves contexts that were already done alone. The next change moves that cancellation.
Each mock context registered its own t.Cleanup, and that closure kept the context reachable until the test ended, even after it was canceled or expired. 100,000 create and cancel cycles retained about 1 KB per context. The cleanup that NewMock registers now cancels the contexts that are still live, found through their pending events. Those contexts are now canceled after, not before, cleanups registered between NewMock and the context's creation.
Removing an element with append(s[:i], s[i+1:]...) left a copy of the last element in the backing array, so the Mock kept the most recently removed events, including done mock contexts, reachable. A mock parent did the same with the AfterFunc of each child that was done. slices.Delete zeroes the vacated slot.
The Mock kept canceled and expired contexts reachable through a cleanup closure and through stale slice slots, and no test noticed. A weak pointer to a cause that only the context references is nil after one runtime.GC once nothing keeps the context. The file needs Go 1.24 for the weak package, and go.mod still allows Go 1.23.
Each case now creates its own Mock, so the case functions take only t.
With one cleanup from NewMock, a cleanup registered after NewMock but before a context was created ran while that context was still live. t.Cleanup(srv.Close) then hung when a server goroutine blocked on a mock context whose parent never cancels. Each context again registers its own cleanup, but the closure holds only a ref that cancel clears, so a done context stays collectable. testing.T.Cleanup still keeps about 500 B per call until the test ends.
A cleanup registered before a context is created must see that context canceled, so a cleanup that waits for a goroutine blocked on it does not hang. The wait is bounded so a regression fails instead of hanging the test binary.
A cleanup registered right after NewMock runs after the contexts' own cleanups, so it can check both the cancellation order and the error and cause in one test. This replaces the separate order test, its goroutine, its 10s bound, and the subtest that only existed to observe the contexts after cleanup.
The comment says they keep their error and cause, but only the cause was checked. A test-end cleanup that overwrote Err with Canceled while leaving the cause alone passed.
The old text said when the cause is used but not what it means for the test's own cleanups: which ones see the context canceled, and which see it live.
The comment explained why the closure holds only a ref but not why each context registers its own cleanup instead of sharing the one from NewMock.
The godoc said "when the test ended", but each context's cleanup is registered on the TestingT passed to NewMock. With a Mock created from a parent test's t, a probe showed the context still live in all of a subtest's cleanups and after t.Run returned, so a subtest's t.Cleanup(srv.Close) waiting on a handler blocked on it hung. It also dropped #30's sentence on contexts created after the test ended.
The README said only that live contexts are canceled at test end. A test writer also needs the order: which of their cleanups see a context canceled, what a subtest using its parent's Mock sees, and that timers and tickers stop delivering instead of being canceled. Each claim was checked by a probe on this branch; the timer and ticker behavior is main's.
A cleanup that is already running when a mock context is created sees the context live until it returns, because the context's cleanup is registered after it. A probe reproduced this: a handler that creates a mock context after t.Cleanup(srv.Close) started kept srv.Close blocked for a second with the context live.
The test-end order holds only for cleanups that have not started when the context is created, or when the ticker is created or the timer fires. Probes showed srv.Close blocked on a handler that created its context after Close started, and returning once that context was derived from t.Context() through srv.Config.BaseContext. A timer that fired while a cleanup ran was still received in that cleanup, and a ticker reader that also selected on t.Context() exited before an earlier cleanup gave up on it.
The section now follows the Deadline() subsection, so it no longer inserts text where #31 and #32 do. The timer and ticker paragraph says what to do instead of how their cleanups work, and the BaseContext advice says to set it on an unstarted server before Start, as the probe that showed srv.Close returning did. On main's timer and ticker code, a reader of C got no tick or value in a cleanup registered before the ticker or the fire, and a reader that also selected on t.Context() exited.
TestWithTimeout_TestEndCancelsLiveContexts becomes TestWithTimeout_TestEndCancelsBeforeEarlierCleanups, named for the order it checks, and sits next to TestWithTimeout_CreatedAfterMockCleanup, which covers the other test-end case.
mafredri
force-pushed
the
mafredri/context-ctx-cleanup
branch
from
November 4, 2026 19:17
29aa068 to
8bc3199
Compare
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
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.
Each mock context registered a
t.Cleanupwhose closure kept the context reachable until the test ended, even after it was canceled or expired. A test that creates many short-lived mock contexts, such as one per request, grew by about 1.1 KB per context until the test ended, measured as live heap after a GC following 100,000 create-and-cancel cycles. Stdlib contexts retain about 0.The cleanup closure now holds only a small reference struct, and
cancelclears it, so a done context can be collected. Removal from the event list now usesslices.Delete, as #30 does for a mock parent's list of child contexts: the oldappend(s[:i], s[i+1:]...)left a copy of the last element in the backing array, which kept the most recently removed context reachable. Retention is now about 500 B per context, almost all of it thet.Cleanupcall itself; an emptyt.Cleanup(func(){})retains about 470 B.One cleanup registered by
NewMockwould bring retention to about 0, but it changes cleanup order, and I rejected it for that. Each context must keep its own cleanup so it is canceled before cleanups registered before it was created. With a single cleanup, this pattern hangs:NewMock, thent.Cleanup(srv.Close), then a server goroutine blocks on a mock context whose parent never cancels.srv.Closeruns first and waits for that goroutine forever.TestWithTimeout_TestEndCancelsBeforeEarlierCleanupscovers this order.The
ErrMockClockTestEndedgodoc and a new "At test end" README section say which cleanups see a context canceled, and what to do when at.Cleanup(srv.Close)would still wait forever. The README also says what to do about timers and tickers at test end, as they behave onmain; #37 and #38 change that behavior.TestWithTimeout_DoneContextIsCollectablechecks collection withweakpointers, so it requires Go 1.24 and keeps its build tag whilego.modsaysgo 1.23.9. It fails if the cleanup closure, the stale slot in the event list, or the stale slot in a mock parent's list of child contexts keeps the context reachable again.Stacked on #30.
When squash merging, please replace the commit body with this description.