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.

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

[vm/debugger/dart-code] Debugger local variables below asynchronous gap #51763

Open
dcharkes opened this issue Mar 17, 2023 · 13 comments
Open

[vm/debugger/dart-code] Debugger local variables below asynchronous gap #51763

dcharkes opened this issue Mar 17, 2023 · 13 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. vm-debugger

Comments

@dcharkes
Copy link
Contributor

When having <asynchronous gap> on the stack, all frames under it do not have their local variables accessible.
It would be convenient for debugging if these were available.

image

I'm not entirely sure where in the chain the information is missing, is this a limitation of our implementation in the VM? Or are we missing the necessary plumbing in the debugger or Dart-Code?

Local variables above the first asynchronous gap work fine:
image

cc @DanTup

@lrhn lrhn added the area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. label Mar 17, 2023
@mraleph
Copy link
Member

mraleph commented Mar 21, 2023

I would think that we are missing VM implementation for inspecting suspend states. /cc @alexmarkov

@DanTup
Copy link
Collaborator

DanTup commented Mar 21, 2023

In VS Code we're showing the asyncCausalFrames here which I guess don't have any variables. If they were added to those frames in the VM service response I'd expect them just to show up here.

@mraleph slightly related - in the VM Service docs I see there are both asyncCausalFrames and awaiterFrames but the description of the latter only says "Comparable to awaiterFrames, if provided, although some frames may be different". Is there anywhere I can find some examples of these differences? I ask because in Dart-Code/Dart-Code#4308 it was noted that the frames shown in VS Code are "worse" (at least for some cases) than pkg:stack_trace can produce but I couldn't track down an answer of whether I could get similar stack traces from the VM.

@mraleph
Copy link
Member

mraleph commented Mar 21, 2023

I think with the current implementation both should effectively be the same. Maybe we should remove one? /cc @rmacnak-google @bkonyi

I ask because in Dart-Code/Dart-Code#4308 it was noted that the frames shown in VS Code are "worse" (at least for some cases) than pkg:stack_trace can produce but I couldn't track down an answer of whether I could get similar stack traces from the VM.

It is very expensive to produce the type of stack trace that package:stack_trace can produce because it requires eagerly capturing stack traces at various points of the execution. In general we are aiming to be as close to that as possible without adding considerable overhead. Stack trace should follow through any explicit await and through some of the built-in methods (like Future.then), but it will not reach initial synchronous part of the stack. In other words: if you compare stack traces produced by the VM and package:stack_trace and there is more than just the last part (after the last async gap) missing - it is worth filing an issue with a reproduction.

@bkonyi
Copy link
Contributor

bkonyi commented Mar 21, 2023

I've never been 100% confident in my understanding of the difference between asyncCausalFrames and awaiterFrames, but if one is a replacement for another then I'm supportive of marking one as deprecated.

@DanTup
Copy link
Collaborator

DanTup commented Mar 23, 2023

@mraleph great, thanks for the explanation! :-)

@alexmarkov
Copy link
Contributor

It looks like this issue is a duplicate of an older issue #48836. With the new async implementation it should be easier to support this as suspend state of an async function contains a copy of the whole frame.

@DanTup I see 3 different implementations of stack trace scanning in the VM debugger support: Debugger::StackTrace() (frames in the vm-service protocol), Debugger::AsyncCausalStackTrace() (asyncCausalFrames in the vm-service protocol) and Debugger::AwaiterStackTrace() (awaiterFrames in the vm-service protocol). They are all different in how async frames are handled. The difference is subtle and it could be hard to figure out exact difference from the implementation. Maybe we can figure it out from running various different examples. For the VM it would be useful to know which stack traces from the vm-service protocol are currently used by the tools and why. If any of those stack tracing mechanisms are unused, we could deprecate them and cleanup the code, which would allow us to evolve the rest more easily.

@DanTup
Copy link
Collaborator

DanTup commented Mar 24, 2023

For the VM it would be useful to know which stack traces from the vm-service protocol are currently used by the tools and why. If any of those stack tracing mechanisms are unused, we could deprecate them and cleanup the code, which would allow us to evolve the rest more easily.

I'm not sure about IntelliJ/AS/etc., but in VS Code and the DAP we always show asyncCausalFrames ?? frames in the "call stack" pane. The assumption is that asyncCausalFrames was more useful to the user (when available) - but I didn't know about awaiterFrames so I don't know if it's better.

