Repository navigation
Stop releases memory steps its target holds; a capture ends with its stream - #31
Merged
Merged
Conversation
…stream A direct runtime reads the live heap, so a memory step can wait on its target indefinitely. Two places let that wedge an editor: - memory/stop answered while a step on the session was still in flight. An editor's interval autopilot whose capture was in flight at stop stayed busy until that request answered, and never captured the next session. - the .NET engine waited out its 90 s deadline after the event stream had ended, because the target took the session with it. A target that died mid-capture therefore held its step for 90 s, and then failed as a generic "Failed" instead of PROCESS_NOT_FOUND. Stop now ends every step in flight on its session the way cancellation does (dropping the future terminates its tools, the target keeps running), waits for each to answer, then releases the session ([PROF-MEMORY-STOP]). The engine stops waiting the moment the stream ends, stops only a session still streaming, and reports any failure of a command whose target is gone as ProcessNotFound ([PROF-DOTNET-HEAP-CAPTURE], [PROF-DOTNET-GC]). The memory steps move out of service.rs into service/memory_steps.rs, which brings service.rs under the 500-line limit.
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.
TLDR
memory/stopnow ends every step still in flight on its session before it answers, and the .NET engine stops waiting the moment a capture's event stream ends. A target that dies mid-capture is nowPROCESS_NOT_FOUND.Details
SharpLsp's Windows profiler CI hit this as a flake. The interval autopilot's last capture was sent about 30 ms before teardown stopped tracking, and the debuggee was then killed. The engine sat out its full 90 s
SessionRundeadline: the host log showslspProfiler/memory/snapshotfinishing after90273ms. Until it answered, the autopilot stayed busy, so the next test's interval session never captured ("2 automatic capture(s) not met within 40000ms (last: 0)").[PROF-MEMORY-STOP](new spec section). Newmemory/gate.rs:stopruns under its session'sStepGate.stopends the steps in flight the way cancellation does ([PROF-HOST-CANCEL]): it drops their futures, andkill_on_dropterminates their tools. The target keeps running.stopwaits until each step has answered (OTHER, "memory session<id>stopped before the step finished"), then lets the runtime release the session.SessionRun.Await. Waits for the watcher or the end of the stream, whichever comes first, and stops only a session that is still streaming. Agcwhose stream ends early is now a failure instead ofobserved: false.Errors.Describe. Any failure of a command whose--pidis gone isProcessNotFound("process<pid>is not running"). Before, a stream cut off mid-read surfaced asFailed: "Unable to read beyond the end of the stream.".service.rs(631 lines) intoservice/memory_steps.rs, which bringsservice.rsto 493 lines.How Do The Automated Tests Prove It Works?
Each new test was run red first.
crates/lsp-profiler-dotnet/tests/dotnet_held_e2e.rs(Unix):kill -STOP, confirmed byps), so a snapshot or diff of it is held for as long as it stays frozen. No timer is involved.stop_releases_a_snapshot_its_frozen_target_holdsandstop_releases_a_diff_its_frozen_target_holdsassert that:Ackand deletes the graphs;stopping_one_session_leaves_another_sessions_held_step_heldholds two sessions:dotnet/LspProfiler.DotNet.Engine.Tests/SessionRunTests.cshas 6 permutations:Each asserts the outcome, the number of stops, and the stream's state. Red: a stream that had already ended still waited out the deadline (
DeadlinePassed), stopped a session that no longer existed, and threw on a faulted stream.dotnet/LspProfiler.DotNet.Engine.Tests/FailureTests.cscrosses 8 causes with 3 target states (gone, live, none). With the target gone the kind is alwaysProcessNotFoundand the message is exact. Otherwise each cause keeps its own kind and message. Red: the 8 gone-target cases returned their generic kinds.Regressions. All green on Linux:
dotnet_e2edotnet_fsharp_e2e(withstrace)clippy --workspace --all-targets -D warnings,fmt --checkand the spec-citation check also pass.