Don't use async/await for Pool.withResource().
The async gap introduces a potential race condition.
Closes #3
R=rnystrom@google.com
Review URL: https://codereview.chromium.org//1853273002 .
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9ad86d0..70f914b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 1.2.3
+
+* Fix a bug in which `Pool.withResource()` could throw a `StateError` when
+ called immediately before closing the pool.
+
## 1.2.2
* Fix strong mode warnings and add generic method annotations.
diff --git a/lib/pool.dart b/lib/pool.dart
index f7bfd82..deb5333 100644
--- a/lib/pool.dart
+++ b/lib/pool.dart
@@ -106,18 +106,19 @@
/// Future.
///
/// The return value of [callback] is piped to the returned Future.
- Future/*<T>*/ withResource/*<T>*/(/*=T*/ callback()) async {
+ Future/*<T>*/ withResource/*<T>*/(/*=T*/ callback()) {
if (isClosed) {
throw new StateError(
"withResource() may not be called on a closed Pool.");
}
- var resource = await request();
- try {
- return await callback();
- } finally {
- resource.release();
- }
+ // We can't use async/await here because we need to start the request
+ // synchronously in case the pool is closed immediately afterwards. Async
+ // functions have an asynchronous gap between calling and running the body,
+ // and [close] could be called during that gap. See #3.
+ return request().then((resource) {
+ return new Future.sync(callback).whenComplete(resource.release);
+ });
}
/// Closes the pool so that no more resources are requested.
diff --git a/pubspec.yaml b/pubspec.yaml
index 314ac04..58321cf 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,5 +1,5 @@
name: pool
-version: 1.2.2
+version: 1.2.3
author: Dart Team <misc@dartlang.org>
description: A class for managing a finite pool of resources.
homepage: https://github.com/dart-lang/pool
diff --git a/test/pool_test.dart b/test/pool_test.dart
index 91e6833..bd44cb2 100644
--- a/test/pool_test.dart
+++ b/test/pool_test.dart
@@ -96,6 +96,13 @@
async.elapse(new Duration(seconds: 1));
});
});
+
+ // Regression test for #3.
+ test("can be called immediately before close()", () async {
+ var pool = new Pool(1);
+ pool.withResource(expectAsync(() {}));
+ await pool.close();
+ });
});
group("with a timeout", () {
@@ -276,7 +283,7 @@
test("disallows request() and withResource()", () {
var pool = new Pool(1)..close();
expect(pool.request, throwsStateError);
- expect(pool.withResource(() {}), throwsStateError);
+ expect(() => pool.withResource(() {}), throwsStateError);
});
test("pending requests are fulfilled", () async {
