[frontend_server] These changes ensure that only 1 resident_frontend_server is running when multiple entities request to start the compilation server. Only 1 server is allowed to exist per server information file. Multiple resident_frontend_servers may exist if the user passes in different files each time a server is spawned.
Change-Id: I5cd0eceb391f6a289318c17e74069b63ff37126c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254800
Reviewed-by: Siva Annamalai <asiva@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Michael Richards <msrichards@google.com>
diff --git a/pkg/frontend_server/lib/src/resident_frontend_server.dart b/pkg/frontend_server/lib/src/resident_frontend_server.dart
index 6144938..2483b28 100644
--- a/pkg/frontend_server/lib/src/resident_frontend_server.dart
+++ b/pkg/frontend_server/lib/src/resident_frontend_server.dart
@@ -5,7 +5,16 @@
// @dart = 2.9
import 'dart:async';
import 'dart:convert';
-import 'dart:io' show File, InternetAddress, ServerSocket, Socket;
+import 'dart:io'
+ show
+ exit,
+ File,
+ InternetAddress,
+ Link,
+ Platform,
+ ProcessSignal,
+ ServerSocket,
+ Socket;
import 'dart:typed_data' show Uint8List;
import 'package:args/args.dart';
@@ -25,6 +34,12 @@
/// required.
const _STAT_GRANULARITY = const Duration(seconds: 1);
+const RESIDENT_SERVER_LINK_POSTFIX = '_link';
+
+/// Ensures the symbolic link is removed if ctrl-C is sent to the server.
+/// Mostly used when debugging.
+StreamSubscription<ProcessSignal> _cleanupHandler;
+
extension on DateTime {
/// Truncates by [amount].
///
@@ -280,7 +295,8 @@
if (request[_executableString] == null ||
request[_outputString] == null) {
return _encodeErrorMessage(
- 'compilation requests must include an $_executableString and an $_outputString path.');
+ 'compilation requests must include an $_executableString '
+ 'and an $_outputString path.');
}
final executablePath = request[_executableString];
final cachedDillPath = request[_outputString];
@@ -411,12 +427,28 @@
}
/// Closes the ServerSocket and removes the [serverInfoFile] that is used
-/// to access this instance of the Resident Frontend Server
+/// to access this instance of the Resident Frontend Server as well as the
+/// lock to prevent the concurrent start race.
Future<void> residentServerCleanup(
ServerSocket server, File serverInfoFile) async {
- if (serverInfoFile.existsSync()) {
- serverInfoFile.deleteSync();
+ final serverFilesystemLock =
+ Link('${serverInfoFile.path}$RESIDENT_SERVER_LINK_POSTFIX');
+ try {
+ if (_cleanupHandler != null) {
+ _cleanupHandler.cancel();
+ }
+ if (serverInfoFile.existsSync()) {
+ serverInfoFile.deleteSync();
+ }
+ } catch (_) {
+ } finally {
+ try {
+ if (serverFilesystemLock.existsSync()) {
+ serverFilesystemLock.deleteSync();
+ }
+ } catch (_) {}
}
+
await server.close();
}
@@ -437,19 +469,51 @@
InternetAddress address, int port, File serverInfoFile,
{Duration inactivityTimeout = const Duration(minutes: 30)}) async {
ServerSocket server;
+ // Create a link to the serverInfoFile to ensure that concurrent requests
+ // to start the server result in only 1 server being started. This
+ // also ensures that the serverInfoFile is only
+ // visible once the server is started and ready to receive connections.
+ // TODO https://github.com/dart-lang/sdk/issues/49647 use exclusive mode
+ // on File objects
+ final serverInfoLink =
+ Link('${serverInfoFile.path}$RESIDENT_SERVER_LINK_POSTFIX');
try {
+ try {
+ serverInfoLink.createSync(serverInfoFile.path);
+ } catch (e) {
+ // TODO: https://github.com/dart-lang/sdk/issues/49647 Using a File
+ // in exclusive mode removes the need for this check.
+ if (Platform.isWindows && e.toString().contains('errno = 1314')) {
+ throw StateError('Dart must be running in Administrator mode '
+ 'or Developer mode must be enabled when '
+ 'using the Resident Frontend Compiler.');
+ }
+ throw StateError('A server is already running.');
+ }
server = await ServerSocket.bind(address, port);
serverInfoFile
..writeAsStringSync(
'address:${server.address.address} port:${server.port}');
+ } on StateError catch (e) {
+ print('Error: $e\n');
+ return null;
} catch (e) {
+ // lock was acquired but bind or writing failed
+ try {
+ serverInfoLink.deleteSync();
+ } catch (_) {}
print('Error: $e\n');
return null;
}
+
+ _cleanupHandler = ProcessSignal.sigint.watch().listen((signal) async {
+ await residentServerCleanup(server, serverInfoFile);
+ exit(1);
+ });
var shutdownTimer =
startShutdownTimer(inactivityTimeout, server, serverInfoFile);
- print(
- 'The Resident Frontend Compiler is listening at ${server.address.address}:${server.port}');
+ print('The Resident Frontend Compiler is listening at '
+ '${server.address.address}:${server.port}');
return server.listen((client) {
client.listen((Uint8List data) async {
diff --git a/pkg/frontend_server/test/src/resident_frontend_server_test.dart b/pkg/frontend_server/test/src/resident_frontend_server_test.dart
index 0ea901e..8c44392 100644
--- a/pkg/frontend_server/test/src/resident_frontend_server_test.dart
+++ b/pkg/frontend_server/test/src/resident_frontend_server_test.dart
@@ -562,6 +562,39 @@
expect(shutdownResult['errorMessage'], contains('SocketException'));
});
+ test('concurrent startup requests', () async {
+ final serverSubscription = await residentListenAndCompile(
+ InternetAddress.loopbackIPv4,
+ 0,
+ serverInfo,
+ );
+ final startWhileAlreadyRunning = await residentListenAndCompile(
+ InternetAddress.loopbackIPv4,
+ 0,
+ serverInfo,
+ );
+
+ expect(serverSubscription, isNot(null));
+ expect(startWhileAlreadyRunning, null);
+ expect(serverInfo.existsSync(), true);
+ expect(
+ Link('${serverInfo.path}$RESIDENT_SERVER_LINK_POSTFIX').existsSync(),
+ true);
+
+ final info = serverInfo.readAsStringSync();
+ final address = InternetAddress(
+ info.substring(info.indexOf(':') + 1, info.indexOf(' ')));
+ final port = int.parse(info.substring(info.lastIndexOf(':') + 1));
+
+ final shutdownResult = await sendAndReceiveResponse(
+ address, port, ResidentFrontendServer.shutdownCommand);
+ expect(shutdownResult, equals(<String, dynamic>{"shutdown": true}));
+ expect(serverInfo.existsSync(), false);
+ expect(
+ Link('${serverInfo.path}$RESIDENT_SERVER_LINK_POSTFIX').existsSync(),
+ false);
+ });
+
test('resident server starter', () async {
final returnValue =
starter(['--resident-info-file-name=${serverInfo.path}']);
