Skip to content

Breakpoints aren't hit inside "part" files #35859

New issue

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

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

Already on GitHub? # to your account

Closed
DanTup opened this issue Feb 5, 2019 · 13 comments
Closed

Breakpoints aren't hit inside "part" files #35859

DanTup opened this issue Feb 5, 2019 · 13 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. vm-debugger

Comments

@DanTup
Copy link
Collaborator

DanTup commented Feb 5, 2019

Moving from Dart-Code/Dart-Code#1443

It seems if you include a file using part/part-of then breakpoints set inside it aren't hit (at least, those set by VS Code while the app is paused at startup).

Here's a repro:

pubspec.yaml

name:          breakpoint_test
version:       0.0.1

bin/main.dart

import 'package:breakpoint_test/all_scripts.dart';

void main() {
  func();
}

lib/all_scripts.dart

library foo;

part 'src/func.dart';

lib/src/func.dart

part of foo;

void func() {
  print("Breakpoints don't work here!");
}

If I move func directly into all_scripts then it works fine.

breakpoint_test.zip


Some info on what be the issue from @peter-ahe-google:

My guess it that there could be a flaw in how the VM maps fileUris to importUris (this is a concept from Kernel, let me know if I need to elaborate).

We only store a fileUri on func. So you have to translate that back to the importUri, and it is not sufficient to look at the enclosing library's importUri.

I think it would be more reliable to translate importUris to fileUris (that is, convert package-URIs to file-URIs), because the debug information is based on those.

@DanTup DanTup changed the title Breakpoints aren't hit inside "part-of" files Breakpoints aren't hit inside "part" files Feb 5, 2019
@vsmenon vsmenon added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. vm-debugger labels Feb 7, 2019
@zichangg
Copy link
Contributor

@DanTup Hi Danny, I failed to reproduce the bug with your files on Linux. I used observatory to set breakpoints at func.dart and it then hit the breakpoint if I continued running. The code is latest from master branch.
Breakpoint 1 resolved at func.dart:4:3
Paused at breakpoint 1 at func.dart:4:3
Do I need to reproduce with VS Code?

@arsen
Copy link

arsen commented Feb 13, 2019

It is still happening for me with VS Code (v1.31.0)
Flutter extension version 2.22.3

@DanTup
Copy link
Collaborator Author

DanTup commented Feb 13, 2019

@zichangg Yah, the issue only seems to occur for the breakpoints that are sent at startup while the isolate is paused (PauseStart?). If I add breakpoints later via Observatory, they work fine.

It may also happen with IntelliJ, I'm not sure.

Assuming you have VS Code set up, you can capture a log while reproducing by running the Dart: Capture Logs command (from the VS Code command palette) and then tick just the Debugger (Observatory) category. Then once you've finished, click Stop Logging on the notification (note it may have collapsed into tje bell icon in the status bar). That will open a log of all the JSON to/from the VM service (I though I'd attached a log here, but apparently I did not).

Let me know if you have any trouble reproducing!

@zichangg
Copy link
Contributor

@DanTup Have a great weekend!
A question for Uris format. VM sent out the Uris with absolute path. From addBreakpoint request, the Uri received is package:breakpoint_test/src/func.dart. This causes scripts matching failed. Why transform Uris to packages:.. from your side?

@DanTup
Copy link
Collaborator Author

DanTup commented Feb 16, 2019

@zichangg The breakpoints seem to be expected as package: URIs in all other cases. I think it used to be only the case for external packages, but I think since #33076 it's also required for the package being run too.

