Add consistent implementations for close. (#851)

Author: brianquinlan

pkgs/cronet_http/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.1.1
+
+* `CronetClient` throws an exception if `send` is called after `close`.
+
 ## 0.1.0
 
 * Add a CronetClient that accepts a `Future<CronetEngine>`.

pkgs/cronet_http/example/integration_test/client_test.dart

@@ -22,6 +22,7 @@ void testClientConformance(CronetClient Function() clientFactory) {
   testRedirect(client);
   testCompressedResponseBody(client);
   testMultipleClients(clientFactory);
+  testClose(clientFactory);
 }
 
 Future<void> testConformance() async {

pkgs/cronet_http/lib/cronet_client.dart

@@ -134,6 +134,7 @@ class CronetEngine {
 class CronetClient extends BaseClient {
   CronetEngine? _engine;
   Future<CronetEngine>? _engineFuture;
+  bool _isClosed = false;
 
   /// Indicates that [_engine] was constructed as an implementation detail for
   /// this [CronetClient] (i.e. was not provided as a constructor argument) and
@@ -170,13 +171,19 @@ class CronetClient extends BaseClient {
 
   @override
   void close() {
-    if (_ownedEngine) {
+    if (!_isClosed && _ownedEngine) {
       _engine?.close();
     }
+    _isClosed = true;
   }
 
   @override
   Future<StreamedResponse> send(BaseRequest request) async {
+    if (_isClosed) {
+      throw ClientException(
+          'HTTP request failed. Client is already closed.', request.url);
+    }
+
     if (_engine == null) {
       // Create the future here rather than in the [fromCronetEngineFuture]
       // factory so that [close] does not have to await the future just to

pkgs/cronet_http/pubspec.yaml

@@ -1,7 +1,7 @@
 name: cronet_http
 description: >
   An Android Flutter plugin that provides access to the Cronet HTTP client.
-version: 0.1.0
+version: 0.1.1
 repository: https://github.com/dart-lang/http/tree/master/pkgs/cronet_http
 
 environment:

pkgs/cupertino_http/CHANGELOG.md

@@ -2,6 +2,7 @@
 
 * Fix a bug where the images in the example would be loaded using `dart:io`
   `HttpClient`.
+* `CupertinoClient` throws an exception if `send` is called after `close`.
 
 ## 0.0.10
 

pkgs/cupertino_http/lib/cupertino_client.dart

@@ -52,9 +52,9 @@ class _TaskTracker {
 class CupertinoClient extends BaseClient {
   static final Map<URLSessionTask, _TaskTracker> _tasks = {};
 
-  URLSession _urlSession;
+  URLSession? _urlSession;
 
-  CupertinoClient._(this._urlSession);
+  CupertinoClient._(URLSession urlSession) : _urlSession = urlSession;
 
   String? _findReasonPhrase(int statusCode) {
     switch (statusCode) {
@@ -205,6 +205,11 @@ class CupertinoClient extends BaseClient {
     return CupertinoClient._(session);
   }
 
+  @override
+  void close() {
+    _urlSession = null;
+  }
+
   @override
   Future<StreamedResponse> send(BaseRequest request) async {
     // The expected sucess case flow (without redirects) is:
@@ -219,6 +224,12 @@ class CupertinoClient extends BaseClient {
     //    StreamController that controls the Stream<UInt8List>
     // 7. _onComplete is called after all the data is read and closes the
     //    StreamController
+    if (_urlSession == null) {
+      throw ClientException(
+          'HTTP request failed. Client is already closed.', request.url);
+    }
+    final urlSession = _urlSession!;
+
     final stream = request.finalize();
 
     final bytes = await stream.toBytes();
@@ -231,7 +242,7 @@ class CupertinoClient extends BaseClient {
     // This will preserve Apple default headers - is that what we want?
     request.headers.forEach(urlRequest.setValueForHttpHeaderField);
 
-    final task = _urlSession.dataTaskWithRequest(urlRequest);
+    final task = urlSession.dataTaskWithRequest(urlRequest);
     final taskTracker = _TaskTracker(request);
     _tasks[task] = taskTracker;
     task.resume();

pkgs/cupertino_http/pubspec.yaml

@@ -2,7 +2,7 @@ name: cupertino_http
 description: >
   A macOS/iOS Flutter plugin that provides access to the Foundation URL
   Loading System.
-version: 0.0.10
+version: 0.0.11
 repository: https://github.com/dart-lang/http/tree/master/pkgs/cupertino_http
 
 environment:

pkgs/http/CHANGELOG.md

@@ -1,5 +1,7 @@
 ## 0.13.6-dev
 
+* `BrowserClient` throws an exception if `send` is called after `close`.
+
 ## 0.13.5
 
 * Allow async callbacks in RetryClient.

pkgs/http/lib/src/browser_client.dart

@@ -37,9 +37,15 @@ class BrowserClient extends BaseClient {
   /// Defaults to `false`.
   bool withCredentials = false;
 
+  bool _isClosed = false;
+
   /// Sends an HTTP request and asynchronously returns the response.
   @override
   Future<StreamedResponse> send(BaseRequest request) async {
+    if (_isClosed) {
+      throw ClientException(
+          'HTTP request failed. Client is already closed.', request.url);
+    }
     var bytes = await request.finalize().toBytes();
     var xhr = HttpRequest();
     _xhrs.add(xhr);
@@ -83,8 +89,10 @@ class BrowserClient extends BaseClient {
   /// This terminates all active requests.
   @override
   void close() {
+    _isClosed = true;
     for (var xhr in _xhrs) {
       xhr.abort();
     }
+    _xhrs.clear();
   }
 }

pkgs/http/lib/src/client.dart

@@ -145,6 +145,10 @@ abstract class Client {
   ///
   /// It's important to close each client when it's done being used; failing to
   /// do so can cause the Dart process to hang.
+  ///
+  /// Once [close] is called, no other methods should be called. If [close] is
+  /// called while other asynchronous methods are running, the behavior is
+  /// undefined.
   void close();
 }