From fa53fb66d06f60e3cec847cb3543088d40ad986c Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 14 Sep 2022 18:17:33 +0000 Subject: [PATCH] Throw an error if an attempt is made to create more than one resource from a ResourceHandle Change-Id: I629d2b9cfddd8e5f7fe8ae65a03362c7bcce5e2c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/257000 Reviewed-by: Alexander Aprelev Commit-Queue: Brian Quinlan --- CHANGELOG.md | 9 ++++ sdk/lib/_internal/vm/bin/socket_patch.dart | 44 ++++++++++++++++--- sdk/lib/io/socket.dart | 24 ++++++++++ tests/standalone/io/resource_handle_test.dart | 41 +++++++++++++++++ .../standalone_2/io/resource_handle_test.dart | 43 ++++++++++++++++++ 5 files changed, 154 insertions(+), 7 deletions(-) create mode 100644 tests/standalone/io/resource_handle_test.dart create mode 100644 tests/standalone_2/io/resource_handle_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index 65bc8026670..1ddc7e090cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,15 @@ Additionally, a type test of the form `v is Never` (where `v` is a local variable) no longer promotes `v` to type `Never`. +- **Breaking change** [#49878][]: Calling `ResourceHandle.toFile()`, + `ResourceHandle.toSocket()`, `ResourceHandle.toRawSocket()` or + `ResourceHandle.toRawDatagramSocket()`, more than once now throws a + `StateError`. + + The previous behavior would allow multiple Dart objects to refer to the same + file descriptor, which would produce errors when one object was closed or + garbage collected. + [#49635]: https://github.com/dart-lang/sdk/issues/49635 ### Libraries diff --git a/sdk/lib/_internal/vm/bin/socket_patch.dart b/sdk/lib/_internal/vm/bin/socket_patch.dart index 159aa850594..d0746e0bb65 100644 --- a/sdk/lib/_internal/vm/bin/socket_patch.dart +++ b/sdk/lib/_internal/vm/bin/socket_patch.dart @@ -2607,19 +2607,35 @@ class ResourceHandle { @pragma("vm:entry-point") class _ResourceHandleImpl implements ResourceHandle { + bool _toMethodCalled = false; + @pragma("vm:entry-point") int _handle; // file descriptor on linux @pragma("vm:entry-point") _ResourceHandleImpl(this._handle); - @pragma("vm:external-name", "ResourceHandleImpl_toFile") - external RandomAccessFile toFile(); - @pragma("vm:external-name", "ResourceHandleImpl_toSocket") - external Socket toSocket(); - @pragma("vm:external-name", "ResourceHandleImpl_toRawSocket") - external List _toRawSocket(); + RandomAccessFile toFile() { + if (_toMethodCalled) { + throw StateError('Resource handle has already been used.'); + } + _toMethodCalled = true; + return _toFile(); + } + + RawDatagramSocket toRawDatagramSocket() { + if (_toMethodCalled) { + throw StateError('Resource handle has already been used.'); + } + _toMethodCalled = true; + return _toRawDatagramSocket(); + } RawSocket toRawSocket() { + if (_toMethodCalled) { + throw StateError('Resource handle has already been used.'); + } + _toMethodCalled = true; + List list = _toRawSocket(); InternetAddressType type = InternetAddressType._from(list[0] as int); String hostname = list[1] as String; @@ -2633,6 +2649,14 @@ class _ResourceHandleImpl implements ResourceHandle { return _RawSocket(nativeSocket); } + Socket toSocket() { + if (_toMethodCalled) { + throw StateError('Resource handle has already been used.'); + } + _toMethodCalled = true; + return _toSocket(); + } + _ReadPipe toReadPipe() { return _ReadPipe(toFile()); } @@ -2641,8 +2665,14 @@ class _ResourceHandleImpl implements ResourceHandle { return _WritePipe(toFile()); } + @pragma("vm:external-name", "ResourceHandleImpl_toFile") + external RandomAccessFile _toFile(); + @pragma("vm:external-name", "ResourceHandleImpl_toSocket") + external Socket _toSocket(); + @pragma("vm:external-name", "ResourceHandleImpl_toRawSocket") + external List _toRawSocket(); @pragma("vm:external-name", "ResourceHandleImpl_toRawDatagramSocket") - external RawDatagramSocket toRawDatagramSocket(); + external RawDatagramSocket _toRawDatagramSocket(); @pragma("vm:entry-point") static final _ResourceHandleImpl _sentinel = _ResourceHandleImpl(-1); diff --git a/sdk/lib/io/socket.dart b/sdk/lib/io/socket.dart index a660e885d14..0f76121466a 100644 --- a/sdk/lib/io/socket.dart +++ b/sdk/lib/io/socket.dart @@ -880,6 +880,10 @@ abstract class ResourceHandle { /// This can also be used when receiving stdin and stdout handles and read /// and write pipes. /// + /// Since the [ResourceHandle] represents a single OS resource, + /// none of [toFile], [toSocket], [toRawSocket], or [toRawDatagramSocket], + /// [toReadPipe], [toWritePipe], can be called after a call to this method. + /// /// If this resource handle is not a file or stdio handle, the behavior of the /// returned [RandomAccessFile] is completely unspecified. /// Be very careful to avoid using a handle incorrectly. @@ -887,6 +891,10 @@ abstract class ResourceHandle { /// Extracts opened socket from resource handle. /// + /// Since the [ResourceHandle] represents a single OS resource, + /// none of [toFile], [toSocket], [toRawSocket], or [toRawDatagramSocket], + /// [toReadPipe], [toWritePipe], can be called after a call to this method. + // /// If this resource handle is not a socket handle, the behavior of the /// returned [Socket] is completely unspecified. /// Be very careful to avoid using a handle incorrectly. @@ -894,6 +902,10 @@ abstract class ResourceHandle { /// Extracts opened raw socket from resource handle. /// + /// Since the [ResourceHandle] represents a single OS resource, + /// none of [toFile], [toSocket], [toRawSocket], or [toRawDatagramSocket], + /// [toReadPipe], [toWritePipe], can be called after a call to this method. + /// /// If this resource handle is not a socket handle, the behavior of the /// returned [RawSocket] is completely unspecified. /// Be very careful to avoid using a handle incorrectly. @@ -901,6 +913,10 @@ abstract class ResourceHandle { /// Extracts opened raw datagram socket from resource handle. /// + /// Since the [ResourceHandle] represents a single OS resource, + /// none of [toFile], [toSocket], [toRawSocket], or [toRawDatagramSocket], + /// [toReadPipe], [toWritePipe], can be called after a call to this method. + /// /// If this resource handle is not a datagram socket handle, the behavior of /// the returned [RawDatagramSocket] is completely unspecified. /// Be very careful to avoid using a handle incorrectly. @@ -908,6 +924,10 @@ abstract class ResourceHandle { /// Extracts a read pipe from resource handle. /// + /// Since the [ResourceHandle] represents a single OS resource, + /// none of [toFile], [toSocket], [toRawSocket], or [toRawDatagramSocket], + /// [toReadPipe], [toWritePipe], can be called after a call to this method. + /// /// If this resource handle is not a readable pipe, the behavior of the /// returned [ReadPipe] is completely unspecified. /// Be very careful to avoid using a handle incorrectly. @@ -915,6 +935,10 @@ abstract class ResourceHandle { /// Extracts a write pipe from resource handle. /// + /// Since the [ResourceHandle] represents a single OS resource, + /// none of [toFile], [toSocket], [toRawSocket], or [toRawDatagramSocket], + /// [toReadPipe], [toWritePipe], can be called after a call to this method. + /// /// If this resource handle is not a writeable pipe, the behavior of the /// returned [ReadPipe] is completely unspecified. /// Be very careful to avoid using a handle incorrectly. diff --git a/tests/standalone/io/resource_handle_test.dart b/tests/standalone/io/resource_handle_test.dart new file mode 100644 index 00000000000..2ad922ced25 --- /dev/null +++ b/tests/standalone/io/resource_handle_test.dart @@ -0,0 +1,41 @@ +// Copyright (c) 2022, 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. + +// Most tests for [ResourceHandle] are in unix_socket_test.dart. + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:expect/expect.dart'; + +import 'test_utils.dart' show throws, withTempDir; + +Future testToCallsAfterFromFile(String tempDirPath) async { + final file = File('$tempDirPath/sock1'); + + await file.create(); + final openFile = await file.open(); + + final handle = ResourceHandle.fromFile(openFile); + + handle.toFile().close(); + + throws(handle.toFile, (e) => e is StateError); + throws(handle.toRawDatagramSocket, (e) => e is StateError); + throws(handle.toRawSocket, (e) => e is StateError); + throws(handle.toSocket, (e) => e is StateError); + throws(handle.toReadPipe, (e) => e is StateError); + throws(handle.toWritePipe, (e) => e is StateError); +} + +void main(List args) async { + if (!Platform.isMacOS && !Platform.isLinux && !Platform.isAndroid) { + return; + } + + await withTempDir('resource_handle_test', (Directory dir) async { + await testToCallsAfterFromFile('${dir.path}'); + }); +} diff --git a/tests/standalone_2/io/resource_handle_test.dart b/tests/standalone_2/io/resource_handle_test.dart new file mode 100644 index 00000000000..11a25c82865 --- /dev/null +++ b/tests/standalone_2/io/resource_handle_test.dart @@ -0,0 +1,43 @@ +// Copyright (c) 2022, 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 = 2.9 + +// Most tests for [ResourceHandle] are in unix_socket_test.dart. + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:expect/expect.dart'; + +import 'test_utils.dart' show throws, withTempDir; + +Future testToCallsAfterFromFile(String tempDirPath) async { + final file = File('$tempDirPath/sock1'); + + await file.create(); + final openFile = await file.open(); + + final handle = ResourceHandle.fromFile(openFile); + + handle.toFile().close(); + + throws(handle.toFile, (e) => e is StateError); + throws(handle.toRawDatagramSocket, (e) => e is StateError); + throws(handle.toRawSocket, (e) => e is StateError); + throws(handle.toSocket, (e) => e is StateError); + throws(handle.toReadPipe, (e) => e is StateError); + throws(handle.toWritePipe, (e) => e is StateError); +} + +void main(List args) async { + if (!Platform.isMacOS && !Platform.isLinux && !Platform.isAndroid) { + return; + } + + await withTempDir('resource_handle_test', (Directory dir) async { + await testToCallsAfterFromFile('${dir.path}'); + }); +}