From 4cd6243d77184363ab2779f67fb8c0198047d244 Mon Sep 17 00:00:00 2001 From: Jonas Termansen Date: Mon, 3 Feb 2020 15:10:45 +0000 Subject: [PATCH] [dart:io] Backport semantic changes from the dart:io NNBD migration. The NNBD migration required making subtle changes to some dart:io semantics in order to provide a better API. This change backports these semantic changes to the unmigrated SDK so any issues can be discovered now instead of blocking the future SDK unfork. The Process class will now throw a StateError if the process is detached upon accessing the stdin, stdout, stderr, and exitCode getters. The Socket class will now throw a SocketException if the socket has been destroyed or upgraded to a secure socket upon setting or getting socket options. Change-Id: I956fd07e713e51ebd479ebbfe4790d8d2fdf0744 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/133989 Commit-Queue: Jonas Termansen Reviewed-by: Lasse R.H. Nielsen --- CHANGELOG.md | 10 ++++++++ sdk/lib/_internal/vm/bin/process_patch.dart | 17 ++++++------- sdk/lib/_internal/vm/bin/socket_patch.dart | 11 +++----- sdk/lib/io/socket.dart | 25 +++++++++++++++++-- .../lib/_internal/vm/bin/process_patch.dart | 2 +- .../io/process_detached_test.dart | 12 ++++----- 6 files changed, 50 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c21b05c5d9a..cb5f0dbedf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,16 @@ used (see Issue [39627][]). #### `dart:io` +* The `Process` class will now throw a `StateError` if the process is detached + upon accessing the `exitCode` getter. It also now throws when the process is + detached without stdio upon accessing the `stdin`, `stdout`, and `stderr` + getters. Previously these getters would all return `null`. + +* The `Socket` class will now throw a `SocketException` if the socket has been + destroyed or upgraded to a secure socket upon setting or getting socket + options. Previously setting a socket options would be ignored and getting a + socket option would return `null`. + ### Dart VM ### Tools diff --git a/sdk/lib/_internal/vm/bin/process_patch.dart b/sdk/lib/_internal/vm/bin/process_patch.dart index 7fc67dc781d..539bb5f7a9a 100644 --- a/sdk/lib/_internal/vm/bin/process_patch.dart +++ b/sdk/lib/_internal/vm/bin/process_patch.dart @@ -534,19 +534,16 @@ class _ProcessImpl extends _ProcessImplNativeWrapper implements Process { _wait(_NativeSocket stdin, _NativeSocket stdout, _NativeSocket stderr, _NativeSocket exitHandler) native "Process_Wait"; - Stream> get stdout { - return _stdout; - } + Stream> get stdout => + _stdout ?? (throw StateError("Process is detached")); - Stream> get stderr { - return _stderr; - } + Stream> get stderr => + _stderr ?? (throw StateError("Process is detached")); - IOSink get stdin { - return _stdin; - } + IOSink get stdin => _stdin ?? (throw StateError("Process is detached")); - Future get exitCode => _exitCode != null ? _exitCode.future : null; + Future get exitCode => + _exitCode?.future ?? (throw StateError("Process is detached")); bool kill([ProcessSignal signal = ProcessSignal.sigterm]) { if (signal is! ProcessSignal) { diff --git a/sdk/lib/_internal/vm/bin/socket_patch.dart b/sdk/lib/_internal/vm/bin/socket_patch.dart index e8e2bd1e390..2e75234442c 100644 --- a/sdk/lib/_internal/vm/bin/socket_patch.dart +++ b/sdk/lib/_internal/vm/bin/socket_patch.dart @@ -1779,40 +1779,37 @@ class _Socket extends Stream implements Socket { } bool setOption(SocketOption option, bool enabled) { - if (_raw == null) return false; + if (_raw == null) throw const SocketException.closed(); return _raw.setOption(option, enabled); } Uint8List getRawOption(RawSocketOption option) { - if (_raw == null) return null; + if (_raw == null) throw const SocketException.closed(); return _raw.getRawOption(option); } void setRawOption(RawSocketOption option) { - _raw?.setRawOption(option); + if (_raw == null) throw const SocketException.closed(); + _raw.setRawOption(option); } int get port { if (_raw == null) throw const SocketException.closed(); - ; return _raw.port; } InternetAddress get address { if (_raw == null) throw const SocketException.closed(); - ; return _raw.address; } int get remotePort { if (_raw == null) throw const SocketException.closed(); - ; return _raw.remotePort; } InternetAddress get remoteAddress { if (_raw == null) throw const SocketException.closed(); - ; return _raw.remoteAddress; } diff --git a/sdk/lib/io/socket.dart b/sdk/lib/io/socket.dart index 7b70858b419..2b7edeb76bd 100644 --- a/sdk/lib/io/socket.dart +++ b/sdk/lib/io/socket.dart @@ -652,21 +652,29 @@ abstract class RawSocket implements Stream { /** * Returns the port used by this socket. + * + * Throws a [SocketException] if the socket is closed. */ int get port; /** * Returns the remote port connected to by this socket. + * + * Throws a [SocketException] if the socket is closed. */ int get remotePort; /** * Returns the [InternetAddress] used to connect this socket. + * + * Throws a [SocketException] if the socket is closed. */ InternetAddress get address; /** * Returns the remote [InternetAddress] connected to by this socket. + * + * Throws a [SocketException] if the socket is closed. */ InternetAddress get remoteAddress; @@ -791,6 +799,9 @@ abstract class Socket implements Stream, IOSink { * available options. * * Returns [:true:] if the option was set successfully, false otherwise. + * + * Throws a [SocketException] if the socket has been destroyed or upgraded to + * a secure socket. */ bool setOption(SocketOption option, bool enabled); @@ -800,7 +811,8 @@ abstract class Socket implements Stream, IOSink { * * Returns the [RawSocketOption.value] on success. * - * Throws an [OSError] on failure. + * Throws an [OSError] on failure and a [SocketException] if the socket has + * been destroyed or upgraded to a secure socket. */ Uint8List getRawOption(RawSocketOption option); @@ -808,27 +820,36 @@ abstract class Socket implements Stream, IOSink { * Use [setRawOption] to customize the [RawSocket]. See [RawSocketOption] for * available options. * - * Throws an [OSError] on failure. + * Throws an [OSError] on failure and a [SocketException] if the socket has + * been destroyed or upgraded to a secure socket. */ void setRawOption(RawSocketOption option); /** * Returns the port used by this socket. + * + * Throws a [SocketException] if the socket is closed. */ int get port; /** * Returns the remote port connected to by this socket. + * + * Throws a [SocketException] if the socket is closed. */ int get remotePort; /** * Returns the [InternetAddress] used to connect this socket. + * + * Throws a [SocketException] if the socket is closed. */ InternetAddress get address; /** * Returns the remote [InternetAddress] connected to by this socket. + * + * Throws a [SocketException] if the socket is closed. */ InternetAddress get remoteAddress; diff --git a/sdk_nnbd/lib/_internal/vm/bin/process_patch.dart b/sdk_nnbd/lib/_internal/vm/bin/process_patch.dart index 49592e7ce8c..0b80ac8f6c0 100644 --- a/sdk_nnbd/lib/_internal/vm/bin/process_patch.dart +++ b/sdk_nnbd/lib/_internal/vm/bin/process_patch.dart @@ -510,7 +510,7 @@ class _ProcessImpl extends _ProcessImplNativeWrapper implements Process { _stdout ?? (throw StateError("Process is detached")); Stream> get stderr => - _stdout ?? (throw StateError("Process is detached")); + _stderr ?? (throw StateError("Process is detached")); IOSink get stdin => _stdin ?? (throw StateError("Process is detached")); diff --git a/tests/standalone_2/io/process_detached_test.dart b/tests/standalone_2/io/process_detached_test.dart index b6b59e76e4e..528d0cfff72 100644 --- a/tests/standalone_2/io/process_detached_test.dart +++ b/tests/standalone_2/io/process_detached_test.dart @@ -12,8 +12,6 @@ import 'dart:io'; import "package:async_helper/async_helper.dart"; import "package:expect/expect.dart"; -import "process_test_util.dart"; - void test() { asyncStart(); var script = @@ -27,10 +25,10 @@ void test() { future.then((process) { Expect.isNotNull(process.pid); Expect.isTrue(process.pid is int); - Expect.isNull(process.exitCode); - Expect.isNull(process.stderr); - Expect.isNull(process.stdin); - Expect.isNull(process.stdout); + Expect.throwsStateError(() => process.exitCode); + Expect.throwsStateError(() => process.stderr); + Expect.throwsStateError(() => process.stdin); + Expect.throwsStateError(() => process.stdout); Expect.isTrue(process.kill()); }).whenComplete(() { asyncEnd(); @@ -47,7 +45,7 @@ void testWithStdio() { future.then((process) { Expect.isNotNull(process.pid); Expect.isTrue(process.pid is int); - Expect.isNull(process.exitCode); + Expect.throwsStateError(() => process.exitCode); var message = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; process.stdin.add(message); process.stdin.flush().then((_) => process.stdin.close());