languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-06 14:49:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "[VM/Runtime] Fix 'File' object leak in async file open operation"

Browse source 
This reverts commit b67f45f955.

Reason for revert: This seems to cause a crash on ARMv7 aot compiled code.

Original change's description:
> [VM/Runtime] Fix 'File' object leak in async file open operation
>
> The 'File' object created in the async file open method is leaked
> if the operation is not completed when the isolate shuts down
> with an unhandled exception.
> This change adds a finalizable state for the 'File' object so that
> the message deletion that happens when ports are closed could run
> the callback to clean up the 'File' object.
>
> TEST=new tests added
>
> Change-Id: I4a3cb28370d27306c795c1914aeb5c18a1d85e2b
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/210761
> Reviewed-by: Alexander Aprelev <aam@google.com>
> Commit-Queue: Siva Annamalai <asiva@google.com>

TBR=aam@google.com,rmacnak@google.com,asiva@google.com

Change-Id: Ie91aadd318ef19a0bb4d7f769c9e876e76d719d8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/211021
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: William Hesse <whesse@google.com>
This commit is contained in:
William Hesse 2021-08-24 12:49:23 +00:00 committed by commit-bot@chromium.org
parent ae4543fcb9
commit 637e14006c
8 changed files with 6 additions and 393 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
runtime/bin/dartutils.cc
View file

