Skip to content

Commit f280058

Browse files
alexmarkovcommit-bot@chromium.org
alexmarkov
authored and
[email protected]
committed
Revert "[VM] Fix OOM handling when allocating buffers in dart:io"
This reverts commit 1d6c102. Reason: failures on ASAN bots as ASAN is not configured to accept memory allocations with huge size. Change-Id: Ibb3b27f60017fb42e0f36caea2262125f5bb40d0 Reviewed-on: https://dart-review.googlesource.com/20900 Reviewed-by: Alexander Markov <[email protected]> Reviewed-by: Zach Anderson <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
1 parent 09c1bea commit f280058

File tree

10 files changed

+38
-81
lines changed

10 files changed

+38
-81
lines changed

‎runtime/bin/dartutils.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -945,9 +945,7 @@ Dart_CObject* CObject::NewIOBuffer(int64_t length) {
945945
return NULL;
946946
}
947947
uint8_t* data = IOBuffer::Allocate(static_cast<intptr_t>(length));
948-
if (data == NULL) {
949-
return NULL;
950-
}
948+
ASSERT(data != NULL);
951949
return NewExternalUint8Array(static_cast<intptr_t>(length), data, data,
952950
IOBuffer::Finalizer);
953951
}

‎runtime/bin/file.cc

