languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-05 16:41:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

[vm/debugger] Simplify async breakpoints implementation

Browse source 
We no longer rewrite suspendable function into closures nested
inside the original function, so the whole synthetic async breakpoint
machinery is not needed.

Refactor `Breakpoint` class to make one-shot and per-closure
separate properties of breakpoint, which allows creating
one-shot-per-closure breakpoints.

TEST=existing service tests

Change-Id: I208afe13f36efa40f5746e44867bd24684cf5f03
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310601
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This commit is contained in:
Vyacheslav Egorov 2023-06-26 22:45:10 +00:00 committed by Commit Queue
parent d1ecf01f71
commit 4a6f9328a2
13 changed files with 109 additions and 200 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
pkg/vm_service/CHANGELOG.md
View file

@ -1,3 +1,7 @@
## 11.7.3
- Update to version `4.10` of the spec.
- Add deprecation notice to `Breakpoint.isSyntheticAsyncContinuation`.
## 11.7.2
- Make Event classes abstract to permit `dap` event stream extensions.

2
pkg/vm_service/java/version.properties
View file

@ -1 +1 @@
version=4.9
version=4.10

5
pkg/vm_service/lib/src/vm_service.dart
View file

@ -28,7 +28,7 @@ export 'snapshot_graph.dart'
HeapSnapshotObjectNoData,
HeapSnapshotObjectNullData;
const String vmServiceVersion = '4.9.0';
const String vmServiceVersion = '4.10.0';
/// @optional
const String optional = 'optional';
@ -3347,8 +3347,7 @@ class Breakpoint extends Obj {
/// Has this breakpoint been assigned to a specific program location?
bool? resolved;
/// Is this a breakpoint that was added synthetically as part of a step
/// OverAsyncSuspension resume command?
/// Note: this property is deprecated and is always absent from the response.
@optional
bool? isSyntheticAsyncContinuation;

2
pkg/vm_service/pubspec.yaml
View file

@ -1,5 +1,5 @@
name: vm_service
version: 11.7.2
version: 11.7.3
description: >-
A library to communicate with a service implementing the Dart VM
service protocol.

41
runtime/observatory/tests/service/async_step_out_test.dart
View file

@ -8,26 +8,37 @@ import 'dart:developer';
import 'service_test_common.dart';
import 'test_helper.dart';
const LINE_A = 21;
const LINE_B = 22;
const LINE_C = 23;
const LINE_D = 28;
const LINE_E = 29;
const LINE_F = 30;
// AUTOGENERATED START
//
// Update these constants by running:
//
// dart runtime/observatory/tests/service/update_line_numbers.dart <test.dart>
//
const int LINE_A = 27;
const int LINE_B = 28;
const int LINE_C = 29;
const int LINE_0 = 38;
const int LINE_D = 39;
const int LINE_E = 40;
const int LINE_F = 41;
// AUTOGENERATED END
const LINE_0 = 27;
helper() async {
Future<int> helper() async {
await null; // LINE_A.
print('helper'); // LINE_B.
print('foobar'); // LINE_C.
print('line b'); // LINE_B.
print('line c'); // LINE_C.
return 0;
}
testMain() async {
Future<void> testMain() async {
int process(int value) {
return value + 1;
}
debugger(); // LINE_0.
print('mmmmm'); // LINE_D.
await helper(); // LINE_E.
print('z'); // LINE_F.
print('line d'); // LINE_D.
await helper().then(process); // LINE_E.
print('line f'); // LINE_F.
}
var tests = <IsolateTest>[

2
runtime/observatory/tests/service/get_version_rpc_test.dart
View file

@ -12,7 +12,7 @@ var tests = <VMTest>[
final result = await vm.invokeRpcNoUpgrade('getVersion', {});
expect(result['type'], 'Version');
expect(result['major'], 4);
expect(result['minor'], 9);
expect(result['minor'], 10);
expect(result['_privateMajor'], 0);
expect(result['_privateMinor'], 0);
},

36
runtime/observatory_2/tests/service_2/async_step_out_test.dart
View file

@ -8,26 +8,36 @@ import 'dart:developer';
import 'service_test_common.dart';
import 'test_helper.dart';
const LINE_A = 21;
const LINE_B = 22;
const LINE_C = 23;
const LINE_D = 28;
const LINE_E = 29;
const LINE_F = 30;
const LINE_0 = 27;
// AUTOGENERATED START
//
// Update these constants by running:
//
// dart runtime/observatory/tests/service/update_line_numbers.dart <test.dart>
//
const int LINE_A = 27;
const int LINE_B = 28;
const int LINE_C = 29;
const int LINE_0 = 37;
const int LINE_D = 38;
const int LINE_E = 39;
const int LINE_F = 40;
// AUTOGENERATED END
helper() async {
await null; // LINE_A.
print('helper'); // LINE_B.
print('foobar'); // LINE_C.
print('line b'); // LINE_B.
print('line c'); // LINE_C.
}
testMain() async {
int process(int value) {
return value + 1;
}
debugger(); // LINE_0.
print('mmmmm'); // LINE_D.
await helper(); // LINE_E.
print('z'); // LINE_F.
print('line d'); // LINE_D.
await helper().then(process); // LINE_E.
print('line f'); // LINE_F.
}
var tests = <IsolateTest>[

2
runtime/observatory_2/tests/service_2/get_version_rpc_test.dart
View file

@ -12,7 +12,7 @@ var tests = <VMTest>[
final result = await vm.invokeRpcNoUpgrade('getVersion', {});
expect(result['type'], equals('Version'));
expect(result['major'], equals(4));
expect(result['minor'], equals(9));
expect(result['minor'], equals(10));
expect(result['_privateMajor'], equals(0));
expect(result['_privateMinor'], equals(0));
},

131
runtime/vm/debugger.cc
View file

@ -195,9 +195,6 @@ void Breakpoint::PrintJSON(JSONStream* stream) {
jsobj.AddFixedServiceId("breakpoints/%" Pd "", id());
jsobj.AddProperty("enabled", enabled_);
jsobj.AddProperty("breakpointNumber", id());
if (is_synthetic_async()) {
jsobj.AddProperty("isSyntheticAsyncContinuation", is_synthetic_async());
}
jsobj.AddProperty("resolved", bpt_location_->IsResolved());
if (bpt_location_->IsResolved()) {
jsobj.AddLocation(bpt_location_);
@ -354,51 +351,26 @@ void BreakpointLocation::AddBreakpoint(Breakpoint* bpt, Debugger* dbg) {
}
Breakpoint* BreakpointLocation::AddRepeated(Debugger* dbg) {
Breakpoint* bpt = breakpoints();
while (bpt != nullptr) {
if (bpt->IsRepeated()) break;
bpt = bpt->next();
}
if (bpt == nullptr) {
bpt = new Breakpoint(dbg->nextId(), this);
bpt->SetIsRepeated();
AddBreakpoint(bpt, dbg);
}
return bpt;
return AddBreakpoint(dbg, Closure::Handle(), /*single_shot=*/false);
}
Breakpoint* BreakpointLocation::AddSingleShot(Debugger* dbg) {
return AddBreakpoint(dbg, Closure::Handle(), /*single_shot=*/true);
}
Breakpoint* BreakpointLocation::AddBreakpoint(Debugger* dbg,
const Closure& closure,
bool single_shot) {
Breakpoint* bpt = breakpoints();
while (bpt != nullptr) {
if (bpt->IsSingleShot()) break;
if ((bpt->closure() == closure.ptr()) &&
(bpt->is_single_shot() == single_shot)) {
break;
}
bpt = bpt->next();
}
if (bpt == nullptr) {
bpt = new Breakpoint(dbg->nextId(), this);
bpt->SetIsSingleShot();
AddBreakpoint(bpt, dbg);
}
return bpt;
}
Breakpoint* BreakpointLocation::AddPerClosure(Debugger* dbg,
const Instance& closure,
bool for_over_await) {
Breakpoint* bpt = nullptr;
// Do not reuse existing breakpoints for stepping over await clauses.
// A second async step-over command will set a new breakpoint before
// the existing one gets deleted when first async step-over resumes.
if (!for_over_await) {
bpt = breakpoints();
while (bpt != nullptr) {
if (bpt->IsPerClosure() && (bpt->closure() == closure.ptr())) break;
bpt = bpt->next();
}
}
if (bpt == nullptr) {
bpt = new Breakpoint(dbg->nextId(), this);
bpt->SetIsPerClosure(closure);
bpt->set_is_synthetic_async(for_over_await);
bpt = new Breakpoint(dbg->nextId(), this, single_shot, closure);
AddBreakpoint(bpt, dbg);
}
return bpt;
@ -1468,7 +1440,6 @@ Debugger::Debugger(Isolate* isolate)
last_stepping_fp_(0),
last_stepping_pos_(TokenPosition::kNoSource),
skip_next_step_(false),
synthetic_async_breakpoint_(nullptr),
exc_pause_info_(kNoPauseOnExceptions) {}
Debugger::~Debugger() {
@ -1477,7 +1448,6 @@ Debugger::~Debugger() {
ASSERT(breakpoint_locations_ == nullptr);
ASSERT(stack_trace_ == nullptr);
ASSERT(async_causal_stack_trace_ == nullptr);
ASSERT(synthetic_async_breakpoint_ == nullptr);
}
void Debugger::Shutdown() {
@ -2737,7 +2707,7 @@ Breakpoint* Debugger::SetBreakpointAtEntry(const Function& target_function,
}
Breakpoint* Debugger::SetBreakpointAtActivation(const Instance& closure,
bool for_over_await) {
bool single_shot) {
if (!closure.IsClosure()) {
return nullptr;
}
@ -2746,7 +2716,7 @@ Breakpoint* Debugger::SetBreakpointAtActivation(const Instance& closure,
BreakpointLocation* bpt_location =
SetBreakpoint(script, func.token_pos(), func.end_token_pos(), -1,
-1 /* no line/col */, func);
return bpt_location->AddPerClosure(this, closure, for_over_await);
return bpt_location->AddBreakpoint(this, Closure::Cast(closure), single_shot);
}
Breakpoint* Debugger::BreakpointAtActivation(const Instance& closure) {
@ -2758,10 +2728,8 @@ Breakpoint* Debugger::BreakpointAtActivation(const Instance& closure) {
while (loc != nullptr) {
Breakpoint* bpt = loc->breakpoints();
while (bpt != nullptr) {
if (bpt->IsPerClosure()) {
if (closure.ptr() == bpt->closure()) {
return bpt;
}
if (closure.ptr() == bpt->closure()) {
return bpt;
}
bpt = bpt->next();
}
@ -3098,17 +3066,6 @@ void Debugger::ResetSteppingFramePointer() {
stepping_fp_ = 0;
}
bool Debugger::SteppedForSyntheticAsyncBreakpoint() const {
return synthetic_async_breakpoint_ != nullptr;
}
void Debugger::CleanupSyntheticAsyncBreakpoint() {
if (synthetic_async_breakpoint_ != nullptr) {
RemoveBreakpoint(synthetic_async_breakpoint_->id());
synthetic_async_breakpoint_ = nullptr;
}
}
void Debugger::SetSyncSteppingFramePointer(DebuggerStackTrace* stack_trace) {
if (stack_trace->Length() > 0) {
stepping_fp_ = stack_trace->FrameAt(0)->fp();
@ -3518,7 +3475,7 @@ void Debugger::SignalPausedEvent(ActivationFrame* top_frame, Breakpoint* bpt) {
ResetSteppingFramePointer();
NotifySingleStepping(false);
ASSERT(!IsPaused());
if ((bpt != nullptr) && bpt->IsSingleShot()) {
if ((bpt != nullptr) && bpt->is_single_shot()) {
RemoveBreakpoint(bpt->id());
bpt = nullptr;
}
@ -3630,9 +3587,6 @@ ErrorPtr Debugger::PauseStepping() {
CacheStackTraces(DebuggerStackTrace::Collect(),
DebuggerStackTrace::CollectAsyncCausal());
if (SteppedForSyntheticAsyncBreakpoint()) {
CleanupSyntheticAsyncBreakpoint();
}
SignalPausedEvent(frame, nullptr);
HandleSteppingRequest(stack_trace_);
ClearCachedStackTraces();
@ -3679,34 +3633,6 @@ ErrorPtr Debugger::PauseBreakpoint() {
return Error::null();
}
if (bpt_hit->is_synthetic_async()) {
CacheStackTraces(stack_trace, DebuggerStackTrace::CollectAsyncCausal());
// Hit a synthetic async breakpoint.
if (FLAG_verbose_debug) {
OS::PrintErr(
">>> hit synthetic %s"
" (func %s token %s address %#" Px " offset %#" Px ")\n",
cbpt_tostring,
String::Handle(top_frame->QualifiedFunctionName()).ToCString(),
bpt_location->token_pos().ToCString(), top_frame->pc(),
top_frame->pc() - top_frame->code().PayloadStart());
}
ASSERT(synthetic_async_breakpoint_ == nullptr);
synthetic_async_breakpoint_ = bpt_hit;
bpt_hit = nullptr;
// We are at the entry of an async function.
// We issue a step over to resume at the point after the await statement.
SetResumeAction(kStepOver);
// When we single step from a user breakpoint, our next stepping
// point will be at the exact same pc. Skip it.
HandleSteppingRequest(stack_trace_, true /* skip next step */);
ClearCachedStackTraces();
return Error::null();
}
if (FLAG_verbose_debug) {
OS::PrintErr(">>> hit %" Pd
" %s"
@ -3736,7 +3662,7 @@ Breakpoint* BreakpointLocation::FindHitBreakpoint(ActivationFrame* top_frame) {
// First check for a single-shot breakpoint.
Breakpoint* bpt = breakpoints();
while (bpt != nullptr) {
if (bpt->IsSingleShot()) {
if (bpt->is_single_shot() && bpt->closure() == Instance::null()) {
return bpt;
}
bpt = bpt->next();
@ -3745,11 +3671,9 @@ Breakpoint* BreakpointLocation::FindHitBreakpoint(ActivationFrame* top_frame) {
// Now check for a closure-specific breakpoint.
bpt = breakpoints();
while (bpt != nullptr) {
if (bpt->IsPerClosure()) {
Closure& closure = Closure::Handle(top_frame->GetClosure());
if (closure.ptr() == bpt->closure()) {
return bpt;
}
if (bpt->closure() != Instance::null() &&
bpt->closure() == top_frame->GetClosure()) {
return bpt;
}
bpt = bpt->next();
}
@ -3757,7 +3681,7 @@ Breakpoint* BreakpointLocation::FindHitBreakpoint(ActivationFrame* top_frame) {
// Finally, check for a general breakpoint.
bpt = breakpoints();
while (bpt != nullptr) {
if (bpt->IsRepeated()) {
if (!bpt->is_single_shot() && bpt->closure() == Instance::null()) {
return bpt;
}
bpt = bpt->next();
@ -4041,9 +3965,6 @@ bool Debugger::RemoveBreakpointFromTheList(intptr_t bp_id,
if (pause_event_ != nullptr && pause_event_->breakpoint() == curr_bpt) {
pause_event_->set_breakpoint(nullptr);
}
if (synthetic_async_breakpoint_ == curr_bpt) {
synthetic_async_breakpoint_ = nullptr;
}
delete curr_bpt;
curr_bpt = nullptr;
@ -4211,18 +4132,18 @@ void Debugger::MaybeAsyncStepInto(const Closure& async_op) {
}
}
void Debugger::AsyncStepInto(const Closure& async_op) {
void Debugger::AsyncStepInto(const Closure& awaiter) {
Zone* zone = Thread::Current()->zone();
CallerClosureFinder caller_closure_finder(zone);
if (caller_closure_finder.IsAsyncCallback(
Function::Handle(zone, async_op.function()))) {
Function::Handle(zone, awaiter.function()))) {
const auto& suspend_state = SuspendState::Handle(
zone, caller_closure_finder.GetSuspendStateFromAsyncCallback(async_op));
zone, caller_closure_finder.GetSuspendStateFromAsyncCallback(awaiter));
const auto& function_data =
Object::Handle(zone, suspend_state.function_data());
SetBreakpointAtResumption(function_data);
} else {
SetBreakpointAtActivation(async_op, true);
SetBreakpointAtActivation(awaiter, /*single_shot=*/true);
}
Continue();
}

72
runtime/vm/debugger.h
View file

@ -41,17 +41,20 @@ class ObjectPointerVisitor;
class BreakpointLocation;
class StackFrame;
// A user-defined breakpoint, which either fires once, for a particular closure,
// or always. The API's notion of a breakpoint corresponds to this object.
// A user-defined breakpoint, which can be set for a particular closure
// (if |closure| is not |null|) and can fire one (|is_single_shot| is |true|)
// or many times.
class Breakpoint {
public:
Breakpoint(intptr_t id, BreakpointLocation* bpt_location)
Breakpoint(intptr_t id,
BreakpointLocation* bpt_location,
bool is_single_shot,
const Closure& closure)
: id_(id),
kind_(Breakpoint::kNone),
next_(nullptr),
closure_(Instance::null()),
closure_(closure.ptr()),
bpt_location_(bpt_location),
is_synthetic_async_(false) {}
is_single_shot_(is_single_shot) {}
intptr_t id() const { return id_; }
Breakpoint* next() const { return next_; }
@ -60,26 +63,8 @@ class Breakpoint {
BreakpointLocation* bpt_location() const { return bpt_location_; }
void set_bpt_location(BreakpointLocation* new_bpt_location);
bool IsRepeated() const { return kind_ == kRepeated; }
bool IsSingleShot() const { return kind_ == kSingleShot; }
bool IsPerClosure() const { return kind_ == kPerClosure; }
InstancePtr closure() const { return closure_; }
void SetIsRepeated() {
ASSERT(kind_ == kNone);
kind_ = kRepeated;
}
void SetIsSingleShot() {
ASSERT(kind_ == kNone);
kind_ = kSingleShot;
}
void SetIsPerClosure(const Instance& closure) {
ASSERT(kind_ == kNone);
kind_ = kPerClosure;
closure_ = closure.ptr();
}
bool is_single_shot() const { return is_single_shot_; }
ClosurePtr closure() const { return closure_; }
void Enable() {
ASSERT(!enabled_);
@ -93,30 +78,16 @@ class Breakpoint {
bool is_enabled() const { return enabled_; }
// Mark that this breakpoint is a result of a step OverAwait request.
void set_is_synthetic_async(bool is_synthetic_async) {
is_synthetic_async_ = is_synthetic_async;
}
bool is_synthetic_async() const { return is_synthetic_async_; }
void PrintJSON(JSONStream* stream);
private:
void VisitObjectPointers(ObjectPointerVisitor* visitor);
enum ConditionKind {
kNone,
kRepeated,
kSingleShot,
kPerClosure,
};
intptr_t id_;
ConditionKind kind_;
Breakpoint* next_;
InstancePtr closure_;
ClosurePtr closure_;
BreakpointLocation* bpt_location_;
bool is_synthetic_async_;
bool is_single_shot_;
bool enabled_ = false;
friend class BreakpointLocation;
@ -170,9 +141,9 @@ class BreakpointLocation {
Breakpoint* AddRepeated(Debugger* dbg);
Breakpoint* AddSingleShot(Debugger* dbg);
Breakpoint* AddPerClosure(Debugger* dbg,
const Instance& closure,
bool for_over_await);
Breakpoint* AddBreakpoint(Debugger* dbg,
const Closure& closure,
bool single_shot);
bool AnyEnabled() const;
bool IsResolved() const { return code_token_pos_.IsReal(); }
@ -705,7 +676,7 @@ class Debugger {
Breakpoint* SetBreakpointAtEntry(const Function& target_function,
bool single_shot);
Breakpoint* SetBreakpointAtActivation(const Instance& closure,
bool for_over_await);
bool single_shot);
Breakpoint* BreakpointAtActivation(const Instance& closure);
// TODO(turnidge): script_url may no longer be specific enough.
@ -726,7 +697,7 @@ class Debugger {
Breakpoint* GetBreakpointById(intptr_t id);
void MaybeAsyncStepInto(const Closure& async_op);
void AsyncStepInto(const Closure& async_op);
void AsyncStepInto(const Closure& awaiter);
void Continue();
@ -895,8 +866,6 @@ class Debugger {
intptr_t post_deopt_frame_index);
void ResetSteppingFramePointer();
bool SteppedForSyntheticAsyncBreakpoint() const;
void CleanupSyntheticAsyncBreakpoint();
void SetSyncSteppingFramePointer(DebuggerStackTrace* stack_trace);
GroupDebugger* group_debugger() { return isolate_->group()->debugger(); }
@ -945,11 +914,6 @@ class Debugger {
// breakpoint.
bool skip_next_step_;
// We keep this breakpoint alive until after the debugger does the step over
// async continuation machinery so that we can report that we've stopped
// at the breakpoint.
Breakpoint* synthetic_async_breakpoint_;
Dart_ExceptionPauseInfo exc_pause_info_;
// Holds function data corresponding to suspendable

4
runtime/vm/service.cc
View file

@ -3974,8 +3974,8 @@ static void AddBreakpointAtActivation(Thread* thread, JSONStream* js) {
return;
}
const Instance& closure = Instance::Cast(obj);
Breakpoint* bpt =
thread->isolate()->debugger()->SetBreakpointAtActivation(closure, false);
Breakpoint* bpt = thread->isolate()->debugger()->SetBreakpointAtActivation(
closure, /*single_shot=*/false);
if (bpt == nullptr) {
js->PrintError(kCannotAddBreakpoint,
"%s: Cannot add breakpoint at activation", js->method());

2
runtime/vm/service.h
View file

@ -17,7 +17,7 @@
namespace dart {
#define SERVICE_PROTOCOL_MAJOR_VERSION 4
#define SERVICE_PROTOCOL_MINOR_VERSION 9
#define SERVICE_PROTOCOL_MINOR_VERSION 10
class Array;
class EmbedderServiceHandler;

6
runtime/vm/service/service.md
View file

@ -1,4 +1,4 @@
# Dart VM Service Protocol 4.9
# Dart VM Service Protocol 4.10
> Please post feedback to the [observatory-discuss group][discuss-list]
@ -1927,8 +1927,7 @@ class Breakpoint extends Object {
// Has this breakpoint been assigned to a specific program location?
bool resolved;
// Is this a breakpoint that was added synthetically as part of a step
// OverAsyncSuspension resume command?
// Note: this property is deprecated and is always absent from the response.
bool isSyntheticAsyncContinuation [optional];
// SourceLocation when breakpoint is resolved, UnresolvedSourceLocation
@ -4700,5 +4699,6 @@ version | comments
4.7 | Added a deprecation notice to `Stack.awaiterFrames` field. Added a deprecation notice to `FrameKind.AsyncActivation`.
4.8 | Added `getIsolatePauseEvent` RPC.
4.9 | Added `isolateGroup` property to `Event`.
4.10 | Deprecated `isSyntheticAsyncContinuation` on `Breakpoint`.
[discuss-list]: https://groups.google.com/a/dartlang.org/forum/#!forum/observatory-discuss