@ -931,16 +931,6 @@ Dart_CObject* CObject::NewExternalUint8Array(intptr_t length,
return cobject;
}
Dart_CObject* CObject::NewNativePointer(intptr_t ptr,
intptr_t size,
Dart_HandleFinalizer callback) {
Dart_CObject* cobject = New(Dart_CObject_kNativePointer);
cobject->value.as_native_pointer.ptr = ptr;
cobject->value.as_native_pointer.size = size;
cobject->value.as_native_pointer.callback = callback;
return cobject;
}
Dart_CObject* CObject::NewIOBuffer(int64_t length) {
// Make sure that we do not have an integer overflow here. Actual check
// against max elements will be done at the time of writing, as the constant

17
runtime/bin/dartutils.h
View file

@ -351,9 +351,6 @@ class CObject {
uint8_t* data,
void* peer,
Dart_HandleFinalizer callback);
static Dart_CObject* NewNativePointer(intptr_t ptr,
intptr_t size,
Dart_HandleFinalizer callback);
static Dart_CObject* NewIOBuffer(int64_t length);
static void ShrinkIOBuffer(Dart_CObject* cobject, int64_t new_length);
@ -582,20 +579,6 @@ class CObjectExternalUint8Array : public CObject {
DISALLOW_COPY_AND_ASSIGN(CObjectExternalUint8Array);
};
class CObjectNativePointer : public CObject {
public:
DECLARE_COBJECT_CONSTRUCTORS(NativePointer)
intptr_t Ptr() const { return cobject_->value.as_native_pointer.ptr; }
intptr_t Size() const { return cobject_->value.as_native_pointer.size; }
Dart_HandleFinalizer Callback() const {
return cobject_->value.as_native_pointer.callback;
}
private:
DISALLOW_COPY_AND_ASSIGN(CObjectNativePointer);
};
class ScopedBlockingCall {
public:
ScopedBlockingCall() { Dart_ThreadDisableProfiling(); }

4
runtime/bin/file.cc
View file

@ -923,8 +923,8 @@ CObject* File::OpenRequest(const CObjectArray& request) {
if (file == NULL) {
return CObject::NewOSError();
}
return new CObjectNativePointer(CObject::NewNativePointer(
reinterpret_cast<intptr_t>(file), sizeof(*file), ReleaseFile));
return new CObjectIntptr(
CObject::NewIntptr(reinterpret_cast<intptr_t>(file)));
}
CObject* File::DeleteRequest(const CObjectArray& request) {

6
runtime/include/dart_native_api.h
View file

@ -45,7 +45,6 @@ typedef enum {
Dart_CObject_kExternalTypedData,
Dart_CObject_kSendPort,
Dart_CObject_kCapability,
Dart_CObject_kNativePointer,
Dart_CObject_kUnsupported,
Dart_CObject_kNumberOfTypes
} Dart_CObject_Type;
@ -81,11 +80,6 @@ typedef struct _Dart_CObject {
void* peer;
Dart_HandleFinalizer callback;
} as_external_typed_data;
struct {
intptr_t ptr;
intptr_t size;
Dart_HandleFinalizer callback;
} as_native_pointer;
} value;
} Dart_CObject;
// This struct is versioned by DART_API_DL_MAJOR_VERSION, bump the version when

13
runtime/vm/class_id.h
View file

@ -203,10 +203,6 @@ enum ClassId : intptr_t {
// Illegal class id.
kIllegalCid = 0,
// Pseudo class id for native pointers, the heap should never see an
// object with this class id.
kNativePointer,
// The following entries describes classes for pseudo-objects in the heap
// that should never be reachable from live objects. Free list elements
// maintain the free list for old space, and forwarding corpses are used to
@ -285,11 +281,10 @@ COMPILE_ASSERT(kInstanceCid == kLastInternalOnlyCid + 1);
// and should not be exposed directly to user code.
inline bool IsInternalOnlyClassId(intptr_t index) {
// Fix the condition below if these become non-contiguous.
COMPILE_ASSERT(kIllegalCid + 1 == kNativePointer &&
kIllegalCid + 2 == kFreeListElement &&
kIllegalCid + 3 == kForwardingCorpse &&
kIllegalCid + 4 == kObjectCid &&
kIllegalCid + 5 == kFirstInternalOnlyCid);
COMPILE_ASSERT(kIllegalCid + 1 == kFreeListElement &&
kIllegalCid + 2 == kForwardingCorpse &&
kIllegalCid + 3 == kObjectCid &&
kIllegalCid + 4 == kFirstInternalOnlyCid);
return index <= kLastInternalOnlyCid;
}

69
runtime/vm/message_snapshot.cc
View file

@ -1818,67 +1818,6 @@ class ExternalTypedDataMessageDeserializationCluster
const intptr_t cid_;
};
class NativePointerMessageSerializationCluster
: public MessageSerializationCluster {
public:
explicit NativePointerMessageSerializationCluster(Zone* zone)
: MessageSerializationCluster("NativePointer",
MessagePhase::kNonCanonicalInstances,
kNativePointer),
objects_(zone, 0) {}
~NativePointerMessageSerializationCluster() {}
void Trace(MessageSerializer* s, Object* object) { UNREACHABLE(); }
void WriteNodes(MessageSerializer* s) { UNREACHABLE(); }
void TraceApi(ApiMessageSerializer* s, Dart_CObject* object) {
objects_.Add(object);
}
void WriteNodesApi(ApiMessageSerializer* s) {
intptr_t count = objects_.length();
s->WriteUnsigned(count);
for (intptr_t i = 0; i < count; i++) {
Dart_CObject* data = objects_[i];
s->AssignRef(data);
intptr_t ptr = data->value.as_native_pointer.ptr;
s->WriteUnsigned(ptr);
s->finalizable_data()->Put(
data->value.as_native_pointer.size, nullptr,
reinterpret_cast<void*>(data->value.as_native_pointer.ptr),
data->value.as_native_pointer.callback);
}
}
private:
GrowableArray<Dart_CObject*> objects_;
};
class NativePointerMessageDeserializationCluster
: public MessageDeserializationCluster {
public:
NativePointerMessageDeserializationCluster()
: MessageDeserializationCluster("NativePointer"), cid_(kNativePointer) {}
~NativePointerMessageDeserializationCluster() {}
void ReadNodes(MessageDeserializer* d) {
intptr_t count = d->ReadUnsigned();
for (intptr_t i = 0; i < count; i++) {
intptr_t ptr = d->ReadUnsigned();
d->finalizable_data()->Take();
d->AssignRef(Integer::New(ptr));
}
}
void ReadNodesApi(ApiMessageDeserializer* d) { UNREACHABLE(); }
private:
const intptr_t cid_;
};
class TypedDataViewMessageSerializationCluster
: public MessageSerializationCluster {
public:
@ -3250,9 +3189,6 @@ bool ApiMessageSerializer::Trace(Dart_CObject* object) {
case Dart_CObject_kCapability:
cid = kCapabilityCid;
break;
case Dart_CObject_kNativePointer:
cid = kNativePointer;
break;
default:
return Fail("invalid Dart_CObject type");
}
@ -3306,8 +3242,6 @@ MessageSerializationCluster* BaseSerializer::NewClusterForClass(
}
switch (cid) {
case kNativePointer:
return new (Z) NativePointerMessageSerializationCluster(Z);
case kClassCid:
return new (Z) ClassMessageSerializationCluster();
case kTypeArgumentsCid:
@ -3392,9 +3326,6 @@ MessageDeserializationCluster* BaseDeserializer::ReadCluster() {
}
switch (cid) {
case kNativePointer:
ASSERT(!is_canonical);
return new (Z) NativePointerMessageDeserializationCluster();
case kClassCid:
ASSERT(!is_canonical);
return new (Z) ClassMessageDeserializationCluster();

139
tests/standalone/io/file_leak_test.dart
View file

@ -1,139 +0,0 @@
// 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.
//
// Dart test program for testing file I/O.
// OtherResources=fixed_length_file
// OtherResources=read_as_text.dat
import 'dart:async';
import 'dart:convert';
import 'dart:collection';
import 'dart:io';
import "package:async_helper/async_helper.dart";
import "package:expect/expect.dart";
import "package:path/path.dart";
class FileTest {
static Directory? tempDirectory;
static int numLiveAsyncTests = 0;
static void asyncTestStarted() {
asyncStart();
++numLiveAsyncTests;
}
static void asyncTestDone(String name) {
asyncEnd();
--numLiveAsyncTests;
if (numLiveAsyncTests == 0) {
deleteTempDirectory();
}
}
static void createTempDirectory(Function doNext) {
Directory.systemTemp.createTemp('dart_file').then((temp) {
tempDirectory = temp;
doNext();
});
}
static void deleteTempDirectory() {
tempDirectory!.deleteSync(recursive: true);
}
static testReadInto() async {
asyncTestStarted();
File file = new File(tempDirectory!.path + "/out_read_into");
var openedFile = await file.open(mode: FileMode.write);
await openedFile.writeFrom(const [1, 2, 3]);
await openedFile.setPosition(0);
var list = <int>[1, 2, 3];
Expect.equals(3, await openedFile.readInto(list));
Expect.listEquals([1, 2, 3], list);
read(start, end, length, expected) async {
var list = <int>[1, 2, 3];
await openedFile.setPosition(0);
Expect.equals(length, await openedFile.readInto(list, start, end));
Expect.listEquals(expected, list);
return list;
}
await read(0, 3, 3, [1, 2, 3]);
await read(0, 2, 2, [1, 2, null]);
await read(1, 2, 1, [null, 1, null]);
await read(1, 3, 2, [null, 1, 2]);
await read(2, 3, 1, [null, null, 1]);
await read(0, 0, 0, [null, null, null]);
await openedFile.close();
asyncTestDone("testReadInto");
}
static void testReadAsText() {
asyncTestStarted();
var name = getFilename("fixed_length_file");
var f = new File(name);
f.readAsString(encoding: utf8).then((text) {
Expect.isTrue(text.endsWith("42 bytes."));
Expect.equals(42, text.length);
var name = getFilename("read_as_text.dat");
var f = new File(name);
f.readAsString(encoding: utf8).then((text) {
Expect.equals(6, text.length);
var expected = [955, 120, 46, 32, 120, 10];
Expect.listEquals(expected, text.codeUnits);
f.readAsString(encoding: latin1).then((text) {
Expect.equals(7, text.length);
var expected = [206, 187, 120, 46, 32, 120, 10];
Expect.listEquals(expected, text.codeUnits);
var readAsStringFuture = f.readAsString(encoding: ascii);
readAsStringFuture.then((text) {
Expect.fail("Non-ascii char should cause error");
}).catchError((e) {
asyncTestDone("testReadAsText");
});
});
});
});
}
static String getFilename(String path) {
return Platform.script.resolve(path).toFilePath();
}
// Main test entrypoint.
// This test results in an unhandled exception in the isolate while
// some async file IO operations are still pending. The unhandled
// exception results in the 'File' object being leaked, the error
// only shows up in the ASAN bots which detect the leak.
static testMain() {
asyncStart();
var outerZone = Zone.current;
var firstZone = Zone.current.fork(specification: ZoneSpecification(
handleUncaughtError: (self, parent, zone, error, stacktrace) {
asyncEnd();
print("unittest-suite-success"); // For the test harness.
exit(0);
}));
firstZone.run(() async {
Expect.identical(firstZone, Zone.current);
createTempDirectory(() {
testReadAsText();
testReadInto();
Expect.equals(1, 0); // Should not execute this.
asyncEnd();
});
});
}
}
main() {
FileTest.testMain();
}

141
tests/standalone_2/io/file_leak_test.dart
View file

@ -1,141 +0,0 @@
// 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.
//
// Dart test program for testing file I/O.
// @dart = 2.9
// OtherResources=fixed_length_file
// OtherResources=read_as_text.dat
import 'dart:async';
import 'dart:convert';
import 'dart:collection';
import 'dart:io';
import "package:async_helper/async_helper.dart";
import "package:expect/expect.dart";
import "package:path/path.dart";
class FileTest {
static Directory tempDirectory;
static int numLiveAsyncTests = 0;
static void asyncTestStarted() {
asyncStart();
++numLiveAsyncTests;
}
static void asyncTestDone(String name) {
asyncEnd();
--numLiveAsyncTests;
if (numLiveAsyncTests == 0) {
deleteTempDirectory();
}
}
static void createTempDirectory(Function doNext) {
Directory.systemTemp.createTemp('dart_file').then((temp) {
tempDirectory = temp;
doNext();
});
}
static void deleteTempDirectory() {
tempDirectory.deleteSync(recursive: true);
}
static testReadInto() async {
asyncTestStarted();
File file = new File(tempDirectory.path + "/out_read_into");
var openedFile = await file.open(mode: FileMode.write);
await openedFile.writeFrom(const [1, 2, 3]);
await openedFile.setPosition(0);
var list = [null, null, null];
Expect.equals(3, await openedFile.readInto(list));
Expect.listEquals([1, 2, 3], list);
read(start, end, length, expected) async {
var list = [null, null, null];
await openedFile.setPosition(0);
Expect.equals(length, await openedFile.readInto(list, start, end));
Expect.listEquals(expected, list);
return list;
}
await read(0, 3, 3, [1, 2, 3]);
await read(0, 2, 2, [1, 2, null]);
await read(1, 2, 1, [null, 1, null]);
await read(1, 3, 2, [null, 1, 2]);
await read(2, 3, 1, [null, null, 1]);
await read(0, 0, 0, [null, null, null]);
await openedFile.close();
asyncTestDone("testReadInto");
}
static void testReadAsText() {
asyncTestStarted();
var name = getFilename("fixed_length_file");
var f = new File(name);
f.readAsString(encoding: utf8).then((text) {
Expect.isTrue(text.endsWith("42 bytes."));
Expect.equals(42, text.length);
var name = getFilename("read_as_text.dat");
var f = new File(name);
f.readAsString(encoding: utf8).then((text) {
Expect.equals(6, text.length);
var expected = [955, 120, 46, 32, 120, 10];
Expect.listEquals(expected, text.codeUnits);
f.readAsString(encoding: latin1).then((text) {
Expect.equals(7, text.length);
var expected = [206, 187, 120, 46, 32, 120, 10];
Expect.listEquals(expected, text.codeUnits);
var readAsStringFuture = f.readAsString(encoding: ascii);
readAsStringFuture.then((text) {
Expect.fail("Non-ascii char should cause error");
}).catchError((e) {
asyncTestDone("testReadAsText");
});
});
});
});
}
static String getFilename(String path) {
return Platform.script.resolve(path).toFilePath();
}
// Main test entrypoint.
// This test results in an unhandled exception in the isolate while
// some async file IO operations are still pending. The unhandled
// exception results in the 'File' object being leaked, the error
// only shows up in the ASAN bots which detect the leak.
static testMain() {
asyncStart();
var outerZone = Zone.current;
var firstZone = Zone.current.fork(specification: ZoneSpecification(
handleUncaughtError: (self, parent, zone, error, stacktrace) {
asyncEnd();
print("unittest-suite-success"); // For the test harness.
exit(0);
}));
firstZone.run(() async {
Expect.identical(firstZone, Zone.current);
createTempDirectory(() {
testReadAsText();
testReadInto();
Expect.equals(1, 0); // Should not execute this.
asyncEnd();
});
});
}
}
main() {
FileTest.testMain();
}