TEST=pkg/vm_service/test/get_queued_microtasks_rpc_test
CoreLibraryReviewExempt: This CL does not include any core library API
changes, only VM Service implementation changes within
sdk/lib/vmservice/.
Change-Id: Ie5488f498e4d0f3d201e3f31423fd5029b74a726
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425160
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
This is follow up to commit cb0c2bf5ed.
SampleBlockProcessor only enters isolate group and not a specific
isolate so the code must not rely on thread->isolate(). This fixes two
places where this was not the case:
* ProfileBuilder::IsPCInDartHeap
* UserTags::TagName
pkg/dds/test/get_cached_cpu_samples_test was supposed to cover this but
it has two problems:
First I observed that SampleBlockProcessor never gets a chance to
process a block if mutator thread always gets to it first (via a
scheduled interrupt), so this code is not well exercised. I started by
adding a variant of the test where interrupts are inhibited via a
vm:unsafe:no-interrupts pragma - which revealed the crashes in the
SampleBlockProcessor code.
This revealed the second problem: get_cached_cpu_samples_test does not
actually fail if testee crashes during the test, it just silently
completes with success. This seems to happen because disposal of
VmService connection is not forwarded into the future on which the test
is awaiting - and the whole process just exits once VmService connection
to the testee disappears (because all ports are closed, no pending
activity is possible after that one). I have fixed this by adding a
helper function which checks that connection to VmService only goes away
when we dispose it.
Note: there is another obvious issue here, which I am leaving unfixed
for now. SampleBlockProcessor calls UserTags::TagName in a way that can
race with isolate itself modifying the table. I think this race is
extremely unlikely but it can cause crashes on ARMs with its weak memory
model (e.g. we might end up reading garbage due to the reordering of
stores).
TEST=pkg/dds/test/get_cached_cpu_samples_test
Change-Id: Iee15ec2b019928b798c312e63edc76696abf5527
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426300
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This is a reland of commit b9b77058a9
Original change's description:
> Migrate to use pub workspace
>
> Use `pub get` to generate `.dart_tool/package_config.json` on gclient sync.
>
> All pkg/ (and a few third_party) packages that are developed inside the sdk repo are included in the workspace from the root `pubspec.yaml`.
>
> All dependencies that are pulled in via DEPS are added as path dependencies via `dependency_overrides` in the root `pubspec.yaml`.
>
> Bug: https://github.com/dart-lang/sdk/issues/56220
> Change-Id: I38c12b608c68da54c57821116cf9aa6696936746
> Tested: relies on CQ of existing tests. Should have no effect on functionality
> CoreLibraryReviewExempt: only core library change is adding a `// ignore:` comment. Should have no influence on functionality
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/397164
> Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
> Reviewed-by: Alexander Thomas <athom@google.com>
Bug: https://github.com/dart-lang/sdk/issues/56220
Change-Id: I29afabade2d2447dea05121cb87ff50bb21a4b76
Cq-Include-Trybots: luci.dart.try:flutter-linux-try,flutter-web-try
Tested: relies on CQ of existing tests. Should have no effect on functionality
CoreLibraryReviewExempt: only core library change is adding a `//
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415561
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
The change at https://dart-review.googlesource.com/c/sdk/+/410760 to fix a race incorrectly assumed that once we had handled startup for a thread, we would never need to send an automatic resume again. However this was not the case - after a hot reload, we need to resume the thread even though we had technically already handled startup.
This is essentially a partial revert of 84e6ed0784, with a test to verify we trigger readyToResume on PausePostRequest.
Change-Id: I1e171043f04f38dd6f52e1692d9257d34e5248be
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/416580
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
This reverts commit b9b77058a9.
Revert "Add missing sample pubspec to workspace"
This reverts commit 892ea15ac7.
These seem to break the engine.
Change-Id: Ieee26deb7928c3869a1b6265326c3ce568ffe731
Tested: this is a revert.
CoreLibraryReviewExempt: this is a revert
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415582
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Use `pub get` to generate `.dart_tool/package_config.json` on gclient sync.
All pkg/ (and a few third_party) packages that are developed inside the sdk repo are included in the workspace from the root `pubspec.yaml`.
All dependencies that are pulled in via DEPS are added as path dependencies via `dependency_overrides` in the root `pubspec.yaml`.
Bug: https://github.com/dart-lang/sdk/issues/56220
Change-Id: I38c12b608c68da54c57821116cf9aa6696936746
Tested: relies on CQ of existing tests. Should have no effect on functionality
CoreLibraryReviewExempt: only core library change is adding a `// ignore:` comment. Should have no influence on functionality
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/397164
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
The DAP tests have some debug logging that prints if a request/event takes longer than 10 seconds to arrive. In the case where tests run quickly and the DAP client is torn down, these delays cause the test run to stay alive for 10 sec unnecessarily (and print irrelevant warnings).
This changes that code to check periodically instead, and exits early if the client is torn down avoiding both the 10s wait and the spurious warning.
Change-Id: Ic07826942e8ae307867820020b30595a42e68d15
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/411680
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
We start isolates paused so that we can send breakpoints before any code runs. This means we need to resume after initialization is complete.
It's important we don't try to resume multiple times during initialization (regardless of the order of isolate events or whether the isolate was found when we queried for isolates during connection).. This is done with the flag `startupHandled`.
One code path was not taking this flag into account, which meant multiple resumes were still possible. This seemed to occur on Linux during test runs (I've noticed the order of events being different on Linux in the past).
This change extracts the checking of `startupHandled` before sending `readyToResume` and uses it in all places that handle this kind of startup resume.
Fixes https://github.com/dart-lang/sdk/issues/60128
Change-Id: Ie2679fc806ab3edf007259298da82dbc8b802a6f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410760
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Handler functions may return a Future<Response> that completes with an error. If that happens, then the top level error handling in the Shelf server will try to log the error to stderr.
But this will fail because DDS closes its stderr at startup. So Shelf will not write any response to the client, and the client will time out.
This PR adds a middleware to the DDS pipeline that receives errors thrown by the pipeline's handlers and returns a failure response.
Change-Id: I31beaf6abb0cbb42137c6b03f6081809de5d7119
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405681
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
While reviewing logs for flaky DAP tests, I found that these tests have been failing sometimes with errors that look like the isolate might not have paused on the `debugger()` call yet:
```
evaluateInFrame: (113) Expression compilation error
_buildExpressionEvaluationScope: invalid 'frameIndex' parameter: 0
```
and
```
evaluateInFrame: (113) Expression compilation error
org-dartlang-debug:synthetic_debug_expression:1:1: Error: Undefined name 'myInstance'.
myInstance
^^^^^^^^^^
```
This waits for the isolate to be paused, not just runnable, which I think might fix this.
Change-Id: Ib1d6b1c44a7ef93452065aa43c4d05058914db28
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/395302
Reviewed-by: Derek Xu <derekx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This is the proper fix for https://github.com/Dart-Code/Dart-Code/issues/5302, which is to advertise to clients that we may use ansi color codes in output events.
It requires increasing the DAP constraint because this field was added to the DAP package in 1.4.0 and we need to ensure anyone (like Flutter) building on top of this base debug adapter has a matching version.
Change-Id: Ie595cda91389f1c7f031a657c96e9f7480b00933
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/397521
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Danny Tuppeny