Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Debug Adapter should drop references to variables, scopes, stack frames when execution resumes #4420

Closed
DanTup opened this issue Mar 3, 2023 · 2 comments
Closed

Debug Adapter should drop references to variables, scopes, stack frames when execution resumes #4420

DanTup opened this issue Mar 3, 2023 · 2 comments
Labels
in debugger Relates to the debug adapter or process of launching a debug session is performance relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Milestone
v3.68.0

Comments

@DanTup
Copy link
Member

DanTup commented Mar 3, 2023

https://microsoft.github.io/debug-adapter-protocol/overview#lifetime-of-objects-references

To simplify the management of object references in debug adapters, their lifetime is limited to the current suspended state. Once execution resumes, object references become invalid and DAP clients must not use them.
...
Please note that other object references like threadId do not have limited lifetime because that would prevent something like the pause request from working in the running state.

We should probably store a flag with StoredData stating whether data can be dropped when resuming.

(We may need to consider when we clear variablesReferences for global evaluations, since those may still be visible in the Debug Console and users might still want to interact with them).
@DanTup DanTup added in debugger Relates to the debug adapter or process of launching a debug session is performance labels Mar 3, 2023
@DanTup DanTup added this to the v3.62.0 milestone Mar 3, 2023
@DanTup DanTup modified the milestones: v3.62.0, v3.64.0, v3.66.0 Mar 30, 2023
@DanTup DanTup modified the milestones: v3.66.0, v3.68.0 May 16, 2023
@DanTup
Copy link
Member Author

DanTup commented Jun 20, 2023

https://dart-review.googlesource.com/c/sdk/+/310342

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Jun 20, 2023
@DanTup
[dds/dap] Clear stored thread data on resume
01c02d6 
When an isolate is paused, we store some data for things like variables, stack frames, etc. to round-trip an identifier to the client that it can ask about later.

Previously we never cleared these, so over time in a long debug session they could build up. The DAP spec says these references become invalid when execution is resumed:

> To simplify the management of object references in debug adapters, their lifetime is limited to the current suspended state. Once execution resumes, object references become invalid and DAP clients must not use them.

Because it's possible to resume one thread without another, I'm clearing up only the specific thread (isolate)s data when it resumes, rather than all of it.

Fixes Dart-Code/Dart-Code#4420

Change-Id: I2ba7d9cd3f23b5a628d9e279e49edf2bee5afd29
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310342
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
@DanTup DanTup added the relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available label Jun 28, 2023
@DanTup
Copy link
Member Author

DanTup commented Jun 28, 2023

Fixed by dart-lang/sdk@01c02d6, ships in SDKs.

@DanTup DanTup closed this as completed Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
in debugger Relates to the debug adapter or process of launching a debug session is performance relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Projects
None yet
Milestone
v3.68.0
Development

No branches or pull requests
1 participant
@DanTup