To be honest, this stuff isn't well documented and it's been a lot of trial and error (and bugs). It would be best if the formats for breakpoints could be explicitly documented to avoid confusion like this (this has come up before, because often there have been mismatches across platforms - for ex see #32500).

We used to send every breakpoint twice (once as package: and once as a path) but this really complicated supporting "resolving" breakpoints in VS Code (something I still haven't finished) because we end up with two VM breakpoints for one Code breakpoint so I removed it. I think it should be predictable what we should send to avoid this.

@zichangg
Copy link
Contributor

Blocked on #36023.

dart-bot pushed a commit that referenced this issue Mar 20, 2019
This will enable the VM to map URIs to package-URIs to solve problems
such as #35859

Change-Id: I15520325a5b81a99a7e3f56c2e35fd775d9da946
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/96905
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
@DanTup
Copy link
Collaborator Author

DanTup commented Mar 20, 2019

@jensjoha thanks for this! Though I just built the SDK locally and still don't seem to hit the breakpoint. Here's the project I'm testing with (breakpoint is on the line BREAK HERE, line 4 in part.dart):

part_breakpoint_repro.zip

I'm using this version of the SDK:

Dart VM version: 2.2.1-edge.58882ffdd49afdfb53c7be15361340bb64760fe4 (Wed Mar 20 09:39:33 2019 +0000) on "macos_x64"

And here's a log of the VM service chatter:

observatory.txt

Any ideas what's wrong?

@jensjoha
Copy link
Contributor

Hmm... I think that's a VM bug (or VSCode somehow setting breakpoints too soon according to who knows what) --- if you add a breakpoint to the line in main, then, when it stops there, add the breakpoint to package:hello_world/part.dart:4 and continues, it does actually stop in the part file...

@DanTup
Copy link
Collaborator Author

DanTup commented Mar 20, 2019

From the logs it looks like the isolate was in the PauseStart state which I think is when we're expected to send breakpoints. Who would be best to advise if this is a VM or VS Code issue? (AFAIC though, there's no other opportunity for us to send breakpoints - we can't let the code start running before doing it, or there could be races).

@jensjoha
Copy link
Contributor

Reverting a051696 seems to fix the issue so maybe @alexmarkov would be the person to ping here.

The issue seems to be that the top level class stuff is delayed, i.e. when you set the breakpoint that part is not known by the VM yet. A latent breakpoint is set, and I suppose that would be fine if Debugger::NotifyDoneLoading() was called at the right time, but it isn't.

A "funny" thing is that if you add, e.g. "class Foo {}" and the end of "part.dart", the breakpoint is set just fine because that class is created (and thus the VM gets to know about the part up front).

@jensjoha jensjoha reopened this Mar 20, 2019
dart-bot pushed a commit that referenced this issue Mar 20, 2019
The breakpoints sent from IDE will be checked with existing scripts.
Because the loadedscripts() doesn't contain the scripts info for part/part of.
Then the breakpoint will be considerred as a latent breakpoint and didn't get
resolved later. The solution is to finalize the toplevel class before loadedscripts()
uses Dictionary iterator.

Bug: #35859
Change-Id: I90b67ee9e9e6afe2556ca806cdd87eb5661304a8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97402
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Zichang Guo <zichangguo@google.com>
@DanTup
Copy link
Collaborator Author

DanTup commented Mar 21, 2019

With @zichangg's most recent change, this seems to be working as expect. Thanks both! 👍

dart-bot pushed a commit that referenced this issue Mar 29, 2019
… library"

This reverts commit 9f00d1b.

Reason for revert: This change triggered some incorrect runtime typecheck failures in Fuchsia.

Original change's description:
> [VM] Ensure Top level class is finalized for loadedscripts in library
> 
> The breakpoints sent from IDE will be checked with existing scripts.
> Because the loadedscripts() doesn't contain the scripts info for part/part of.
> Then the breakpoint will be considerred as a latent breakpoint and didn't get
> resolved later. The solution is to finalize the toplevel class before loadedscripts()
> uses Dictionary iterator.
> 
> Bug: #35859
> Change-Id: I90b67ee9e9e6afe2556ca806cdd87eb5661304a8
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97402
> Reviewed-by: Alexander Markov <alexmarkov@google.com>
> Commit-Queue: Zichang Guo <zichangguo@google.com>

TBR=alexmarkov@google.com,asiva@google.com,zichangguo@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: #35859
Change-Id: Ib28c2257f94b8d7ee0a3607be4a92153b7229e54
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/98360
Reviewed-by: Zach Anderson <zra@google.com>
@alexmarkov
Copy link
Contributor

The fix caused incorrect type finalization and was reverted. We need to find another way of fixing this.

@alexmarkov alexmarkov reopened this Mar 29, 2019
@DanTup
Copy link
Collaborator Author

DanTup commented Apr 1, 2019

If this isn't likely to happen soon, do you know if there's a workaround? I tried switching from package URIs to file URIs (based on the original comments here), though that didn't seem to help (thanks to @bkonyi fixes, it's likely VS Code will move over to always using file URIs instead of package URIs for breakpoints soon).

dart-bot pushed a commit that referenced this issue Apr 1, 2019
Following the revert #98360, make sure the finalizing happens only in debugger.
Create the test cases to demonstrate the behavior.

Bug: #35859
Change-Id: Ib27fef18a7c0696ec6dc6d045fa06f60677333c8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/98422
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Zichang Guo <zichangguo@google.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. vm-debugger
Projects
None yet
Development

No branches or pull requests

6 participants