[io] Fix a bug where large reads would return partial data.

Bug: https://github.com/dart-lang/sdk/issues/51071
Change-Id: Ia64d803c9709b106e52a1c671c1c3288c051bd85
Tested: ci + new test
CoreLibraryReviewExempt: bug fix only for vm
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279204
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Brian Quinlan <bquinlan@google.com>
This commit is contained in:
Brian Quinlan 2023-01-31 18:41:39 +00:00 committed by Commit Queue
parent d0131191bd
commit 252015b30b
4 changed files with 218 additions and 11 deletions

View file

@ -143,9 +143,15 @@ class File : public ReferenceCounted<File> {
int64_t length,
void* start = nullptr);
// Read/Write attempt to transfer num_bytes to/from buffer. It returns
// the number of bytes read/written.
// Read at most 'num_bytes' from the file. It may read less than 'num_bytes'
// even when EOF is not encountered. If no data is available then `Read`
// will block waiting for input (e.g. if the file represents a pipe that
// contains no unread data). A positive return value indicates the number of
// bytes read. Zero indicates an attempt to read past the end-of-file. -1
// indicates an error.
int64_t Read(void* buffer, int64_t num_bytes);
// Attempt to write 'num_bytes' bytes from 'buffer'. It returns the number
// of bytes written.
int64_t Write(const void* buffer, int64_t num_bytes);
// ReadFully and WriteFully do attempt to transfer num_bytes to/from

View file

