VM, package:vm_service_client and/or package:coverage fail to find source line/column #34841
Assignees
Labels
Milestone
Comments
yjbanov
added
the
P1
yjbanov
added a commit
to yjbanov/flutter
that referenced
this issue
Oct 18, 2018
|
Potentially relevant issue from when something similar came up last time: dart-lang/tools#437 I'd banged up dart-archive/coverage#216 at the time, but I didn't land it; I suspect because perhaps it didn't solve the issue. |
This was referenced Oct 18, 2018
|
Some more info: the vm_service_client is looking for token 3230 but seems to be receiving the following |
|
That table is for this file: https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/widgets/binding.dart Notice that the first line coincides with a |
yjbanov
added a commit
to flutter/flutter
that referenced
this issue
Oct 18, 2018
|
/cc @jensjoha |
|
Similar errors were reported from the Flutter buildbot when an attempt was made to roll Dart version Please see link for details. Here is an extract from that buildbot output file I am trying to reproduce this failure with a local engine build in order to bisect the Dart changes but unfortunately I can't get it to fail with a local engine build. Please note this error was happening even before Yegor's change. |
a-siva
referenced
this issue
in flutter/flutter
Oct 18, 2018
a-siva
added
the
area-front-end
|
@aartbik for dart roll |
|
I dumped the tokenPosition array in vm_service_client and it shows - Clearly position 3230 is missing. |
|
Should someone be assigned to work on this asap? This is a TODAY issue in Flutter flutter/flutter#23225. |
|
Disabling this new option 'unsafePackageSerialization' fixes the problem. I am going to upload a CL that flips this to false. |
|
flutter/pull/23280 has been created for this. |
|
Here's what's happening: When unsafe package serialization is enabled, the VM is given a concatenated dill file. In this case, 3 scripts are created in the VM for the same absolute file. One of them gets the url set as the import-url, the two others are kept as is, so we now have:
When asking for the SourceReport, the VM assumes that if two scripts urls are the same, the scripts are the same too (in the above that is not the case --- this cannot be assumed when supporting concatenated dill files) and reuses an existing (wrong) one, i.e. giving responsibility for both scripts to one of the scripts. The data given from the VM is something like: Note how there are only two outputs. Solution: Don't think Scripts are the same based on their url. Not in the VM, and not in any application of the service protocol. With my CL (https://dart-review.googlesource.com/80821) applied the answer from the VM is now: We now have 3 outputs. Patching in dart-archive/coverage#216 (thanks @cbracken) in [...]/.pub-cache/hosted/pub.dartlang.org/coverage-0.12.2/lib/src/collect.dart and running flutter with my patched VM everything seems to work ( So while disabling unsafe package serialization does work that's hardly a good solution, and it more masks the problem than fixes it. |
|
Oh, and also, probably one doesn't have to do a lot of hot-reloading before the assumption about one script per url goes out the window... |
a-siva
added a commit
to flutter/flutter
that referenced
this issue
Oct 19, 2018
… is turned on (#23280) * Disable unsafePackageSerialization as it causes issues when coverage is used (see dart-lang/sdk#34841) * Restore Coverage.
|
Is this still targeted for 2.1? |
|
I've now landed dart-archive/coverage#216 in the coverage package and released as v0.12.3, which was rolled into Flutter in flutter/flutter#23304. Once the VM rolls to the framework, I'll remove the |
a-siva
added
P2
|
Flutter is unblocked with unsafePackageSerialization being turned off, moving milestore to 2.2 and dropping priority to P2 |
dart-bot
pushed a commit
that referenced
this issue
Oct 29, 2018
When using a concatenated dill file, several scripts with the same URL can exist. They are not the same though, and should not be treated as such. Bug: #34841 Change-Id: Icd46357ffcf72ed35251b2a2793e1f83c02c4a8e Reviewed-on: https://dart-review.googlesource.com/c/80821 Commit-Queue: Jens Johansen <jensj@google.com> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
dart-bot
pushed a commit
that referenced
this issue
Nov 2, 2018
When using a concatenated dill file, several scripts with the same URL can exist. They are not the same though, and should not be treated as such. Bug: #34841 Change-Id: Icd46357ffcf72ed35251b2a2793e1f83c02c4a8e Reviewed-on: https://dart-review.googlesource.com/c/80821 Commit-Queue: Jens Johansen <jensj@google.com> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
|
I'm still seeing this like explained in flutter/flutter#20811 |
|
/cc @kmillikin fyi |
|
With the reproduction from the flutter issue link - further boiled down to this:
(which has the added benefit of showing something wrong when running) this is what's happening:
One solution is to mark the constructor as coming from |
|
I'm landing a test that demonstrates the problem (https://dart-review.googlesource.com/c/sdk/+/91660), but the frontend team doesn't find that this is a frontend issue - it's a VM issue, and it should be fixed in the VM. Unassigning and changing area-label. |
jensjoha
added
area-vm
|
Deleted incorrect comment. |
|
This issue still exists. The mixin class has not been fixed. |
|
I don't believe this is required for 2.3 . If not, can you please move from milestone? |
|
This is not blocking 2.3 - removing from milestone. |
dart-bot
pushed a commit
that referenced
this issue
May 20, 2019
…wrong script When a mixin class is created, there might be anonymous closures copied over to the other script. Previously the script of closure function is the script of its parent function, which is not true in that case. The solution is to create a patch class as the owner if parent function has a different script object. Return script of owner when query for script of closure function. Bug: #34841 Change-Id: I53065cbf13f2d0dc8da320993fd3cd425e5c9714 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/102226 Commit-Queue: Zichang Guo <zichangguo@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com> Reviewed-by: Siva Annamalai <asiva@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is happening on Flutter commit flutter/flutter@3fbd140.
When we run tests with coverage, one of the tests fails with the following:
Full log file.
/cc @JekCharlsonYu @a-siva @cbracken
The text was updated successfully, but these errors were encountered: