Close http connections on dwds and webdev_server exit (#1129)
* Close http connections on dwds and webdev_server exit
proxyHandler is creating new http client if none is passed to it,
and this client is never closed, which causes the program to hang
in some scenarios - for example, in a coming expression evaluation
feature where the expression compiler worker is running in an
isolate, communicating with webdev isolate using send/receive
ports.
- pass an http client to all proxyHandlers
- close that client on exit
- cleanup running services on context.setup fail so to the
subsequent setup retries do not fail due to services stil
running.
* Update dwds/lib/src/readers/frontend_server_asset_reader.dart
Co-authored-by: Nate Bosch <nbosch1@gmail.com>
* Update dwds/lib/src/readers/proxy_server_asset_reader.dart
Co-authored-by: Nate Bosch <nbosch1@gmail.com>
* Update dwds/test/debugger_test.dart
Co-authored-by: Nate Bosch <nbosch1@gmail.com>
* Fix compilation errors
Co-authored-by: Nate Bosch <nbosch1@gmail.com>
diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md
index be26dba..16ab920 100644
--- a/dwds/CHANGELOG.md
+++ b/dwds/CHANGELOG.md
@@ -10,6 +10,7 @@
module paths.
- Fix issue where upgrading `build_web_compilers` would cause missing
module assets (JavaScript code and source maps).
+- Fix issue where open http connections prevent the process for exiting.
**Breaking changes:**
- Change `Dwds.start` to require `MetadataProvider` as a parameter.
diff --git a/dwds/lib/dwds.dart b/dwds/lib/dwds.dart
index a4d51ad..f7ca763 100644
--- a/dwds/lib/dwds.dart
+++ b/dwds/lib/dwds.dart
@@ -51,12 +51,14 @@
final Handler handler;
final DevTools _devTools;
final DevHandler _devHandler;
+ final AssetReader _assetReader;
final bool _enableDebugging;
Dwds._(
this.middleware,
this._devTools,
this._devHandler,
+ this._assetReader,
this._enableDebugging,
) : handler = _devHandler.handler;
@@ -68,6 +70,7 @@
Future<void> stop() async {
await _devTools?.close();
await _devHandler.close();
+ await _assetReader.close();
}
Future<DebugConnection> debugConnection(AppConnection appConnection) async {
@@ -161,6 +164,7 @@
injected.middleware,
devTools,
devHandler,
+ assetReader,
enableDebugging,
);
}
diff --git a/dwds/lib/src/readers/asset_reader.dart b/dwds/lib/src/readers/asset_reader.dart
index 55448cc..3b18864 100644
--- a/dwds/lib/src/readers/asset_reader.dart
+++ b/dwds/lib/src/readers/asset_reader.dart
@@ -14,4 +14,7 @@
/// Returns the contents for the merged metadata output at the provided path.
Future<String> metadataContents(String serverPath);
+
+ /// Closes connections
+ Future<void> close();
}
diff --git a/dwds/lib/src/readers/frontend_server_asset_reader.dart b/dwds/lib/src/readers/frontend_server_asset_reader.dart
index f64286f..c39bcac 100644
--- a/dwds/lib/src/readers/frontend_server_asset_reader.dart
+++ b/dwds/lib/src/readers/frontend_server_asset_reader.dart
@@ -108,4 +108,7 @@
// TODO(grouma) - Implement the merged metadata reader.
throw UnimplementedError();
}
+
+ @override
+ Future<void> close() async {}
}
diff --git a/dwds/lib/src/readers/proxy_server_asset_reader.dart b/dwds/lib/src/readers/proxy_server_asset_reader.dart
index 5cfe853..97f30a4 100644
--- a/dwds/lib/src/readers/proxy_server_asset_reader.dart
+++ b/dwds/lib/src/readers/proxy_server_asset_reader.dart
@@ -2,9 +2,11 @@
// for details. All rights reserved. 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 'dart:io';
+import 'package:http/http.dart' as http;
import 'package:http/io_client.dart';
import 'package:logging/logging.dart';
import 'package:shelf/shelf.dart';
@@ -18,6 +20,7 @@
final LogWriter _logWriter;
Handler _handler;
+ http.Client _client;
ProxyServerAssetReader(int assetServerPort, this._logWriter,
{String root, String host, bool isHttps}) {
@@ -25,13 +28,13 @@
root ??= '';
isHttps ??= false;
var scheme = isHttps ? 'https://' : 'http://';
- var client = isHttps
+ _client = isHttps
? IOClient(
HttpClient()..badCertificateCallback = (cert, host, port) => true)
- : null;
+ : http.Client();
var url = '$scheme$host:$assetServerPort/';
if (root?.isNotEmpty ?? false) url += '$root/';
- _handler = proxyHandler(url, client: client);
+ _handler = proxyHandler(url, client: _client);
}
@override
@@ -66,4 +69,7 @@
@override
Future<String> metadataContents(String serverPath) =>
_readResource(serverPath);
+
+ @override
+ Future<void> close() async => _client.close();
}
diff --git a/dwds/test/debugger_test.dart b/dwds/test/debugger_test.dart
index bc4bb01..e2347cb 100644
--- a/dwds/test/debugger_test.dart
+++ b/dwds/test/debugger_test.dart
@@ -55,6 +55,9 @@
'yCAAC,WAAW;IAChE;AAC0D,IAA3D,AAAS,AAAK,0DAAO;AAAe,kBAAO;;;AAEvC,gBAAQ;'
'AAGV,IAFI,kCAAqC,QAAC;AACX,MAA/B,WAAM,AAAwB,0BAAP,QAAF,AAAE,KAAK,GAAP;'
';EAEzB","file":"main.ddc.js"}';
+
+ @override
+ Future<void> close() async {}
}
void main() async {
diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart
index 7576f92..a40e4e4 100644
--- a/dwds/test/fixtures/context.dart
+++ b/dwds/test/fixtures/context.dart
@@ -6,6 +6,8 @@
import 'dart:convert';
import 'dart:io';
+import 'package:http/http.dart' as http;
+import 'package:dwds/src/readers/proxy_server_asset_reader.dart';
import 'package:build_daemon/client.dart';
import 'package:build_daemon/data/build_status.dart';
import 'package:build_daemon/data/build_target.dart';
@@ -51,6 +53,7 @@
AppConnection appConnection;
DebugConnection debugConnection;
WebkitDebugger webkitDebugger;
+ http.Client client;
int port;
Directory _outputDir;
File _entryFile;
@@ -114,166 +117,175 @@
logWriter ??= (Level level, String message) => printOnFailure(message);
spawnDds ??= true;
- var systemTempDir = Directory.systemTemp;
- _outputDir = systemTempDir.createTempSync('foo bar');
-
- var chromeDriverPort = await findUnusedPort();
- var chromeDriverUrlBase = 'wd/hub';
try {
- chromeDriver = await Process.start('chromedriver$_exeExt',
- ['--port=$chromeDriverPort', '--url-base=$chromeDriverUrlBase']);
- // On windows this takes a while to boot up, wait for the first line
- // of stdout as a signal that it is ready.
- await chromeDriver.stdout
- .transform(utf8.decoder)
- .transform(const LineSplitter())
- .first;
+ client = http.Client();
+
+ var systemTempDir = Directory.systemTemp;
+ _outputDir = systemTempDir.createTempSync('foo bar');
+
+ var chromeDriverPort = await findUnusedPort();
+ var chromeDriverUrlBase = 'wd/hub';
+ try {
+ chromeDriver = await Process.start('chromedriver$_exeExt',
+ ['--port=$chromeDriverPort', '--url-base=$chromeDriverUrlBase']);
+ // On windows this takes a while to boot up, wait for the first line
+ // of stdout as a signal that it is ready.
+ await chromeDriver.stdout
+ .transform(utf8.decoder)
+ .transform(const LineSplitter())
+ .first;
+ } catch (e) {
+ throw StateError(
+ 'Could not start ChromeDriver. Is it installed?\nError: $e');
+ }
+
+ await Process.run('pub$_batExt', ['upgrade'],
+ workingDirectory: workingDirectory);
+
+ ExpressionCompiler expressionCompiler;
+ AssetReader assetReader;
+ Handler assetHandler;
+ Stream<BuildResults> buildResults;
+ RequireStrategy requireStrategy;
+ MetadataProvider metadataProvider;
+
+ switch (compilationMode) {
+ case CompilationMode.buildDaemon:
+ {
+ daemonClient = await connectClient(
+ workingDirectory,
+ [],
+ (log) => logWriter(
+ server_log.toLoggingLevel(log.level), log.message));
+ daemonClient.registerBuildTarget(
+ DefaultBuildTarget((b) => b..target = pathToServe));
+ daemonClient.startBuild();
+
+ await daemonClient.buildResults
+ .firstWhere((results) => results.results
+ .any((result) => result.status == BuildStatus.succeeded))
+ .timeout(const Duration(seconds: 60));
+
+ var assetServerPort = daemonPort(workingDirectory);
+ assetHandler = proxyHandler(
+ 'http://localhost:$assetServerPort/$pathToServe/',
+ client: client);
+ assetReader = ProxyServerAssetReader(assetServerPort, logWriter,
+ root: pathToServe);
+ metadataProvider = MetadataProvider(assetReader, logWriter);
+
+ requireStrategy = BuildRunnerRequireStrategyProvider(
+ assetHandler, reloadConfiguration, metadataProvider)
+ .strategy;
+
+ buildResults = daemonClient.buildResults;
+ }
+ break;
+ case CompilationMode.frontendServer:
+ {
+ var fileSystemRoot = p.dirname(_packagesFilePath);
+ var entryPath =
+ _entryFile.path.substring(fileSystemRoot.length + 1);
+ webRunner = ResidentWebRunner(
+ '${Uri.file(entryPath)}',
+ urlEncoder,
+ fileSystemRoot,
+ _packagesFilePath,
+ [fileSystemRoot],
+ 'org-dartlang-app',
+ _outputDir.path,
+ logWriter);
+
+ var assetServerPort = await findUnusedPort();
+ await webRunner.run(hostname, assetServerPort, pathToServe);
+
+ expressionCompiler = webRunner.expressionCompiler;
+ assetReader = webRunner.devFS.assetServer;
+ assetHandler = webRunner.devFS.assetServer.handleRequest;
+
+ metadataProvider = MetadataProvider(assetReader, logWriter);
+ requireStrategy = FrontendServerRequireStrategyProvider(
+ reloadConfiguration, metadataProvider)
+ .strategy;
+
+ buildResults = const Stream<BuildResults>.empty();
+ }
+ break;
+ default:
+ throw Exception('Unsupported compilation mode: $compilationMode');
+ }
+
+ expressionCompiler = useFakeExpressionCompiler
+ ? FakeExpressionCompiler()
+ : expressionCompiler;
+
+ var debugPort = await findUnusedPort();
+ // If the environment variable DWDS_DEBUG_CHROME is set to the string true
+ // then Chrome will be launched with a UI rather than headless.
+ // If the extension is enabled, then Chrome will be launched with a UI
+ // since headless Chrome does not support extensions.
+ var headless = Platform.environment['DWDS_DEBUG_CHROME'] != 'true' &&
+ !enableDebugExtension;
+
+ var capabilities = Capabilities.chrome
+ ..addAll({
+ Capabilities.chromeOptions: {
+ 'args': [
+ 'remote-debugging-port=$debugPort',
+ if (enableDebugExtension) '--load-extension=debug_extension/web',
+ if (headless) '--headless'
+ ]
+ }
+ });
+ webDriver = await createDriver(
+ spec: WebDriverSpec.JsonWire,
+ desired: capabilities,
+ uri: Uri.parse(
+ 'http://127.0.0.1:$chromeDriverPort/$chromeDriverUrlBase/'));
+ var connection = ChromeConnection('localhost', debugPort);
+
+ port = await findUnusedPort();
+ testServer = await TestServer.start(
+ hostname,
+ port,
+ assetHandler,
+ assetReader,
+ requireStrategy,
+ metadataProvider,
+ pathToServe,
+ buildResults,
+ () async => connection,
+ serveDevTools,
+ enableDebugExtension,
+ autoRun,
+ enableDebugging,
+ useSse,
+ urlEncoder,
+ restoreBreakpoints,
+ expressionCompiler,
+ logWriter,
+ spawnDds);
+
+ appUrl = 'http://localhost:$port/$path';
+ await webDriver.get(appUrl);
+ var tab = await connection.getTab((t) => t.url == appUrl);
+ tabConnection = await tab.connect();
+ await tabConnection.runtime.enable();
+ await tabConnection.debugger.enable();
+
+ if (enableDebugExtension) {
+ var extensionTab = await _fetchDartDebugExtensionTab(connection);
+ extensionConnection = await extensionTab.connect();
+ await extensionConnection.runtime.enable();
+ }
+
+ appConnection = await testServer.dwds.connectedApps.first;
+ if (enableDebugging && !waitToDebug) {
+ await startDebugging();
+ }
} catch (e) {
- throw StateError(
- 'Could not start ChromeDriver. Is it installed?\nError: $e');
- }
-
- await Process.run('pub$_batExt', ['upgrade'],
- workingDirectory: workingDirectory);
-
- ExpressionCompiler expressionCompiler;
- AssetReader assetReader;
- Handler assetHandler;
- Stream<BuildResults> buildResults;
- RequireStrategy requireStrategy;
- MetadataProvider metadataProvider;
-
- switch (compilationMode) {
- case CompilationMode.buildDaemon:
- {
- daemonClient = await connectClient(
- workingDirectory,
- [],
- (log) =>
- logWriter(server_log.toLoggingLevel(log.level), log.message));
- daemonClient.registerBuildTarget(
- DefaultBuildTarget((b) => b..target = pathToServe));
- daemonClient.startBuild();
-
- await daemonClient.buildResults
- .firstWhere((results) => results.results
- .any((result) => result.status == BuildStatus.succeeded))
- .timeout(const Duration(seconds: 60));
-
- var assetServerPort = daemonPort(workingDirectory);
- assetHandler =
- proxyHandler('http://localhost:$assetServerPort/$pathToServe/');
- assetReader = ProxyServerAssetReader(assetServerPort, logWriter,
- root: pathToServe);
- metadataProvider = MetadataProvider(assetReader, logWriter);
-
- requireStrategy = BuildRunnerRequireStrategyProvider(
- assetHandler, reloadConfiguration, metadataProvider)
- .strategy;
-
- buildResults = daemonClient.buildResults;
- }
- break;
- case CompilationMode.frontendServer:
- {
- var fileSystemRoot = p.dirname(_packagesFilePath);
- var entryPath = _entryFile.path.substring(fileSystemRoot.length + 1);
- webRunner = ResidentWebRunner(
- '${Uri.file(entryPath)}',
- urlEncoder,
- fileSystemRoot,
- _packagesFilePath,
- [fileSystemRoot],
- 'org-dartlang-app',
- _outputDir.path,
- logWriter);
-
- var assetServerPort = await findUnusedPort();
- await webRunner.run(hostname, assetServerPort, pathToServe);
-
- expressionCompiler = webRunner.expressionCompiler;
- assetReader = webRunner.devFS.assetServer;
- assetHandler = webRunner.devFS.assetServer.handleRequest;
-
- metadataProvider = MetadataProvider(assetReader, logWriter);
- requireStrategy = FrontendServerRequireStrategyProvider(
- reloadConfiguration, metadataProvider)
- .strategy;
-
- buildResults = const Stream<BuildResults>.empty();
- }
- break;
- default:
- throw Exception('Unsupported compilation mode: $compilationMode');
- }
-
- expressionCompiler = useFakeExpressionCompiler
- ? FakeExpressionCompiler()
- : expressionCompiler;
-
- var debugPort = await findUnusedPort();
- // If the environment variable DWDS_DEBUG_CHROME is set to the string true
- // then Chrome will be launched with a UI rather than headless.
- // If the extension is enabled, then Chrome will be launched with a UI
- // since headless Chrome does not support extensions.
- var headless = Platform.environment['DWDS_DEBUG_CHROME'] != 'true' &&
- !enableDebugExtension;
-
- var capabilities = Capabilities.chrome
- ..addAll({
- Capabilities.chromeOptions: {
- 'args': [
- 'remote-debugging-port=$debugPort',
- if (enableDebugExtension) '--load-extension=debug_extension/web',
- if (headless) '--headless'
- ]
- }
- });
- webDriver = await createDriver(
- spec: WebDriverSpec.JsonWire,
- desired: capabilities,
- uri: Uri.parse(
- 'http://127.0.0.1:$chromeDriverPort/$chromeDriverUrlBase/'));
- var connection = ChromeConnection('localhost', debugPort);
-
- port = await findUnusedPort();
- testServer = await TestServer.start(
- hostname,
- port,
- assetHandler,
- assetReader,
- requireStrategy,
- metadataProvider,
- pathToServe,
- buildResults,
- () async => connection,
- serveDevTools,
- enableDebugExtension,
- autoRun,
- enableDebugging,
- useSse,
- urlEncoder,
- restoreBreakpoints,
- expressionCompiler,
- logWriter,
- spawnDds);
-
- appUrl = 'http://localhost:$port/$path';
- await webDriver.get(appUrl);
- var tab = await connection.getTab((t) => t.url == appUrl);
- tabConnection = await tab.connect();
- await tabConnection.runtime.enable();
- await tabConnection.debugger.enable();
-
- if (enableDebugExtension) {
- var extensionTab = await _fetchDartDebugExtensionTab(connection);
- extensionConnection = await extensionTab.connect();
- await extensionConnection.runtime.enable();
- }
-
- appConnection = await testServer.dwds.connectedApps.first;
- if (enableDebugging && !waitToDebug) {
- await startDebugging();
+ await tearDown();
+ rethrow;
}
}
@@ -290,7 +302,17 @@
await daemonClient?.close();
await webRunner?.stop();
await testServer?.stop();
+ client?.close();
await _outputDir?.delete(recursive: true);
+
+ // clear the state for next setup
+ webDriver = null;
+ chromeDriver = null;
+ daemonClient = null;
+ webRunner = null;
+ testServer = null;
+ client = null;
+ _outputDir = null;
}
Future<void> changeInput() async {
diff --git a/dwds/test/metadata_test.dart b/dwds/test/metadata_test.dart
index 913e8b9..76a99cb 100644
--- a/dwds/test/metadata_test.dart
+++ b/dwds/test/metadata_test.dart
@@ -23,6 +23,9 @@
@override
Future<String> sourceMapContents(String serverPath) =>
throw UnimplementedError();
+
+ @override
+ Future<void> close() async {}
}
void main() {
diff --git a/frontend_server_common/lib/src/asset_server.dart b/frontend_server_common/lib/src/asset_server.dart
index d8cbd1a..834afe2 100644
--- a/frontend_server_common/lib/src/asset_server.dart
+++ b/frontend_server_common/lib/src/asset_server.dart
@@ -133,7 +133,8 @@
}
/// Tear down the http server running.
- Future<void> dispose() {
+ @override
+ Future<void> close() {
return _httpServer.close();
}
diff --git a/frontend_server_common/lib/src/devfs.dart b/frontend_server_common/lib/src/devfs.dart
index 9858800..ac11d5a 100644
--- a/frontend_server_common/lib/src/devfs.dart
+++ b/frontend_server_common/lib/src/devfs.dart
@@ -53,7 +53,7 @@
Future<void> dispose() {
fileSystem.currentDirectory = _savedCurrentDirectory;
- return assetServer.dispose();
+ return assetServer.close();
}
Future<UpdateFSReport> update({
diff --git a/webdev/CHANGELOG.md b/webdev/CHANGELOG.md
index ae786a0..c237bba 100644
--- a/webdev/CHANGELOG.md
+++ b/webdev/CHANGELOG.md
@@ -4,6 +4,7 @@
- Expand `package:vm_service` to version `>=3.0.0 <6.0.0`.
- Require at least `build_web_compilers` version `2.12.0`.
- Depend on the latest `package:dwds`.
+- Fix issue where open http connections prevent the process for exiting.
## 2.6.2
diff --git a/webdev/lib/src/serve/webdev_server.dart b/webdev/lib/src/serve/webdev_server.dart
index 169e5fa..afaba86 100644
--- a/webdev/lib/src/serve/webdev_server.dart
+++ b/webdev/lib/src/serve/webdev_server.dart
@@ -5,6 +5,7 @@
import 'dart:async';
import 'dart:io';
+import 'package:http/http.dart' as http;
import 'package:build_daemon/data/build_status.dart' as daemon;
import 'package:dwds/data/build_result.dart';
import 'package:dwds/dwds.dart';
@@ -34,7 +35,7 @@
class WebDevServer {
final HttpServer _server;
-
+ final http.Client _client;
final String _protocol;
final Stream<BuildResult> buildResults;
@@ -47,6 +48,7 @@
WebDevServer._(
this.target,
this._server,
+ this._client,
this._protocol,
this.buildResults,
bool autoRun, {
@@ -66,6 +68,7 @@
Future<void> stop() async {
await dwds?.stop();
await _server.close(force: true);
+ _client?.close();
}
static Future<WebDevServer> start(
@@ -94,8 +97,10 @@
});
var cascade = Cascade();
+ var client = http.Client();
var assetHandler = proxyHandler(
- 'http://localhost:${options.daemonPort}/${options.target}/');
+ 'http://localhost:${options.daemonPort}/${options.target}/',
+ client: client);
Dwds dwds;
if (options.configuration.enableInjectedClient) {
var assetReader = ProxyServerAssetReader(
@@ -149,6 +154,7 @@
return WebDevServer._(
options.target,
server,
+ client,
protocol,
filteredBuildResults,
options.configuration.autoRun,
