Skip to content

Commit 6a04a88

Browse files
authored
Changes to adapt to vscode and fix a deadlock (#2351)
* testing * test_package_flutter_plugin move out of the way * Revert accidental change * Add analysis options changes * Fix deadlock blocking tests * print debugging * more prints, don't duplicate model_test * Fix presubmit error * more debugging * switch to raw, maybe dart run bug fixed? * return to dev version * Unconditionally disable coverage * Fix deadlock on exceptions and strip extraneous stuff. * Fix tests to properly reproduce race * travis setting tweaks * Disallow flutter failures and fix plugin directory
1 parent f5985ce commit 6a04a88

File tree

13 files changed

+100
-16
lines changed

13 files changed

+100
-16
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
.packages
77
.pub
88
.settings/
9+
.vscode/
910
build/
1011
doc/
1112
lcov.info

.travis.yml

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ sudo: false
33
dart:
44
- stable
55
- dev
6-
76
os:
87
- linux
98

@@ -13,9 +12,13 @@ matrix:
1312
- env: DARTDOC_BOT=main
1413
os: osx
1514
dart: dev
16-
allow_failures:
17-
# See https://github.com/dart-lang/dartdoc/issues/2302.
15+
exclude:
1816
- env: DARTDOC_BOT=flutter
17+
# Do not try to run flutter against the "stable" sdk,
18+
# it is unlikely to work and produces uninteresting
19+
# results.
20+
dart: stable
21+
allow_failures:
1922
# Some packages at HEAD require 2.10.0-dev.
2023
- env: DARTDOC_BOT=sdk-analyzer
2124
dart: stable

analysis_options.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ analyzer:
1212
- 'lib/templates/*.html'
1313
- 'pub.dartlang.org/**'
1414
- 'testing/**'
15-
- 'testing/test_package_flutter_plugin/**'
15+
- 'testing/flutter_packages/test_package_flutter_plugin/**'
1616
- 'testing/test_package_export_error/**'
1717
linter:
1818
rules:

analysis_options_presubmit.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ analyzer:
1515
- 'lib/templates/*.html'
1616
- 'pub.dartlang.org/**'
1717
- 'testing/**'
18-
- 'testing/test_package_flutter_plugin/**'
18+
- 'testing/flutter_packages/test_package_flutter_plugin/**'
1919
- 'testing/test_package_export_error/**'
2020
linter:
2121
rules:

lib/src/io_utils.dart

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,21 +123,25 @@ class MultiFutureTracker<T> {
123123
/// Wait until fewer or equal to this many Futures are outstanding.
124124
Future<void> _waitUntil(int max) async {
125125
while (_trackedFutures.length > max) {
126-
await Future.any(_trackedFutures);
126+
try {
127+
await Future.any(_trackedFutures);
128+
// The empty catch is OK because we're just counting future completions
129+
// and we don't care about errors (the original caller of
130+
// addFutureToClosure is responsible for those).
131+
} catch (e) {} // ignore: empty_catches
127132
}
128133
}
129134

130135
/// Generates a [Future] from the given closure and adds it to the queue,
131136
/// once the queue is sufficiently empty. The returned future completes
132137
/// when the generated [Future] has been added to the queue.
133138
Future<void> addFutureFromClosure(Future<T> Function() closure) async {
134-
while (_trackedFutures.length > parallel - 1) {
135-
await Future.any(_trackedFutures);
136-
}
139+
await _waitUntil(parallel - 1);
137140
Future<void> future = closure();
138141
_trackedFutures.add(future);
139142
// ignore: unawaited_futures
140-
future.then((f) => _trackedFutures.remove(future));
143+
future.then((f) => _trackedFutures.remove(future),
144+
onError: (s, e) => _trackedFutures.remove(future));
141145
}
142146

143147
/// Wait until all futures added so far have completed.

test/end2end/dartdoc_integration_test.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ Uri get _currentFileUri =>
1919
(reflect(main) as ClosureMirror).function.location.sourceUri;
2020
String get _testPackagePath =>
2121
path.fromUri(_currentFileUri.resolve('../../testing/test_package'));
22-
String get _testPackageFlutterPluginPath => path.fromUri(
23-
_currentFileUri.resolve('../../testing/test_package_flutter_plugin'));
22+
String get _testPackageFlutterPluginPath => path.fromUri(_currentFileUri
23+
.resolve('../../testing/flutter_packages/test_package_flutter_plugin'));
2424

2525
void main() {
2626
group('Invoking command-line dartdoc', () {
@@ -122,6 +122,7 @@ void main() {
122122

123123
test('help prints command line args', () async {
124124
var outputLines = <String>[];
125+
print('dartdocPath: $dartdocPath');
125126
await subprocessLauncher.runStreamed(
126127
Platform.resolvedExecutable, [dartdocPath, '--help'],
127128
perLine: outputLines.add);

test/io_utils_test.dart

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,77 @@ void main() {
1717
expect(getFileNameFor('dartdoc.generator'), 'dartdoc-generator.html');
1818
});
1919
});
20+
21+
group('MultiFutureTracker', () {
22+
/// A special test designed to check shared [MultiFutureTracker]
23+
/// behavior when exceptions occur after a delay in the passed closures to
24+
/// [MultiFutureTracker.addFutureFromClosure].
25+
test('no deadlock when delayed exceptions fire in closures', () async {
26+
var sharedTracker = MultiFutureTracker(2);
27+
var t =
28+
Future.delayed(Duration(milliseconds: 10), () => throw Exception());
29+
await sharedTracker.addFutureFromClosure(() => t);
30+
expect(t, throwsA(const TypeMatcher<Exception>()));
31+
var u =
32+
Future.delayed(Duration(milliseconds: 10), () => throw Exception());
33+
await sharedTracker.addFutureFromClosure(() => u);
34+
expect(u, throwsA(const TypeMatcher<Exception>()));
35+
var v =
36+
Future.delayed(Duration(milliseconds: 10), () => throw Exception());
37+
await sharedTracker.addFutureFromClosure(() => v);
38+
expect(v, throwsA(const TypeMatcher<Exception>()));
39+
40+
/// We deadlock here if the exception is not handled properly.
41+
await sharedTracker.wait();
42+
});
43+
44+
test('basic sequential processing works with no deadlock', () async {
45+
var completed = <int>{};
46+
var tracker = MultiFutureTracker(1);
47+
await tracker.addFutureFromClosure(() async => completed.add(1));
48+
await tracker.addFutureFromClosure(() async => completed.add(2));
49+
await tracker.addFutureFromClosure(() async => completed.add(3));
50+
await tracker.wait();
51+
expect(completed.length, equals(3));
52+
});
53+
54+
test('basic sequential processing works on exceptions', () async {
55+
var completed = <int>{};
56+
var tracker = MultiFutureTracker(1);
57+
await tracker.addFutureFromClosure(() async => completed.add(0));
58+
await tracker.addFutureFromClosure(() async => throw Exception());
59+
await tracker.addFutureFromClosure(() async => throw Exception());
60+
await tracker.addFutureFromClosure(() async => completed.add(3));
61+
await tracker.wait();
62+
expect(completed.length, equals(2));
63+
});
64+
65+
/// Verify that if there are more exceptions than the maximum number
66+
/// of in-flight [Future]s that there is no deadlock.
67+
test('basic parallel processing works with no deadlock', () async {
68+
var completed = <int>{};
69+
var tracker = MultiFutureTracker(10);
70+
for (var i = 0; i < 100; i++) {
71+
await tracker.addFutureFromClosure(() async => completed.add(i));
72+
}
73+
await tracker.wait();
74+
expect(completed.length, equals(100));
75+
});
76+
77+
test('basic parallel processing works on exceptions', () async {
78+
var completed = <int>{};
79+
var tracker = MultiFutureTracker(10);
80+
for (var i = 0; i < 50; i++) {
81+
await tracker.addFutureFromClosure(() async => completed.add(i));
82+
}
83+
for (var i = 50; i < 65; i++) {
84+
await tracker.addFutureFromClosure(() async => throw Exception());
85+
}
86+
for (var i = 65; i < 100; i++) {
87+
await tracker.addFutureFromClosure(() async => completed.add(i));
88+
}
89+
await tracker.wait();
90+
expect(completed.length, equals(85));
91+
});
92+
});
2093
}

0 commit comments

Comments
 (0)