Revert "Revert r38549, r38552, and r38557."
This reverts commit 9bf6a8ec011fb52fa259109760f649567d514813.
R=nweiz@google.com
Review URL: https://codereview.chromium.org//1152413002
diff --git a/CHANGELOG.md b/CHANGELOG.md
new file mode 100644
index 0000000..e8dee50
--- /dev/null
+++ b/CHANGELOG.md
@@ -0,0 +1,3 @@
+## 1.0.1
+
+* A `TimeoutException` is now correctly thrown if the pool detects a deadlock.
diff --git a/lib/pool.dart b/lib/pool.dart
index 36a29c8..61482e3 100644
--- a/lib/pool.dart
+++ b/lib/pool.dart
@@ -40,14 +40,14 @@
Timer _timer;
/// The amount of time to wait before timing out the pending resources.
- Duration _timeout;
+ final Duration _timeout;
/// Creates a new pool with the given limit on how many resources may be
/// allocated at once.
///
/// If [timeout] is passed, then if that much time passes without any activity
- /// all pending [request] futures will throw an exception. This is indented
- /// to avoid deadlocks.
+ /// all pending [request] futures will throw a [TimeoutException]. This is
+ /// intended to avoid deadlocks.
Pool(this._maxAllocatedResources, {Duration timeout})
: _timeout = timeout;
@@ -106,8 +106,11 @@
/// emit exceptions.
void _onTimeout() {
for (var completer in _requestedResources) {
- completer.completeError("Pool deadlock: all resources have been "
- "allocated for too long.", new Chain.current());
+ completer.completeError(
+ new TimeoutException("Pool deadlock: all resources have been "
+ "allocated for too long.",
+ _timeout),
+ new Chain.current());
}
_requestedResources.clear();
_timer = null;
diff --git a/pubspec.yaml b/pubspec.yaml
index b6a0ff2..a7d23a5 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,9 +1,10 @@
name: pool
-version: 1.0.0
+version: 1.0.1
author: Dart Team <misc@dartlang.org>
description: A class for managing a finite pool of resources.
homepage: http://github.com/dart-lang/pool
dependencies:
stack_trace: ">=0.9.2 <2.0.0"
dev_dependencies:
+ fake_async: ">=0.1.0 <0.2.0"
unittest: ">=0.11.0 <0.12.0"
diff --git a/test/pool_test.dart b/test/pool_test.dart
index bfeb876..bb9e607 100644
--- a/test/pool_test.dart
+++ b/test/pool_test.dart
@@ -4,6 +4,7 @@
import 'dart:async';
+import 'package:fake_async/fake_async.dart';
import 'package:pool/pool.dart';
import 'package:stack_trace/stack_trace.dart';
import 'package:unittest/unittest.dart';
@@ -19,30 +20,36 @@
});
test("resources block past the limit", () {
- var pool = new Pool(50);
- var requests = [];
- for (var i = 0; i < 50; i++) {
- expect(pool.request(), completes);
- }
- expect(pool.request(), doesNotComplete);
+ new FakeAsync().run((async) {
+ var pool = new Pool(50);
+ var requests = [];
+ for (var i = 0; i < 50; i++) {
+ expect(pool.request(), completes);
+ }
+ expect(pool.request(), doesNotComplete);
+
+ async.elapse(new Duration(seconds: 1));
+ });
});
test("a blocked resource is allocated when another is released", () {
- var pool = new Pool(50);
- var requests = [];
- for (var i = 0; i < 49; i++) {
- expect(pool.request(), completes);
- }
+ new FakeAsync().run((async) {
+ var pool = new Pool(50);
+ var requests = [];
+ for (var i = 0; i < 49; i++) {
+ expect(pool.request(), completes);
+ }
- return pool.request().then((lastAllocatedResource) {
- var blockedResource = pool.request();
+ pool.request().then((lastAllocatedResource) {
+ // This will only complete once [lastAllocatedResource] is released.
+ expect(pool.request(), completes);
- return pumpEventQueue().then((_) {
- lastAllocatedResource.release();
- return pumpEventQueue();
- }).then((_) {
- expect(blockedResource, completes);
+ new Future.delayed(new Duration(microseconds: 1)).then((_) {
+ lastAllocatedResource.release();
+ });
});
+
+ async.elapse(new Duration(seconds: 1));
});
});
});
@@ -57,63 +64,113 @@
});
test("blocks the callback past the limit", () {
- var pool = new Pool(50);
- var requests = [];
- for (var i = 0; i < 50; i++) {
- pool.withResource(expectAsync(() => new Completer().future));
- }
- pool.withResource(expectNoAsync());
+ new FakeAsync().run((async) {
+ var pool = new Pool(50);
+ var requests = [];
+ for (var i = 0; i < 50; i++) {
+ pool.withResource(expectAsync(() => new Completer().future));
+ }
+ pool.withResource(expectNoAsync());
+
+ async.elapse(new Duration(seconds: 1));
+ });
});
test("a blocked resource is allocated when another is released", () {
- var pool = new Pool(50);
- var requests = [];
- for (var i = 0; i < 49; i++) {
- pool.withResource(expectAsync(() => new Completer().future));
- }
+ new FakeAsync().run((async) {
+ var pool = new Pool(50);
+ var requests = [];
+ for (var i = 0; i < 49; i++) {
+ pool.withResource(expectAsync(() => new Completer().future));
+ }
- var completer = new Completer();
- var lastAllocatedResource = pool.withResource(() => completer.future);
- var blockedResourceAllocated = false;
- var blockedResource = pool.withResource(() {
- blockedResourceAllocated = true;
- });
+ var completer = new Completer();
+ var lastAllocatedResource = pool.withResource(() => completer.future);
+ var blockedResourceAllocated = false;
+ var blockedResource = pool.withResource(() {
+ blockedResourceAllocated = true;
+ });
- return pumpEventQueue().then((_) {
- expect(blockedResourceAllocated, isFalse);
- completer.complete();
- return pumpEventQueue();
- }).then((_) {
- expect(blockedResourceAllocated, isTrue);
+ new Future.delayed(new Duration(microseconds: 1)).then((_) {
+ expect(blockedResourceAllocated, isFalse);
+ completer.complete();
+ return new Future.delayed(new Duration(microseconds: 1));
+ }).then((_) {
+ expect(blockedResourceAllocated, isTrue);
+ });
+
+ async.elapse(new Duration(seconds: 1));
});
});
});
- // TODO(nweiz): Test timeouts when it's easier to use third-party packages.
- // See r38552.
-}
+ group("with a timeout", () {
+ test("doesn't time out if there are no pending requests", () {
+ new FakeAsync().run((async) {
+ var pool = new Pool(50, timeout: new Duration(seconds: 5));
+ for (var i = 0; i < 50; i++) {
+ expect(pool.request(), completes);
+ }
-/// Returns a [Future] that completes after pumping the event queue [times]
-/// times. By default, this should pump the event queue enough times to allow
-/// any code to run, as long as it's not waiting on some external event.
-Future pumpEventQueue([int times = 20]) {
- if (times == 0) return new Future.value();
- // We use a delayed future to allow microtask events to finish. The
- // Future.value or Future() constructors use scheduleMicrotask themselves and
- // would therefore not wait for microtask callbacks that are scheduled after
- // invoking this method.
- return new Future.delayed(Duration.ZERO, () => pumpEventQueue(times - 1));
+ async.elapse(new Duration(seconds: 6));
+ });
+ });
+
+ test("resets the timer if a resource is returned", () {
+ new FakeAsync().run((async) {
+ var pool = new Pool(50, timeout: new Duration(seconds: 5));
+ for (var i = 0; i < 49; i++) {
+ expect(pool.request(), completes);
+ }
+
+ pool.request().then((lastAllocatedResource) {
+ // This will only complete once [lastAllocatedResource] is released.
+ expect(pool.request(), completes);
+
+ new Future.delayed(new Duration(seconds: 3)).then((_) {
+ lastAllocatedResource.release();
+ expect(pool.request(), doesNotComplete);
+ });
+ });
+
+ async.elapse(new Duration(seconds: 6));
+ });
+ });
+
+ test("resets the timer if a resource is requested", () {
+ new FakeAsync().run((async) {
+ var pool = new Pool(50, timeout: new Duration(seconds: 5));
+ for (var i = 0; i < 50; i++) {
+ expect(pool.request(), completes);
+ }
+ expect(pool.request(), doesNotComplete);
+
+ new Future.delayed(new Duration(seconds: 3)).then((_) {
+ expect(pool.request(), doesNotComplete);
+ });
+
+ async.elapse(new Duration(seconds: 6));
+ });
+ });
+
+ test("times out if nothing happens", () {
+ new FakeAsync().run((async) {
+ var pool = new Pool(50, timeout: new Duration(seconds: 5));
+ for (var i = 0; i < 50; i++) {
+ expect(pool.request(), completes);
+ }
+ expect(pool.request(), throwsA(new isInstanceOf<TimeoutException>()));
+
+ async.elapse(new Duration(seconds: 6));
+ });
+ });
+ });
}
/// Returns a function that will cause the test to fail if it's called.
///
-/// This won't let the test complete until it's confident that the function
-/// won't be called.
+/// This should only be called within a [FakeAsync.run] zone.
Function expectNoAsync() {
- // Make sure the test lasts long enough for the function to get called if it's
- // going to get called.
- expect(pumpEventQueue(), completes);
-
var stack = new Trace.current(1);
return () => handleExternalError(
new TestFailure("Expected function not to be called."), "",
@@ -122,13 +179,9 @@
/// A matcher for Futures that asserts that they don't complete.
///
-/// This won't let the test complete until it's confident that the function
-/// won't be called.
+/// This should only be called within a [FakeAsync.run] zone.
Matcher get doesNotComplete => predicate((future) {
expect(future, new isInstanceOf<Future>('Future'));
- // Make sure the test lasts long enough for the function to get called if it's
- // going to get called.
- expect(pumpEventQueue(), completes);
var stack = new Trace.current(1);
future.then((_) => handleExternalError(
