Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to adapt to vscode and fix a deadlock #2351

Merged
merged 16 commits into from
Sep 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
.packages
.pub
.settings/
.vscode/
build/
doc/
lcov.info
Expand Down
9 changes: 6 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ sudo: false
dart:
- stable
- dev

os:
- linux

Expand All @@ -13,9 +12,13 @@ matrix:
- env: DARTDOC_BOT=main
os: osx
dart: dev
allow_failures:
# See https://github.com/dart-lang/dartdoc/issues/2302.
exclude:
- env: DARTDOC_BOT=flutter
# Do not try to run flutter against the "stable" sdk,
# it is unlikely to work and produces uninteresting
# results.
dart: stable
allow_failures:
# Some packages at HEAD require 2.10.0-dev.
- env: DARTDOC_BOT=sdk-analyzer
dart: stable
Expand Down
2 changes: 1 addition & 1 deletion analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ analyzer:
- 'lib/templates/*.html'
- 'pub.dartlang.org/**'
- 'testing/**'
- 'testing/test_package_flutter_plugin/**'
- 'testing/flutter_packages/test_package_flutter_plugin/**'
- 'testing/test_package_export_error/**'
linter:
rules:
Expand Down
2 changes: 1 addition & 1 deletion analysis_options_presubmit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ analyzer:
- 'lib/templates/*.html'
- 'pub.dartlang.org/**'
- 'testing/**'
- 'testing/test_package_flutter_plugin/**'
- 'testing/flutter_packages/test_package_flutter_plugin/**'
- 'testing/test_package_export_error/**'
linter:
rules:
Expand Down
14 changes: 9 additions & 5 deletions lib/src/io_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -123,21 +123,25 @@ class MultiFutureTracker<T> {
/// Wait until fewer or equal to this many Futures are outstanding.
Future<void> _waitUntil(int max) async {
while (_trackedFutures.length > max) {
await Future.any(_trackedFutures);
try {
await Future.any(_trackedFutures);
// The empty catch is OK because we're just counting future completions
// and we don't care about errors (the original caller of
// addFutureToClosure is responsible for those).
} catch (e) {} // ignore: empty_catches
}
}

/// Generates a [Future] from the given closure and adds it to the queue,
/// once the queue is sufficiently empty. The returned future completes
/// when the generated [Future] has been added to the queue.
Future<void> addFutureFromClosure(Future<T> Function() closure) async {
while (_trackedFutures.length > parallel - 1) {
await Future.any(_trackedFutures);
}
await _waitUntil(parallel - 1);
Future<void> future = closure();
_trackedFutures.add(future);
// ignore: unawaited_futures
future.then((f) => _trackedFutures.remove(future));
future.then((f) => _trackedFutures.remove(future),
onError: (s, e) => _trackedFutures.remove(future));
}

/// Wait until all futures added so far have completed.
Expand Down
5 changes: 3 additions & 2 deletions test/end2end/dartdoc_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ Uri get _currentFileUri =>
(reflect(main) as ClosureMirror).function.location.sourceUri;
String get _testPackagePath =>
path.fromUri(_currentFileUri.resolve('../../testing/test_package'));
String get _testPackageFlutterPluginPath => path.fromUri(
_currentFileUri.resolve('../../testing/test_package_flutter_plugin'));
String get _testPackageFlutterPluginPath => path.fromUri(_currentFileUri
.resolve('../../testing/flutter_packages/test_package_flutter_plugin'));

void main() {
group('Invoking command-line dartdoc', () {
Expand Down Expand Up @@ -122,6 +122,7 @@ void main() {

test('help prints command line args', () async {
var outputLines = <String>[];
print('dartdocPath: $dartdocPath');
await subprocessLauncher.runStreamed(
Platform.resolvedExecutable, [dartdocPath, '--help'],
perLine: outputLines.add);
Expand Down
73 changes: 73 additions & 0 deletions test/io_utils_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,77 @@ void main() {
expect(getFileNameFor('dartdoc.generator'), 'dartdoc-generator.html');
});
});

group('MultiFutureTracker', () {
/// A special test designed to check shared [MultiFutureTracker]
/// behavior when exceptions occur after a delay in the passed closures to
/// [MultiFutureTracker.addFutureFromClosure].
test('no deadlock when delayed exceptions fire in closures', () async {
var sharedTracker = MultiFutureTracker(2);
var t =
Future.delayed(Duration(milliseconds: 10), () => throw Exception());
await sharedTracker.addFutureFromClosure(() => t);
expect(t, throwsA(const TypeMatcher<Exception>()));
var u =
Future.delayed(Duration(milliseconds: 10), () => throw Exception());
await sharedTracker.addFutureFromClosure(() => u);
expect(u, throwsA(const TypeMatcher<Exception>()));
var v =
Future.delayed(Duration(milliseconds: 10), () => throw Exception());
await sharedTracker.addFutureFromClosure(() => v);
expect(v, throwsA(const TypeMatcher<Exception>()));

/// We deadlock here if the exception is not handled properly.
await sharedTracker.wait();
});

test('basic sequential processing works with no deadlock', () async {
var completed = <int>{};
var tracker = MultiFutureTracker(1);
await tracker.addFutureFromClosure(() async => completed.add(1));
await tracker.addFutureFromClosure(() async => completed.add(2));
await tracker.addFutureFromClosure(() async => completed.add(3));
await tracker.wait();
expect(completed.length, equals(3));
});

test('basic sequential processing works on exceptions', () async {
var completed = <int>{};
var tracker = MultiFutureTracker(1);
await tracker.addFutureFromClosure(() async => completed.add(0));
await tracker.addFutureFromClosure(() async => throw Exception());
await tracker.addFutureFromClosure(() async => throw Exception());
await tracker.addFutureFromClosure(() async => completed.add(3));
await tracker.wait();
expect(completed.length, equals(2));
});

/// Verify that if there are more exceptions than the maximum number
/// of in-flight [Future]s that there is no deadlock.
test('basic parallel processing works with no deadlock', () async {
var completed = <int>{};
var tracker = MultiFutureTracker(10);
for (var i = 0; i < 100; i++) {
await tracker.addFutureFromClosure(() async => completed.add(i));
}
await tracker.wait();
expect(completed.length, equals(100));
});

test('basic parallel processing works on exceptions', () async {
var completed = <int>{};
var tracker = MultiFutureTracker(10);
for (var i = 0; i < 50; i++) {
await tracker.addFutureFromClosure(() async => completed.add(i));
}
for (var i = 50; i < 65; i++) {
await tracker.addFutureFromClosure(() async => throw Exception());
}
for (var i = 65; i < 100; i++) {
await tracker.addFutureFromClosure(() async => completed.add(i));
}
await tracker.wait();
expect(completed.length, equals(85));
});
});
}
6 changes: 3 additions & 3 deletions tool/grind.dart
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ Directory get testPackage =>
Directory get testPackageExperiments =>
Directory(path.joinAll(['testing', 'test_package_experiments']));

Directory get pluginPackage =>
Directory(path.joinAll(['testing', 'test_package_flutter_plugin']));
Directory get pluginPackage => Directory(path
.joinAll(['testing', 'flutter_packages', 'test_package_flutter_plugin']));

Directory _testPackageDocsDir;

Expand Down Expand Up @@ -278,7 +278,7 @@ void dartfmt() async {
void presubmit() => null;

@Task('Run long tests, self-test dartdoc, and run the publish test')
@Depends(presubmit, test, testDartdoc)
@Depends(presubmit, longTest, testDartdoc)
void buildbot() => null;

@Task('Generate docs for the Dart SDK')
Expand Down
4 changes: 3 additions & 1 deletion tool/travis.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ set -ex
export PATH="$PATH":"~/.pub-cache/bin"
DART_VERSION=`dart --version 2>&1 | awk '{print $4}'`
# Do not run coverage on non-dev builds or non-Linux platforms.
if ! echo "${DART_VERSION}" | grep -q dev || ! uname | grep -q Linux ; then
# Disable coverage on dev builds and switch to stable until
# dart-lang/sdk#43487 is fixed.
if ! echo "${DART_VERSION}" | grep -q stable || ! uname | grep -q Linux ; then
unset COVERAGE_TOKEN
fi

Expand Down