@ -7,6 +7,23 @@ part of dart.io;
// Read the file in blocks of size 64k.
const int _blockSize = 64 * 1024;
// The maximum number of bytes to read in a single call to `read`.
//
// On Windows and macOS, it is an error to call
// `read/_read(fildes, buf, nbyte)` with `nbyte >= INT_MAX`.
//
// The POSIX specification states that the behavior of `read` is
// implementation-defined if `nbyte > SSIZE_MAX`. On Linux, the `read` will
// transfer at most 0x7ffff000 bytes and return the number of bytes actually.
// transfered.
//
// A smaller value has the advantage of:
// 1. making vm-service clients (e.g. debugger) more responsive
// 2. reducing memory overhead (since `readInto` copies memory)
//
// A bigger value reduces the number of system calls.
const int _maxReadSize = 16 * 1024 * 1024; // 16MB.
class _FileStream extends Stream<List<int>> {
// Stream controller.
late StreamController<Uint8List> _controller;
@ -500,7 +517,7 @@ class _File extends FileSystemEntity implements File {
}
Future<Uint8List> readAsBytes() {
Future<Uint8List> readDataChunked(RandomAccessFile file) {
Future<Uint8List> readUnsized(RandomAccessFile file) {
var builder = new BytesBuilder(copy: false);
var completer = new Completer<Uint8List>();
void read() {
@ -518,13 +535,37 @@ class _File extends FileSystemEntity implements File {
return completer.future;
}
Future<Uint8List> readSized(RandomAccessFile file, int length) {
var data = Uint8List(length);
var offset = 0;
var completer = new Completer<Uint8List>();
void read() {
file.readInto(data, offset, min(offset + _maxReadSize, length)).then(
(readSize) {
if (readSize > 0) {
offset += readSize;
read();
} else {
assert(readSize == 0);
if (offset < length) {
data = Uint8List.sublistView(data, 0, offset);
}
completer.complete(data);
}
}, onError: completer.completeError);
}
read();
return completer.future;
}
return open().then((file) {
return file.length().then((length) {
if (length == 0) {
// May be character device, try to read it in chunks.
return readDataChunked(file);
return readUnsized(file);
}
return file.read(length);
return readSized(file, length);
}).whenComplete(file.close);
});
}
@ -539,11 +580,27 @@ class _File extends FileSystemEntity implements File {
var builder = new BytesBuilder(copy: false);
do {
data = opened.readSync(_blockSize);
if (data.length > 0) builder.add(data);
if (data.length > 0) {
builder.add(data);
}
} while (data.length > 0);
data = builder.takeBytes();
} else {
data = opened.readSync(length);
data = Uint8List(length);
var offset = 0;
while (offset < length) {
final readSize = opened.readIntoSync(
data, offset, min(offset + _maxReadSize, length));
if (readSize == 0) {
break;
}
offset += readSize;
}
if (offset < length) {
data = Uint8List.view(data.buffer, 0, offset);
}
}
return data;
} finally {

View file

@ -14,6 +14,7 @@ import 'dart:async';
import 'dart:convert';
import 'dart:collection';
import 'dart:io';
import 'dart:typed_data';
import "package:async_helper/async_helper.dart";
import "package:expect/expect.dart";
@ -41,6 +42,10 @@ class MyListOfOneElement extends Object
class FileTest {
static late Directory tempDirectory;
static late File largeFile;
static final largeFileLastBytes = [104, 101, 108, 108, 111];
static final largeFileSize = (1 << 31) + largeFileLastBytes.length;
static int numLiveAsyncTests = 0;
static void asyncTestStarted() {
@ -63,6 +68,26 @@ class FileTest {
});
}
static void createLargeFile(Function doNext) {
// Increase the test count to prevent the temp directory from being
// deleted.
++numLiveAsyncTests;
final buffer = Uint8List(1 << 24);
largeFile = new File('${tempDirectory.path}/test_large_file');
IOSink output = largeFile.openWrite();
for (var i = 0;
i < (largeFileSize - largeFileLastBytes.length) / buffer.length;
++i) {
output.add(buffer);
}
output.add(largeFileLastBytes);
output.flush().then((_) => output.close());
output.done.then((_) {
--numLiveAsyncTests;
doNext();
});
}
static void deleteTempDirectory() {
tempDirectory.deleteSync(recursive: true);
}
@ -1132,6 +1157,27 @@ class FileTest {
});
}
static void testReadAsBytesLargeFile() {
asyncTestStarted();
// On Windows and macOS, it is an error to call
// `read/_read(fildes, buf, nbyte)` with `nbyte >= INT_MAX` and, on Linux,
// it returns partial data.
largeFile.readAsBytes().then((bytes) {
Expect.equals(largeFileSize, bytes.length);
Expect.listEquals(
largeFileLastBytes,
Uint8List.sublistView(
bytes, bytes.length - largeFileLastBytes.length));
asyncTestDone("testReadAsBytesLargeFile");
}, onError: (e) {
// The test fails on 32-bit platforms or when using compressed
// pointers. It is not possible to identify when running with
// compressed pointers.
Expect.type<OutOfMemoryError>(e);
asyncTestDone("testReadAsBytesLargeFile");
});
}
static void testReadAsBytesSync() {
var name = getFilename("fixed_length_file");
var bytes = new File(name).readAsBytesSync();
@ -1145,6 +1191,27 @@ class FileTest {
Expect.equals(bytes.length, 0);
}
static testReadAsBytesSyncLargeFile() {
asyncTestStarted();
// On Windows and macOS, it is an error to call
// `read/_read(fildes, buf, nbyte)` with `nbyte >= INT_MAX` and, on Linux,
// it returns partial data.
Uint8List? bytes;
try {
bytes = largeFile.readAsBytesSync();
} on OutOfMemoryError catch (e) {
// The test fails on 32-bit platforms or when using compressed
// pointers. It is not possible to identify when running with
// compressed pointers.
asyncTestDone("testReadAsBytesSyncLargeFile");
return;
}
Expect.equals(largeFileSize, bytes.length);
Expect.listEquals(largeFileLastBytes,
Uint8List.sublistView(bytes, bytes.length - largeFileLastBytes.length));
asyncTestDone("testReadAsBytesSyncLargeFile");
}
static void testReadAsText() {
asyncTestStarted();
var name = getFilename("fixed_length_file");
@ -1612,7 +1679,8 @@ class FileTest {
var absFile = file.absolute;
Expect.isTrue(absFile.isAbsolute);
Expect.isTrue(absFile.path.startsWith(tempDirectory.path));
Expect.isTrue(absFile.path.startsWith(tempDirectory.path),
'${absFile.path} not in ${tempDirectory.path}');
Expect.equals("content", absFile.readAsStringSync());
@ -1729,7 +1797,11 @@ class FileTest {
testSetLastAccessedSyncDirectory();
testDoubleAsyncOperation();
testAbsolute();
asyncEnd();
createLargeFile(() {
testReadAsBytesLargeFile();
testReadAsBytesSyncLargeFile();
asyncEnd();
});
});
}
}

View file

@ -16,6 +16,7 @@ import 'dart:async';
import 'dart:convert';
import 'dart:collection';
import 'dart:io';
import 'dart:typed_data';
import "package:async_helper/async_helper.dart";
import "package:expect/expect.dart";
@ -43,6 +44,10 @@ class MyListOfOneElement extends Object
class FileTest {
static Directory tempDirectory;
static File largeFile;
static final largeFileLastBytes = [104, 101, 108, 108, 111];
static final largeFileSize = (1 << 31) + largeFileLastBytes.length;
static int numLiveAsyncTests = 0;
static void asyncTestStarted() {
@ -65,6 +70,26 @@ class FileTest {
});
}
static void createLargeFile(Function doNext) {
// Increase the test count to prevent the temp directory from being
// deleted.
++numLiveAsyncTests;
final buffer = Uint8List(1 << 24);
largeFile = new File('${tempDirectory.path}/test_large_file');
IOSink output = largeFile.openWrite();
for (var i = 0;
i < (largeFileSize - largeFileLastBytes.length) / buffer.length;
++i) {
output.add(buffer);
}
output.add(largeFileLastBytes);
output.flush().then((_) => output.close());
output.done.then((_) {
--numLiveAsyncTests;
doNext();
});
}
static void deleteTempDirectory() {
tempDirectory.deleteSync(recursive: true);
}
@ -1136,6 +1161,27 @@ class FileTest {
});
}
static void testReadAsBytesLargeFile() {
asyncTestStarted();
// On Windows and macOS, it is an error to call
// `read/_read(fildes, buf, nbyte)` with `nbyte >= INT_MAX` and, on Linux,
// it returns partial data.
largeFile.readAsBytes().then((bytes) {
Expect.equals(largeFileSize, bytes.length);
Expect.listEquals(
largeFileLastBytes,
Uint8List.sublistView(
bytes, bytes.length - largeFileLastBytes.length));
asyncTestDone("testReadAsBytesLargeFile");
}, onError: (e) {
// The test fails on 32-bit platforms or when using compressed
// pointers. It is not possible to identify when running with
// compressed pointers.
Expect.type<OutOfMemoryError>(e);
asyncTestDone("testReadAsBytesLargeFile");
});
}
static void testReadAsBytesSync() {
var name = getFilename("fixed_length_file");
var bytes = new File(name).readAsBytesSync();
@ -1149,6 +1195,27 @@ class FileTest {
Expect.equals(bytes.length, 0);
}
static testReadAsBytesSyncLargeFile() {
asyncTestStarted();
// On Windows and macOS, it is an error to call
// `read/_read(fildes, buf, nbyte)` with `nbyte >= INT_MAX` and, on Linux,
// it returns partial data.
Uint8List bytes;
try {
bytes = largeFile.readAsBytesSync();
} on OutOfMemoryError catch (e) {
// The test fails on 32-bit platforms or when using compressed
// pointers. It is not possible to identify when running with
// compressed pointers.
asyncTestDone("testReadAsBytesSyncLargeFile");
return;
}
Expect.equals(largeFileSize, bytes.length);
Expect.listEquals(largeFileLastBytes,
Uint8List.sublistView(bytes, bytes.length - largeFileLastBytes.length));
asyncTestDone("testReadAsBytesSyncLargeFile");
}
static void testReadAsText() {
asyncTestStarted();
var name = getFilename("fixed_length_file");
@ -1613,7 +1680,8 @@ class FileTest {
var absFile = file.absolute;
Expect.isTrue(absFile.isAbsolute);
Expect.isTrue(absFile.path.startsWith(tempDirectory.path));
Expect.isTrue(absFile.path.startsWith(tempDirectory.path),
'${absFile.path} not in ${tempDirectory.path}');
Expect.equals("content", absFile.readAsStringSync());
@ -1730,7 +1798,11 @@ class FileTest {
testSetLastAccessedSyncDirectory();
testDoubleAsyncOperation();
testAbsolute();
asyncEnd();
createLargeFile(() {
testReadAsBytesLargeFile();
testReadAsBytesSyncLargeFile();
asyncEnd();
});
});
}
}