I can try and build some code examples and capture each type of stack (inc. the package:stack_trace one) to compare. If you have an idea of what sort of code might cause asyncCausalFrames and awaiterFrames to be different, that might be useful (it's possible I'll easily figure it out, but it also sounds possible they're often the same!).

@dcharkes
Copy link
Contributor Author

It looks like this issue is a duplicate of an older issue #48836. With the new async implementation it should be easier to support this as suspend state of an async function contains a copy of the whole frame.

Correct, the example in that issue reproduces the same issue in VSCode, but is a single file repro. 👍

class Foo {}

void main() async {
  final mainVar = new Foo();
  await foo();
  print(mainVar);
}

Future<void> foo() async {
  final fooVar = new Foo();
  await null;
  await foo2();
  print(fooVar);
}

Future<void> foo2() async {
  print("hello"); // breakpoint here: fooVar still available, mainVar not
  await Future.value(); // make a new <asynchronous gap>
  print("hello2"); // breakpoint here: fooVar also unavailable
}

(I'll close the other issue, as this one has more info.)

@DanTup
Copy link
Collaborator

DanTup commented Apr 3, 2023

FWIW (slightly off-topic for this issue, but continuining some discussion from above), I tried testing with the example given in the pkg:stack_trace readme:

import 'dart:async';

void main() {
  _scheduleAsync();
}

void _scheduleAsync() {
  Future.delayed(Duration(seconds: 1)).then((_) => _runAsync());
}

void _runAsync() {
  throw 'oh no!'; // break here
}

Which results in the "bad" (asyncCausal) stack trace (you can't see "main" anywhere in it as you'd probably like):

Screenshot 2023-04-03 at 13 19 16

I was going to compare it to awaiterFrames, but there actually was no field named awaiterFrames in the response. So I can't tell you how different they are, but I can tell you that they don't always appear together.

@mraleph
Copy link
Member

mraleph commented Apr 11, 2023

(you can't see "main" anywhere in it as you'd probably like):

This is currently working as intended. See #46318

@mraleph
Copy link
Member

mraleph commented May 2, 2023

I have been recently looking at our stack trace collection machinery to fix some AOT related bugs and as part of this, I stumbled across this code again, which made me do some code archaeology. Here is what I have found.

awaiterFrames were supposed to replace causalAsyncStack. Original implementation behind awaiterFrames linked Future and Stream objects to async functions which were waiting on them through an explicit _awaiter field. causalAsyncStack at that point in time worked through eager collection of stack traces on entry to async functions. Eventually we implemented an alternative lazy implementation (#37668) which traversed connections between futures by peaking into listeners attached to them and recognising async related listeners in a special way. We then realised that existing _awaiter mechanism can be implemented using the same type of traversal and have rewritten awaiterFrames frames implementation (#42457) to use "lazy async stacks". Later we made lazy implementation the default one and deleted eager causal stacks. At this point awaiterFrames and asyncCausalFrames became almost the same because they became backed by the same shared logic.

The are few differences between these two:

  • asyncCasualFrames contains regular frames (kRegular) and async causal frames (kAsyncCausal). Async causal frames are separated from each other and from regular frames with async suspension markers (kAsyncSuspensionMarker aka asynchronous gaps). awaiterFrames does not use async suspension markers, instead each frame is either regular (kRegular) or an async activation (kAsyncActivation) frame - if it corresponds to an async/async* function.
  • If we are running async function before the first suspension (i.e. synchronous portion of the async function) then awaiterFrames and asyncCausalFrames will treat this situation differently: awaiterFrames will contain this frame as an async activation frame, while asyncCausalFrames will contain this frame as a regular frame.

I don't think there are any other fundamental differences between these two fields. I have looked around in the internal and external codesearch (GitHub) and I could not find any uses of awaiterFrames - both VSCode and Intellij plugins seem to use asyncCausalFrames.

Internally in the VM awaiterFrames were used in one place - Debugger::PauseException, but given that asyncCausalFrames and awaiterFrames should basically agree, I should be able to rewrite this code to use asyncCausalFrames instead.

Based on these observations I have started to remove awaiterFrames. It is marked as optional in the protocol, so callers should be ready to deal with null value anyway.

copybara-service bot pushed a commit that referenced this issue May 9, 2023
This field was supposed[1] to replace Stack.asyncCausalFrames but IDEs
and other service protocol users never adopted it.

The mechanism for collecting awaiterFrames and asyncCausalFrames
was originally considerably different, but we have since unified
both[2] after we switched to lazy async stack traces[3].

Today the only differences between the two are:

- asyncCausalFrames represens async gaps explicitly, while
  awaiterFrames does not;
- when asynchronous function is running its synchronous part
  before the first suspension awaiterFrames will still represent
  the corresponding frame as an asynchronous activation, while
  asyncCausalFrames will treat the same frame as regular frame.

Consequently having this field does not add any value.

This CL removes it from the response and updates all tests to
test against asyncCausalFrames.

[1]: https://codereview.chromium.org/2692803006/
[2]: https://dart-review.googlesource.com/c/sdk/+/167561
[3]: #37668

Tested: pkg/vm_service, service, service_2
Bug: #51763
Change-Id: If2b035a8840047423b8ed8ce3b5d81155e9f38d0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301062
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
@mraleph
Copy link
Member

mraleph commented May 10, 2023

awaiterFrames are now removed.

@escamoteur
Copy link

what does this mean now? any chance to get local variables over asynchronous gaps?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. vm-debugger
Projects
None yet
Development

No branches or pull requests

7 participants