[dart][inspect] Anti-leak Node / Property delete
Node / Property delete previously could leave deleted things in the parent's
map, if the thing was created and deleted without deleting the parent.
Now, the thing tells its parent any time it's deleted.
Test: fx run-test inspect_mod_test
fx run-host-tests fuchsia_inspect_package_unittests
CF-597 #progress
Change-Id: Iaed64b4bb908aa5b95eb4c84958b4ac994660ee5
diff --git a/public/dart/fuchsia_inspect/lib/src/inspect/node.dart b/public/dart/fuchsia_inspect/lib/src/inspect/node.dart
index a3ec10c..88e0dbf 100644
--- a/public/dart/fuchsia_inspect/lib/src/inspect/node.dart
+++ b/public/dart/fuchsia_inspect/lib/src/inspect/node.dart
@@ -21,30 +21,35 @@
final _properties = <String, Property>{};
final _children = <String, Node>{};
+ final Node _parent;
+ final String _name;
/// Creates a [Node] with [name] under the [parentIndex].
///
/// Private as an implementation detail to code that understands VMO indices.
/// Client code that wishes to create [Node]s should use [child].
- Node._(String name, int parentIndex, this._writer)
- : index = _writer.createNode(parentIndex, name) {
+ Node._(this._parent, this._name, int parentIndex, this._writer)
+ : index = _writer.createNode(parentIndex, _name) {
if (index == invalidIndex) {
_writer = null;
}
}
/// Wraps the special root node.
- Node._root(this._writer) : index = _writer.rootNode;
+ Node._root(this._writer)
+ : index = _writer.rootNode,
+ _parent = null,
+ _name = null;
/// Creates a Node that never does anything.
///
/// These are returned when calling createChild on a deleted [Node].
Node._deleted()
: _writer = null,
+ _parent = null,
+ _name = null,
index = invalidIndex;
- bool get _isDeleted => _writer == null;
-
/// Returns a child [Node] with [name].
///
/// If a child with [name] already exists and was not deleted, this
@@ -53,13 +58,18 @@
if (_writer == null) {
return Node._deleted();
}
- // TODO(cphoenix): Tell parents when deleted, instead of asking children.
- // Asking children allows the map to grow without limit as children are
- // created and deleted (e.g. ring buffer).
- if (_children.containsKey(name) && !_children[name]._isDeleted) {
+ if (_children.containsKey(name)) {
return _children[name];
}
- return _children[name] = Node._(name, index, _writer);
+ return _children[name] = Node._(this, name, index, _writer);
+ }
+
+ void _forgetChild(String name) {
+ _children.remove(name);
+ }
+
+ void _forgetProperty(String name) {
+ _properties.remove(name);
}
/// Deletes this node and any children from underlying storage.
@@ -68,16 +78,23 @@
/// no effect and do not result in an error. Calls on a deleted node that
/// return a Node or property return an already-deleted object.
void delete() {
+ _delete();
+ }
+
+ void _delete({bool deletedByParent = false}) {
if (_writer == null) {
return;
}
_properties
- ..forEach((_, property) => property.delete())
+ ..forEach((_, property) => property._delete(deletedByParent: true))
..clear();
_children
- ..forEach((_, node) => node.delete())
+ ..forEach((_, node) => node._delete(deletedByParent: true))
..clear();
+ if (!deletedByParent) {
+ _parent._forgetChild(_name);
+ }
_writer.deleteEntity(index);
_writer = null;
}
@@ -95,14 +112,14 @@
if (_writer == null) {
return StringProperty._deleted();
}
- if (_properties.containsKey(name) && !_properties[name]._isDeleted) {
+ if (_properties.containsKey(name)) {
if (_properties[name] is! StringProperty) {
throw InspectStateError("Can't create StringProperty named $name;"
' a different type exists.');
}
return _properties[name];
}
- return _properties[name] = StringProperty._(name, index, _writer);
+ return _properties[name] = StringProperty._(name, this, _writer);
}
/// Returns a [ByteDataProperty] with [name] on this node.
@@ -119,14 +136,14 @@
if (_writer == null) {
return ByteDataProperty._deleted();
}
- if (_properties.containsKey(name) && !_properties[name]._isDeleted) {
+ if (_properties.containsKey(name)) {
if (_properties[name] is! ByteDataProperty) {
throw InspectStateError("Can't create ByteDataProperty named $name;"
' a different type exists.');
}
return _properties[name];
}
- return _properties[name] = ByteDataProperty._(name, index, _writer);
+ return _properties[name] = ByteDataProperty._(name, this, _writer);
}
/// Returns an [IntProperty] with [name] on this node.
@@ -142,14 +159,14 @@
if (_writer == null) {
return IntProperty._deleted();
}
- if (_properties.containsKey(name) && !_properties[name]._isDeleted) {
+ if (_properties.containsKey(name)) {
if (_properties[name] is! IntProperty) {
throw InspectStateError(
"Can't create IntProperty named $name; a different type exists.");
}
return _properties[name];
}
- return _properties[name] = IntProperty._(name, index, _writer);
+ return _properties[name] = IntProperty._(name, this, _writer);
}
/// Returns a [DoubleProperty] with [name] on this node.
@@ -165,14 +182,14 @@
if (_writer == null) {
return DoubleProperty._deleted();
}
- if (_properties.containsKey(name) && !_properties[name]._isDeleted) {
+ if (_properties.containsKey(name)) {
if (_properties[name] is! DoubleProperty) {
throw InspectStateError("Can't create DoubleProperty named $name;"
' a different type exists.');
}
return _properties[name];
}
- return _properties[name] = DoubleProperty._(name, index, _writer);
+ return _properties[name] = DoubleProperty._(name, this, _writer);
}
}
diff --git a/public/dart/fuchsia_inspect/lib/src/inspect/property.dart b/public/dart/fuchsia_inspect/lib/src/inspect/property.dart
index b68f0d6..d41c512 100644
--- a/public/dart/fuchsia_inspect/lib/src/inspect/property.dart
+++ b/public/dart/fuchsia_inspect/lib/src/inspect/property.dart
@@ -13,13 +13,16 @@
/// The writer for the underlying VMO.
///
- /// Will be set to null if the [Property] has been deleted or could not be
- /// created in the VMO.
+ /// Will be set to null if the [Property] has been deleted, could not be
+ /// created in the VMO, or was created on a deleted (or never-created) Node.
/// If so, all actions on this [Property] should be no-ops and not throw.
VmoWriter _writer;
+ final Node _parent;
+ final String _name;
+
/// Creates a modifiable [Property].
- Property._(this.index, this._writer) {
+ Property._(this._parent, this._name, this.index, this._writer) {
if (index == invalidIndex) {
_writer = null;
}
@@ -27,22 +30,28 @@
/// Creates a property that never does anything.
///
- /// These are returned when calling methods on a deleted Node,
- /// or if there is no space for a newly created property in underlying storage.
+ /// These are returned when calling create-property methods on a deleted Node.
Property.deleted()
: _writer = null,
+ _name = null,
+ _parent = null,
index = invalidIndex;
- bool get _isDeleted => _writer == null;
-
/// Sets the value of this property.
void setValue(T value);
/// Deletes this property from underlying storage.
/// Calls on a deleted property have no effect and do not result in an error.
void delete() {
+ _delete();
+ }
+
+ void _delete({bool deletedByParent = false}) {
_writer?.deleteEntity(index);
_writer = null;
+ if (!deletedByParent) {
+ _parent._forgetProperty(_name);
+ }
}
}
@@ -76,8 +85,9 @@
///
/// Only [Node.intProperty()] can create this object.
class IntProperty extends Property<int> with Arithmetic<int> {
- IntProperty._(String name, int parentIndex, VmoWriter writer)
- : super._(writer.createMetric(parentIndex, name, 0), writer);
+ IntProperty._(String name, Node parent, VmoWriter writer)
+ : super._(
+ parent, name, writer.createMetric(parent.index, name, 0), writer);
IntProperty._deleted() : super.deleted();
}
@@ -86,8 +96,9 @@
///
/// Only [Node.doubleProperty()] can create this object.
class DoubleProperty extends Property<double> with Arithmetic<double> {
- DoubleProperty._(String name, int parentIndex, VmoWriter writer)
- : super._(writer.createMetric(parentIndex, name, 0.0), writer);
+ DoubleProperty._(String name, Node parent, VmoWriter writer)
+ : super._(
+ parent, name, writer.createMetric(parent.index, name, 0.0), writer);
DoubleProperty._deleted() : super.deleted();
}
@@ -96,8 +107,9 @@
///
/// Only [Node.stringProperty()] can create this object.
class StringProperty extends Property<String> with BytesProperty<String> {
- StringProperty._(String name, int parentIndex, VmoWriter writer)
- : super._(writer.createProperty(parentIndex, name), writer);
+ StringProperty._(String name, Node parent, VmoWriter writer)
+ : super._(
+ parent, name, writer.createProperty(parent.index, name), writer);
StringProperty._deleted() : super.deleted();
}
@@ -106,8 +118,9 @@
///
/// Only [Node.byteDataProperty()] can create this object.
class ByteDataProperty extends Property<ByteData> with BytesProperty<ByteData> {
- ByteDataProperty._(String name, int parentIndex, VmoWriter writer)
- : super._(writer.createProperty(parentIndex, name), writer);
+ ByteDataProperty._(String name, Node parent, VmoWriter writer)
+ : super._(
+ parent, name, writer.createProperty(parent.index, name), writer);
ByteDataProperty._deleted() : super.deleted();
}