Lines changed: 30 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -176,39 +176,36 @@ void FUNCTION_NAME(File_Read)(Dart_NativeArguments args) {
176176
ASSERT(file != NULL);
177177
Dart_Handle length_object = Dart_GetNativeArgument(args, 1);
178178
int64_t length = 0;
179-
if (!DartUtils::GetInt64Value(length_object, &length) || (length < 0)) {
180-
OSError os_error(-1, "Invalid argument", OSError::kUnknown);
181-
Dart_SetReturnValue(args, DartUtils::NewDartOSError(&os_error));
182-
return;
183-
}
184-
uint8_t* buffer = NULL;
185-
Dart_Handle external_array = IOBuffer::Allocate(length, &buffer);
186-
if (Dart_IsNull(external_array)) {
187-
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
188-
return;
189-
}
190-
int64_t bytes_read = file->Read(reinterpret_cast<void*>(buffer), length);
191-
if (bytes_read < 0) {
192-
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
193-
return;
194-
}
195-
if (bytes_read < length) {
196-
const int kNumArgs = 3;
197-
Dart_Handle dart_args[kNumArgs];
198-
dart_args[0] = external_array;
199-
dart_args[1] = Dart_NewInteger(0);
200-
dart_args[2] = Dart_NewInteger(bytes_read);
201-
// TODO(sgjesse): Cache the _makeUint8ListView function somewhere.
202-
Dart_Handle io_lib = Dart_LookupLibrary(DartUtils::NewString("dart:io"));
203-
if (Dart_IsError(io_lib)) {
204-
Dart_PropagateError(io_lib);
179+
if (DartUtils::GetInt64Value(length_object, &length)) {
180+
uint8_t* buffer = NULL;
181+
Dart_Handle external_array = IOBuffer::Allocate(length, &buffer);
182+
int64_t bytes_read = file->Read(reinterpret_cast<void*>(buffer), length);
183+
if (bytes_read < 0) {
184+
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
185+
} else {
186+
if (bytes_read < length) {
187+
const int kNumArgs = 3;
188+
Dart_Handle dart_args[kNumArgs];
189+
dart_args[0] = external_array;
190+
dart_args[1] = Dart_NewInteger(0);
191+
dart_args[2] = Dart_NewInteger(bytes_read);
192+
// TODO(sgjesse): Cache the _makeUint8ListView function somewhere.
193+
Dart_Handle io_lib =
194+
Dart_LookupLibrary(DartUtils::NewString("dart:io"));
195+
if (Dart_IsError(io_lib)) {
196+
Dart_PropagateError(io_lib);
197+
}
198+
Dart_Handle array_view =
199+
Dart_Invoke(io_lib, DartUtils::NewString("_makeUint8ListView"),
200+
kNumArgs, dart_args);
201+
Dart_SetReturnValue(args, array_view);
202+
} else {
203+
Dart_SetReturnValue(args, external_array);
204+
}
205205
}
206-
Dart_Handle array_view =
207-
Dart_Invoke(io_lib, DartUtils::NewString("_makeUint8ListView"),
208-
kNumArgs, dart_args);
209-
Dart_SetReturnValue(args, array_view);
210206
} else {
211-
Dart_SetReturnValue(args, external_array);
207+
OSError os_error(-1, "Invalid argument", OSError::kUnknown);
208+
Dart_SetReturnValue(args, DartUtils::NewDartOSError(&os_error));
212209
}
213210
}
214211

@@ -1108,9 +1105,7 @@ CObject* File::ReadRequest(const CObjectArray& request) {
11081105
}
11091106
const int64_t length = CObjectInt32OrInt64ToInt64(request[1]);
11101107
Dart_CObject* io_buffer = CObject::NewIOBuffer(length);
1111-
if (io_buffer == NULL) {
1112-
return CObject::NewOSError();
1113-
}
1108+
ASSERT(io_buffer != NULL);
11141109
uint8_t* data = io_buffer->value.as_external_typed_data.data;
11151110
const int64_t bytes_read = file->Read(data, length);
11161111
if (bytes_read < 0) {
@@ -1140,9 +1135,7 @@ CObject* File::ReadIntoRequest(const CObjectArray& request) {
11401135
}
11411136
const int64_t length = CObjectInt32OrInt64ToInt64(request[1]);
11421137
Dart_CObject* io_buffer = CObject::NewIOBuffer(length);
1143-
if (io_buffer == NULL) {
1144-
return CObject::NewOSError();
1145-
}
1138+
ASSERT(io_buffer != NULL);
11461139
uint8_t* data = io_buffer->value.as_external_typed_data.data;
11471140
const int64_t bytes_read = file->Read(data, length);
11481141
if (bytes_read < 0) {

‎runtime/bin/filter.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,10 +246,6 @@ void FUNCTION_NAME(Filter_Processed)(Dart_NativeArguments args) {
246246
} else {
247247
uint8_t* io_buffer;
248248
Dart_Handle result = IOBuffer::Allocate(read, &io_buffer);
249-
if (Dart_IsNull(result)) {
250-
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
251-
return;
252-
}
253249
memmove(io_buffer, filter->processed_buffer(), read);
254250
Dart_SetReturnValue(args, result);
255251
}

‎runtime/bin/io_buffer.cc

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,6 @@ namespace bin {
99

1010
Dart_Handle IOBuffer::Allocate(intptr_t size, uint8_t** buffer) {
1111
uint8_t* data = Allocate(size);
12-
if (data == NULL) {
13-
return Dart_Null();
14-
}
1512
Dart_Handle result =
1613
Dart_NewExternalTypedData(Dart_TypedData_kUint8, data, size);
1714
Dart_NewWeakPersistentHandle(result, data, size, IOBuffer::Finalizer);
@@ -27,7 +24,7 @@ Dart_Handle IOBuffer::Allocate(intptr_t size, uint8_t** buffer) {
2724
}
2825

2926
uint8_t* IOBuffer::Allocate(intptr_t size) {
30-
return reinterpret_cast<uint8_t*>(malloc(size));
27+
return new uint8_t[size];
3128
}
3229

3330
} // namespace bin

‎runtime/bin/io_buffer.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ class IOBuffer {
2222

2323
// Function for disposing of IO buffer storage. All backing storage
2424
// for IO buffers must be freed using this function.
25-
static void Free(void* buffer) { free(buffer); }
25+
static void Free(void* buffer) {
26+
delete[] reinterpret_cast<uint8_t*>(buffer);
27+
}
2628

2729
// Function for finalizing external byte arrays used as IO buffers.
2830
static void Finalizer(void* isolate_callback_data,

‎runtime/bin/process.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -346,10 +346,6 @@ void FUNCTION_NAME(StringToSystemEncoding)(Dart_NativeArguments args) {
346346
}
347347
uint8_t* buffer = NULL;
348348
Dart_Handle external_array = IOBuffer::Allocate(system_len, &buffer);
349-
if (Dart_IsNull(external_array)) {
350-
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
351-
return;
352-
}
353349
if (!Dart_IsError(external_array)) {
354350
memmove(buffer, system_string, system_len);
355351
}

‎runtime/bin/process.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,6 @@ class BufferListBase {
247247
uint8_t* buffer;
248248
intptr_t buffer_position = 0;
249249
Dart_Handle result = IOBuffer::Allocate(data_size_, &buffer);
250-
if (Dart_IsNull(result)) {
251-
return DartUtils::NewDartOSError();
252-
}
253250
if (Dart_IsError(result)) {
254251
Free();
255252
return result;

‎runtime/bin/socket.cc

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -323,17 +323,12 @@ void FUNCTION_NAME(Socket_Read)(Dart_NativeArguments args) {
323323
Socket* socket =
324324
Socket::GetSocketIdNativeField(Dart_GetNativeArgument(args, 0));
325325
int64_t length = 0;
326-
if (DartUtils::GetInt64Value(Dart_GetNativeArgument(args, 1), &length) &&
327-
(length >= 0)) {
326+
if (DartUtils::GetInt64Value(Dart_GetNativeArgument(args, 1), &length)) {
328327
if (Socket::short_socket_read()) {
329328
length = (length + 1) / 2;
330329
}
331330
uint8_t* buffer = NULL;
332331
Dart_Handle result = IOBuffer::Allocate(length, &buffer);
333-
if (Dart_IsNull(result)) {
334-
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
335-
return;
336-
}
337332
if (Dart_IsError(result)) {
338333
Dart_PropagateError(result);
339334
}
@@ -345,10 +340,6 @@ void FUNCTION_NAME(Socket_Read)(Dart_NativeArguments args) {
345340
} else if (bytes_read > 0) {
346341
uint8_t* new_buffer = NULL;
347342
Dart_Handle new_result = IOBuffer::Allocate(bytes_read, &new_buffer);
348-
if (Dart_IsNull(new_result)) {
349-
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
350-
return;
351-
}
352343
if (Dart_IsError(new_result)) {
353344
Dart_PropagateError(new_result);
354345
}
@@ -402,10 +393,6 @@ void FUNCTION_NAME(Socket_RecvFrom)(Dart_NativeArguments args) {
402393
ASSERT(bytes_read > 0);
403394
uint8_t* data_buffer = NULL;
404395
Dart_Handle data = IOBuffer::Allocate(bytes_read, &data_buffer);
405-
if (Dart_IsNull(data)) {
406-
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
407-
return;
408-
}
409396
if (Dart_IsError(data)) {
410397
Dart_PropagateError(data);
411398
}

‎runtime/bin/sync_socket.cc

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -213,29 +213,20 @@ void FUNCTION_NAME(SynchronousSocket_Read)(Dart_NativeArguments args) {
213213
DART_CHECK_ERROR(result);
214214

215215
int64_t length = 0;
216-
if (!DartUtils::GetInt64Value(Dart_GetNativeArgument(args, 1), &length) ||
217-
(length < 0)) {
216+
if (!DartUtils::GetInt64Value(Dart_GetNativeArgument(args, 1), &length)) {
218217
Dart_SetReturnValue(args, DartUtils::NewDartArgumentError(
219218
"First parameter must be an integer."));
220219
return;
221220
}
222221
uint8_t* buffer = NULL;
223222
result = IOBuffer::Allocate(length, &buffer);
224-
if (Dart_IsNull(result)) {
225-
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
226-
return;
227-
}
228223
ASSERT(buffer != NULL);
229224
intptr_t bytes_read = SynchronousSocket::Read(socket->fd(), buffer, length);
230225
if (bytes_read == length) {
231226
Dart_SetReturnValue(args, result);
232227
} else if (bytes_read > 0) {
233228
uint8_t* new_buffer = NULL;
234229
Dart_Handle new_result = IOBuffer::Allocate(bytes_read, &new_buffer);
235-
if (Dart_IsNull(new_result)) {
236-
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
237-
return;
238-
}
239230
ASSERT(new_buffer != NULL);
240231
memmove(new_buffer, buffer, bytes_read);
241232
Dart_SetReturnValue(args, new_result);

‎tests/standalone_2/io/file_error_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ testRepeatedlyCloseFileSync() {
400400

401401
testReadSyncBigInt() {
402402
createTestFile((file, done) {
403-
var bigint = 9223372036854775807;
403+
var bigint = 100000000000000000000000000000000000000000;
404404
var openedFile = file.openSync();
405405
Expect.throws(
406406
() => openedFile.readSync(bigint), (e) => e is FileSystemException);

0 commit comments

Comments
 (0)