Unverified Commit 76ad8647 authored by gaaclarke's avatar gaaclarke Committed by GitHub

Added timeout for closing devfs sync http connections. (#66152)

parent 21ad3e7a
......@@ -239,7 +239,11 @@ class _DevFSHttpWriter implements DevFSWriter {
final String fsName;
final Uri httpAddress;
static const int kMaxInFlight = 6;
// 3 was chosen to try to limit the varience in the time it takes to execute
// `await request.close()` since there is a known bug in Dart where it doesn't
// always return a status code in response to a PUT request:
// https://github.com/dart-lang/sdk/issues/43525.
static const int kMaxInFlight = 3;
int _inFlight = 0;
Map<Uri, DevFSContent> _outstanding;
......@@ -289,13 +293,29 @@ class _DevFSHttpWriter implements DevFSWriter {
_osUtils,
);
await request.addStream(contents);
final HttpClientResponse response = await request.close();
response.listen((_) {},
onError: (dynamic error) {
_logger.printTrace('error: $error');
},
cancelOnError: true,
);
// The contents has already been streamed, closing the request should
// not take long but we are experiencing hangs with it, see #63869.
//
// Once the bug in Dart is solved we can remove the timeout
// (https://github.com/dart-lang/sdk/issues/43525). The timeout was
// chosen to be inflated based on the max observed time when running the
// tests in "Google Tests".
try {
final HttpClientResponse response = await request.close().timeout(
const Duration(milliseconds: 10000));
response.listen((_) {},
onError: (dynamic error) {
_logger.printTrace('error: $error');
},
cancelOnError: true,
);
} on TimeoutException {
request.abort();
// This should throw "HttpException: Request has been aborted".
await request.done;
// Just to be safe we rethrow the TimeoutException.
rethrow;
}
break;
} on Exception catch (error, trace) {
if (!_completer.isCompleted) {
......
......@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'dart:async';
import 'dart:convert';
import 'package:file/file.dart';
......@@ -258,7 +259,7 @@ void main() {
expect(devFS.lastCompiled, isNot(previousCompile));
});
testWithoutContext('DevFS uses provided DevFSWriter instead of default HTTP writer', () async {
testWithoutContext('DevFS uses provided DevFSWriter instead of default HTTP writer', () async {
final FileSystem fileSystem = MemoryFileSystem.test();
final FakeDevFSWriter writer = FakeDevFSWriter();
final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
......@@ -332,6 +333,84 @@ void main() {
Uri.parse('goodbye'): DevFSFileContent(file),
}, Uri.parse('/foo/bar/devfs/')), throwsA(isA<DevFSException>()));
});
testWithoutContext('test handles request closure hangs', () async {
final FileSystem fileSystem = MemoryFileSystem.test();
final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
requests: <VmServiceExpectation>[createDevFSRequest],
);
final HttpClient httpClient = MockHttpClient();
final MockHttpClientRequest httpRequest = MockHttpClientRequest();
when(httpRequest.headers).thenReturn(MockHttpHeaders());
when(httpClient.putUrl(any)).thenAnswer((Invocation invocation) {
return Future<HttpClientRequest>.value(httpRequest);
});
int closeCount = 0;
final Completer<MockHttpClientResponse> hanger = Completer<MockHttpClientResponse>();
final Completer<MockHttpClientResponse> succeeder = Completer<MockHttpClientResponse>();
final List<Completer<MockHttpClientResponse>> closeCompleters =
<Completer<MockHttpClientResponse>>[hanger, succeeder];
succeeder.complete(MockHttpClientResponse());
when(httpRequest.close()).thenAnswer((Invocation invocation) {
final Completer<MockHttpClientResponse> completer = closeCompleters[closeCount];
closeCount += 1;
return completer.future;
});
when(httpRequest.abort()).thenAnswer((_) {
hanger.completeError(const HttpException('aborted'));
});
when(httpRequest.done).thenAnswer((_) {
if (closeCount == 1) {
return hanger.future;
} else if (closeCount == 2) {
return succeeder.future;
} else {
// This branch shouldn't happen.
fail('This branch should not happen');
}
});
final BufferLogger logger = BufferLogger.test();
final DevFS devFS = DevFS(
fakeVmServiceHost.vmService,
'test',
fileSystem.currentDirectory,
fileSystem: fileSystem,
logger: logger,
osUtils: FakeOperatingSystemUtils(),
httpClient: httpClient,
);
await devFS.create();
final DateTime previousCompile = devFS.lastCompiled;
final MockResidentCompiler residentCompiler = MockResidentCompiler();
when(residentCompiler.recompile(
any,
any,
outputPath: anyNamed('outputPath'),
packageConfig: anyNamed('packageConfig'),
)).thenAnswer((Invocation invocation) async {
fileSystem.file('example').createSync();
return const CompilerOutput('lib/foo.txt.dill', 0, <Uri>[]);
});
final UpdateFSReport report = await devFS.update(
mainUri: Uri.parse('lib/main.dart'),
generator: residentCompiler,
dillOutputPath: 'lib/foo.dill',
pathToReload: 'lib/foo.txt.dill',
trackWidgetCreation: false,
invalidatedFiles: <Uri>[],
packageConfig: PackageConfig.empty,
);
expect(report.success, true);
expect(devFS.lastCompiled, isNot(previousCompile));
expect(closeCount, 2);
expect(logger.errorText, '');
});
}
class MockHttpClientRequest extends Mock implements HttpClientRequest {}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment