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 <aam@google.com>
Commit-Queue: Brian Quinlan <bquinlan@google.com>

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

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<dynamic> _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<dynamic> 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<dynamic> _toRawSocket();
   @pragma("vm:external-name", "ResourceHandleImpl_toRawDatagramSocket")
-  external RawDatagramSocket toRawDatagramSocket();
+  external RawDatagramSocket _toRawDatagramSocket();
 
   @pragma("vm:entry-point")
   static final _ResourceHandleImpl _sentinel = _ResourceHandleImpl(-1);

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.

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<String> args) async {
+  if (!Platform.isMacOS && !Platform.isLinux && !Platform.isAndroid) {
+    return;
+  }
+
+  await withTempDir('resource_handle_test', (Directory dir) async {
+    await testToCallsAfterFromFile('${dir.path}');
+  });
+}

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<String> args) async {
+  if (!Platform.isMacOS && !Platform.isLinux && !Platform.isAndroid) {
+    return;
+  }
+
+  await withTempDir('resource_handle_test', (Directory dir) async {
+    await testToCallsAfterFromFile('${dir.path}');
+  });
+}