mirror of
https://github.com/dart-lang/sdk
synced 2024-09-16 01:30:32 +00:00
[vm/compiler] Make closures have no context if no variables are closed over
As a side-effect of making closures without captured variables have an empty context, we not only avoid a memory leak, but also allow such closures to be sent across [SendPort]s. Issue https://github.com/dart-lang/sdk/issues/36983 Issue https://github.com/dart-lang/sdk/issues/45603 TEST=vm/dart{_2,}/isolates/closures_without_captured_variables_test Change-Id: I5e8845203059ff625f7cff3f072d845b5e48ba36 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212297 Commit-Queue: Martin Kustermann <kustermann@google.com> Reviewed-by: Slava Egorov <vegorov@google.com>
This commit is contained in:
parent
9bec5d6b3b
commit
e9b56794fc
|
@ -0,0 +1,95 @@
|
||||||
|
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
|
||||||
|
// for details. All rights reserved. Use of this source code is governed by a
|
||||||
|
// BSD-style license that can be found in the LICENSE file.
|
||||||
|
|
||||||
|
// VMOptions=--enable-isolate-groups
|
||||||
|
|
||||||
|
// The tests in this file will only succeed when isolate groups are enabled
|
||||||
|
// (hence the VMOptions above).
|
||||||
|
|
||||||
|
import 'dart:async';
|
||||||
|
import 'dart:isolate';
|
||||||
|
|
||||||
|
import 'package:expect/expect.dart';
|
||||||
|
|
||||||
|
import 'fast_object_copy_test.dart' show ClassWithNativeFields;
|
||||||
|
|
||||||
|
main() async {
|
||||||
|
final rp = ReceivePort();
|
||||||
|
|
||||||
|
testNormalEnclosingFunction(rp);
|
||||||
|
testNormalNestedEnclosingFunction(rp);
|
||||||
|
testNormalNestedEnclosingFunction2(rp);
|
||||||
|
|
||||||
|
final si = StreamIterator(rp);
|
||||||
|
for (int i = 0; i < 3; ++i) {
|
||||||
|
Expect.isTrue(await si.moveNext());
|
||||||
|
Expect.equals(42, (si.current)());
|
||||||
|
}
|
||||||
|
si.cancel(); // closes the port
|
||||||
|
}
|
||||||
|
|
||||||
|
testNormalEnclosingFunction(ReceivePort rp) {
|
||||||
|
final invalidObject = ClassWithNativeFields();
|
||||||
|
final normalObject = Object();
|
||||||
|
|
||||||
|
captureInvalidObject() => invalidObject;
|
||||||
|
captureNormalObject() => normalObject;
|
||||||
|
captureNothing() => 42;
|
||||||
|
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureInvalidObject));
|
||||||
|
|
||||||
|
// TODO(http://dartbug.com/36983): Avoid capturing more than needed.
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureNormalObject));
|
||||||
|
|
||||||
|
// Should not throw, since the [captureNothing] closure should not have a
|
||||||
|
// parent context and therefore not transitively refer [rp].
|
||||||
|
rp.sendPort.send(captureNothing);
|
||||||
|
}
|
||||||
|
|
||||||
|
testNormalNestedEnclosingFunction(ReceivePort rp) {
|
||||||
|
final invalidObject = ClassWithNativeFields();
|
||||||
|
final normalObject = Object();
|
||||||
|
nested() {
|
||||||
|
captureInvalidObject() => invalidObject;
|
||||||
|
captureNormalObject() => normalObject;
|
||||||
|
captureNothing() => 42;
|
||||||
|
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureInvalidObject));
|
||||||
|
|
||||||
|
// TODO(http://dartbug.com/36983): Avoid capturing more than needed.
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureNormalObject));
|
||||||
|
|
||||||
|
// Should not throw, since the [captureNothing] closure should not have a
|
||||||
|
// parent context and therefore not transitively refer [rp].
|
||||||
|
rp.sendPort.send(captureNothing);
|
||||||
|
}
|
||||||
|
|
||||||
|
nested();
|
||||||
|
}
|
||||||
|
|
||||||
|
testNormalNestedEnclosingFunction2(ReceivePort rp) {
|
||||||
|
final invalidObject = ClassWithNativeFields();
|
||||||
|
final normalObject = Object();
|
||||||
|
|
||||||
|
captureInvalidObject() {
|
||||||
|
local() => invalidObject;
|
||||||
|
return local;
|
||||||
|
}
|
||||||
|
|
||||||
|
captureNormalObject() {
|
||||||
|
local() => normalObject;
|
||||||
|
return local;
|
||||||
|
}
|
||||||
|
|
||||||
|
captureNothing() => 42;
|
||||||
|
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureInvalidObject));
|
||||||
|
|
||||||
|
// TODO(http://dartbug.com/36983): Avoid capturing more than needed.
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureNormalObject));
|
||||||
|
|
||||||
|
// Should not throw, since the [captureNothing] closure should not have a
|
||||||
|
// parent context and therefore not transitively refer [rp].
|
||||||
|
rp.sendPort.send(captureNothing);
|
||||||
|
}
|
|
@ -0,0 +1,97 @@
|
||||||
|
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
|
||||||
|
// for details. All rights reserved. Use of this source code is governed by a
|
||||||
|
// BSD-style license that can be found in the LICENSE file.
|
||||||
|
|
||||||
|
// VMOptions=--enable-isolate-groups
|
||||||
|
|
||||||
|
// The tests in this file will only succeed when isolate groups are enabled
|
||||||
|
// (hence the VMOptions above).
|
||||||
|
|
||||||
|
// @dart=2.9
|
||||||
|
|
||||||
|
import 'dart:async';
|
||||||
|
import 'dart:isolate';
|
||||||
|
|
||||||
|
import 'package:expect/expect.dart';
|
||||||
|
|
||||||
|
import 'fast_object_copy_test.dart' show ClassWithNativeFields;
|
||||||
|
|
||||||
|
main() async {
|
||||||
|
final rp = ReceivePort();
|
||||||
|
|
||||||
|
testNormalEnclosingFunction(rp);
|
||||||
|
testNormalNestedEnclosingFunction(rp);
|
||||||
|
testNormalNestedEnclosingFunction2(rp);
|
||||||
|
|
||||||
|
final si = StreamIterator(rp);
|
||||||
|
for (int i = 0; i < 3; ++i) {
|
||||||
|
Expect.isTrue(await si.moveNext());
|
||||||
|
Expect.equals(42, (si.current)());
|
||||||
|
}
|
||||||
|
si.cancel(); // closes the port
|
||||||
|
}
|
||||||
|
|
||||||
|
testNormalEnclosingFunction(ReceivePort rp) {
|
||||||
|
final invalidObject = ClassWithNativeFields();
|
||||||
|
final normalObject = Object();
|
||||||
|
|
||||||
|
captureInvalidObject() => invalidObject;
|
||||||
|
captureNormalObject() => normalObject;
|
||||||
|
captureNothing() => 42;
|
||||||
|
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureInvalidObject));
|
||||||
|
|
||||||
|
// TODO(http://dartbug.com/36983): Avoid capturing more than needed.
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureNormalObject));
|
||||||
|
|
||||||
|
// Should not throw, since the [captureNothing] closure should not have a
|
||||||
|
// parent context and therefore not transitively refer [rp].
|
||||||
|
rp.sendPort.send(captureNothing);
|
||||||
|
}
|
||||||
|
|
||||||
|
testNormalNestedEnclosingFunction(ReceivePort rp) {
|
||||||
|
final invalidObject = ClassWithNativeFields();
|
||||||
|
final normalObject = Object();
|
||||||
|
nested() {
|
||||||
|
captureInvalidObject() => invalidObject;
|
||||||
|
captureNormalObject() => normalObject;
|
||||||
|
captureNothing() => 42;
|
||||||
|
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureInvalidObject));
|
||||||
|
|
||||||
|
// TODO(http://dartbug.com/36983): Avoid capturing more than needed.
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureNormalObject));
|
||||||
|
|
||||||
|
// Should not throw, since the [captureNothing] closure should not have a
|
||||||
|
// parent context and therefore not transitively refer [rp].
|
||||||
|
rp.sendPort.send(captureNothing);
|
||||||
|
}
|
||||||
|
|
||||||
|
nested();
|
||||||
|
}
|
||||||
|
|
||||||
|
testNormalNestedEnclosingFunction2(ReceivePort rp) {
|
||||||
|
final invalidObject = ClassWithNativeFields();
|
||||||
|
final normalObject = Object();
|
||||||
|
|
||||||
|
captureInvalidObject() {
|
||||||
|
local() => invalidObject;
|
||||||
|
return local;
|
||||||
|
}
|
||||||
|
|
||||||
|
captureNormalObject() {
|
||||||
|
local() => normalObject;
|
||||||
|
return local;
|
||||||
|
}
|
||||||
|
|
||||||
|
captureNothing() => 42;
|
||||||
|
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureInvalidObject));
|
||||||
|
|
||||||
|
// TODO(http://dartbug.com/36983): Avoid capturing more than needed.
|
||||||
|
Expect.throwsArgumentError(() => rp.sendPort.send(captureNormalObject));
|
||||||
|
|
||||||
|
// Should not throw, since the [captureNothing] closure should not have a
|
||||||
|
// parent context and therefore not transitively refer [rp].
|
||||||
|
rp.sendPort.send(captureNothing);
|
||||||
|
}
|
|
@ -5252,7 +5252,7 @@ Fragment StreamingFlowGraphBuilder::BuildFunctionNode(
|
||||||
bool has_valid_annotation,
|
bool has_valid_annotation,
|
||||||
bool has_pragma,
|
bool has_pragma,
|
||||||
intptr_t func_decl_offset) {
|
intptr_t func_decl_offset) {
|
||||||
intptr_t offset = ReaderOffset();
|
const intptr_t offset = ReaderOffset();
|
||||||
|
|
||||||
FunctionNodeHelper function_node_helper(this);
|
FunctionNodeHelper function_node_helper(this);
|
||||||
function_node_helper.ReadUntilExcluding(FunctionNodeHelper::kTypeParameters);
|
function_node_helper.ReadUntilExcluding(FunctionNodeHelper::kTypeParameters);
|
||||||
|
@ -5385,7 +5385,11 @@ Fragment StreamingFlowGraphBuilder::BuildFunctionNode(
|
||||||
|
|
||||||
Fragment instructions;
|
Fragment instructions;
|
||||||
instructions += Constant(function);
|
instructions += Constant(function);
|
||||||
instructions += LoadLocal(parsed_function()->current_context_var());
|
if (scopes()->IsClosureWithEmptyContext(offset)) {
|
||||||
|
instructions += NullConstant();
|
||||||
|
} else {
|
||||||
|
instructions += LoadLocal(parsed_function()->current_context_var());
|
||||||
|
}
|
||||||
instructions += flow_graph_builder_->AllocateClosure();
|
instructions += flow_graph_builder_->AllocateClosure();
|
||||||
LocalVariable* closure = MakeTemporary();
|
LocalVariable* closure = MakeTemporary();
|
||||||
|
|
||||||
|
|
|
@ -1492,7 +1492,7 @@ void ScopeBuilder::VisitTypeParameterType() {
|
||||||
|
|
||||||
void ScopeBuilder::HandleLocalFunction(intptr_t parent_kernel_offset) {
|
void ScopeBuilder::HandleLocalFunction(intptr_t parent_kernel_offset) {
|
||||||
// "Peek" ahead into the function node
|
// "Peek" ahead into the function node
|
||||||
intptr_t offset = helper_.ReaderOffset();
|
const intptr_t offset = helper_.ReaderOffset();
|
||||||
|
|
||||||
FunctionNodeHelper function_node_helper(&helper_);
|
FunctionNodeHelper function_node_helper(&helper_);
|
||||||
function_node_helper.ReadUntilExcluding(FunctionNodeHelper::kTypeParameters);
|
function_node_helper.ReadUntilExcluding(FunctionNodeHelper::kTypeParameters);
|
||||||
|
@ -1534,6 +1534,14 @@ void ScopeBuilder::HandleLocalFunction(intptr_t parent_kernel_offset) {
|
||||||
|
|
||||||
VisitFunctionNode(); // read function node.
|
VisitFunctionNode(); // read function node.
|
||||||
|
|
||||||
|
// Remember if this closure and all closures nested within it don't
|
||||||
|
// capture any variables from outer scopes.
|
||||||
|
if (scope_->function_level() == 1) {
|
||||||
|
if (scope_->NumCapturedVariables() == 0) {
|
||||||
|
result_->closure_offsets_without_captures.Add(offset);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
ExitScope(function_node_helper.position_, function_node_helper.end_position_);
|
ExitScope(function_node_helper.position_, function_node_helper.end_position_);
|
||||||
depth_ = saved_depth_state;
|
depth_ = saved_depth_state;
|
||||||
current_function_scope_ = saved_function_scope;
|
current_function_scope_ = saved_function_scope;
|
||||||
|
|
|
@ -189,6 +189,15 @@ class ScopeBuildingResult : public ZoneAllocated {
|
||||||
yield_context_variable(NULL),
|
yield_context_variable(NULL),
|
||||||
raw_variable_counter_(0) {}
|
raw_variable_counter_(0) {}
|
||||||
|
|
||||||
|
bool IsClosureWithEmptyContext(intptr_t function_node_offset) {
|
||||||
|
for (intptr_t i = 0; i < closure_offsets_without_captures.length(); ++i) {
|
||||||
|
if (closure_offsets_without_captures[i] == function_node_offset) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
IntMap<LocalVariable*> locals;
|
IntMap<LocalVariable*> locals;
|
||||||
IntMap<LocalScope*> scopes;
|
IntMap<LocalScope*> scopes;
|
||||||
GrowableArray<FunctionScope> function_scopes;
|
GrowableArray<FunctionScope> function_scopes;
|
||||||
|
@ -230,6 +239,10 @@ class ScopeBuildingResult : public ZoneAllocated {
|
||||||
// For-in iterators, one per for-in nesting level.
|
// For-in iterators, one per for-in nesting level.
|
||||||
GrowableArray<LocalVariable*> iterator_variables;
|
GrowableArray<LocalVariable*> iterator_variables;
|
||||||
|
|
||||||
|
// Remembers closure function kernel offsets that do not capture any
|
||||||
|
// variables.
|
||||||
|
GrowableArray<intptr_t> closure_offsets_without_captures;
|
||||||
|
|
||||||
private:
|
private:
|
||||||
DISALLOW_COPY_AND_ASSIGN(ScopeBuildingResult);
|
DISALLOW_COPY_AND_ASSIGN(ScopeBuildingResult);
|
||||||
};
|
};
|
||||||
|
|
|
@ -2,7 +2,6 @@
|
||||||
// for details. All rights reserved. Use of this source code is governed by a
|
// for details. All rights reserved. Use of this source code is governed by a
|
||||||
// BSD-style license that can be found in the LICENSE file.
|
// BSD-style license that can be found in the LICENSE file.
|
||||||
|
|
||||||
// VMOptions=--enable-isolate-groups
|
|
||||||
// VMOptions=--no-enable-isolate-groups
|
// VMOptions=--no-enable-isolate-groups
|
||||||
|
|
||||||
library illegal_msg_function_test;
|
library illegal_msg_function_test;
|
||||||
|
|
|
@ -4,7 +4,6 @@
|
||||||
|
|
||||||
// @dart = 2.9
|
// @dart = 2.9
|
||||||
|
|
||||||
// VMOptions=--enable-isolate-groups
|
|
||||||
// VMOptions=--no-enable-isolate-groups
|
// VMOptions=--no-enable-isolate-groups
|
||||||
|
|
||||||
library illegal_msg_function_test;
|
library illegal_msg_function_test;
|
||||||
|
|
Loading…
Reference in a new issue