croupier: Start and Stop Discovery Better
3 main changes
- Stop Scan and Advertise using a Future
- Scan and Advertise with attributes
- Do not start scanning until we have settings
Advertise and Scan were also made a LOT less race-y.
https://vanadium-review.googlesource.com/#/c/17722 inspired
the fixed, though it turns out I was mishandling a lot of
Futures along the way.
Closes https://github.com/vanadium/issues/issues/845
Change-Id: I8003a490a82c6522982fde3db2a2c9bee4f356bd
diff --git a/lib/logic/croupier.dart b/lib/logic/croupier.dart
index 5cb2eb4..eec18d6 100644
--- a/lib/logic/croupier.dart
+++ b/lib/logic/croupier.dart
@@ -2,11 +2,13 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
-import 'game/game.dart'
- show Game, GameType, GameStartData, stringToGameType, gameTypeToString;
+import 'dart:async';
+
+import '../src/syncbase/settings_manager.dart' show SettingsManager;
import 'create_game.dart' as cg;
import 'croupier_settings.dart' show CroupierSettings;
-import '../src/syncbase/settings_manager.dart' show SettingsManager;
+import 'game/game.dart'
+ show Game, GameType, GameStartData, stringToGameType, gameTypeToString;
enum CroupierState {
Welcome,
@@ -30,6 +32,10 @@
Game game; // null until chosen
NoArgCb informUICb;
+ // Futures to use in order to cancel scans and advertisements.
+ Future _scanFuture;
+ Future _advertiseFuture;
+
Croupier() {
state = CroupierState.Welcome;
settings_everyone = new Map<int, CroupierSettings>();
@@ -91,12 +97,6 @@
case CroupierState.Welcome:
// data should be empty.
assert(data == null);
-
- // Start scanning for games if that's what's next for you.
- if (nextState == CroupierState.JoinGame) {
- settings_manager.scanSettings(); // don't wait for this future.
- }
-
break;
case CroupierState.Settings:
// data should be empty.
@@ -114,18 +114,20 @@
GameType gt = data as GameType;
game = cg.createGame(gt, 0); // Start as player 0 of whatever game type.
- settings_manager
+ _advertiseFuture = settings_manager
.createGameSyncgroup(gameTypeToString(gt), game.gameID)
.then((GameStartData gsd) {
// Only the game chooser should be advertising the game.
- settings_manager
- .advertiseSettings(gsd); // don't wait for this future.
- });
+ return settings_manager.advertiseSettings(gsd);
+ }); // don't wait for this future.
break;
case CroupierState.JoinGame:
// Note that if we were in join game, we must have been scanning.
- settings_manager.stopScanSettings();
+ _scanFuture.then((_) {
+ settings_manager.stopScanSettings();
+ _scanFuture = null;
+ });
if (data == null) {
// Back button pressed.
@@ -148,7 +150,12 @@
break;
case CroupierState.ArrangePlayers:
// Note that if we were arranging players, we might have been advertising.
- settings_manager.stopAdvertiseSettings();
+ if (_advertiseFuture != null) {
+ _advertiseFuture.then((_) {
+ settings_manager.stopAdvertiseSettings();
+ _advertiseFuture = null;
+ });
+ }
// data should be empty.
// All rearrangements affect the Game's player number without changing app state.
@@ -173,6 +180,10 @@
if (nextState == CroupierState.Welcome) {
games_found = new Map<String, GameStartData>();
players_found = new Map<int, int>();
+ } else if (nextState == CroupierState.JoinGame) {
+ // Start scanning for games since that's what's next for you.
+ _scanFuture =
+ settings_manager.scanSettings(); // don't wait for this future.
}
state = nextState;
diff --git a/lib/src/mocks/log_writer.dart b/lib/src/mocks/log_writer.dart
index 26e9075..19e6d58 100644
--- a/lib/src/mocks/log_writer.dart
+++ b/lib/src/mocks/log_writer.dart
@@ -6,10 +6,10 @@
enum SimulLevel { TURN_BASED, INDEPENDENT, DEPENDENT }
-typedef void updateCallbackT(String key, String value);
+typedef void keyValueCallback(String key, String value);
class LogWriter {
- final updateCallbackT updateCallback;
+ final keyValueCallback updateCallback;
final List<int> users;
String logPrefix; // This can be completely ignored.
diff --git a/lib/src/mocks/settings_manager.dart b/lib/src/mocks/settings_manager.dart
index f6aac0a..71d497a 100644
--- a/lib/src/mocks/settings_manager.dart
+++ b/lib/src/mocks/settings_manager.dart
@@ -9,9 +9,9 @@
import '../../logic/croupier_settings.dart' show CroupierSettings;
class SettingsManager {
- final util.updateCallbackT updateCallback;
- final util.updateCallbackT updateGamesCallback;
- final util.updateCallbackT updatePlayerFoundCallback;
+ final util.keyValueCallback updateCallback;
+ final util.keyValueCallback updateGamesCallback;
+ final util.keyValueCallback updatePlayerFoundCallback;
SettingsManager(this.updateCallback, this.updateGamesCallback,
this.updatePlayerFoundCallback);
@@ -44,13 +44,17 @@
return new Future(() => null);
}
- void stopScanSettings() {}
+ Future stopScanSettings() {
+ return new Future(() => null);
+ }
Future advertiseSettings(logic_game.GameStartData gsd) {
return new Future(() => null);
}
- void stopAdvertiseSettings() {}
+ Future stopAdvertiseSettings() {
+ return new Future(() => null);
+ }
Future createGameSyncgroup(String type, int gameID) {
return new Future(() => null);
diff --git a/lib/src/mocks/util.dart b/lib/src/mocks/util.dart
new file mode 100644
index 0000000..e466b8d
--- /dev/null
+++ b/lib/src/mocks/util.dart
@@ -0,0 +1,5 @@
+// Copyright 2015 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+typedef void keyValueCallback(String key, String value);
diff --git a/lib/src/syncbase/discovery_client.dart b/lib/src/syncbase/discovery_client.dart
index bcf0482..51dcb9f 100644
--- a/lib/src/syncbase/discovery_client.dart
+++ b/lib/src/syncbase/discovery_client.dart
@@ -17,10 +17,13 @@
/// Make this into the Dart Discovery client
/// https://github.com/vanadium/issues/issues/835
class DiscoveryClient {
- final Map<String, ProxyHandlePair<discovery.AdvertiserProxy>> advertisers =
- new Map<String, ProxyHandlePair<discovery.AdvertiserProxy>>();
- final Map<String, ProxyHandlePair<discovery.ScannerProxy>> scanners =
- new Map<String, ProxyHandlePair<discovery.ScannerProxy>>();
+ final Map<String,
+ Future<ProxyHandlePair<discovery.AdvertiserProxy>>> _advertisers =
+ new Map<String, Future<ProxyHandlePair<discovery.AdvertiserProxy>>>();
+ final Map<String, Future<ProxyHandlePair<discovery.ScannerProxy>>> _scanners =
+ new Map<String, Future<ProxyHandlePair<discovery.ScannerProxy>>>();
+ final Map<String, Future> _stoppingAdvertisers = new Map<String, Future>();
+ final Map<String, Future> _stoppingScanners = new Map<String, Future>();
static final String discoveryUrl = 'https://mojo2.v.io/discovery.mojo';
@@ -46,77 +49,132 @@
// Scans for this query and handles found/lost objects with the handler.
// Keeps track of this scanner via the key.
Future scan(String key, String query, discovery.ScanHandler handler) async {
- // Cancel the scan if one is already going for this key.
- if (scanners.containsKey(key)) {
- stopScan(key);
+ // Return the existing scan if one is already going.
+ if (_scanners.containsKey(key)) {
+ return _scanners[key];
}
- discovery.ScannerProxy s = new discovery.ScannerProxy.unbound();
+ Future _scanHelper() async {
+ discovery.ScannerProxy s = new discovery.ScannerProxy.unbound();
- print('Starting up discovery scanner ${key}. Looking for ${query}');
+ print('Starting up discovery scanner ${key}. Looking for ${query}');
- shell.connectToService(discoveryUrl, s);
+ shell.connectToService(discoveryUrl, s);
- // Use a ScanHandlerStub (Mojo-encodable interface) to wrap the scan handler.
- discovery.ScanHandlerStub shs = new discovery.ScanHandlerStub.unbound();
- shs.impl = handler;
+ // Use a ScanHandlerStub (Mojo-encodable interface) to wrap the scan handler.
+ discovery.ScanHandlerStub shs = new discovery.ScanHandlerStub.unbound();
+ shs.impl = handler;
- print('Scanning begins!');
- return s.ptr
- .scan(query, shs)
- .then((discovery.ScannerScanResponseParams response) {
- print(
- "${key} scanning started. The cancel handle is ${response.handle}.");
- scanners[key] =
- new ProxyHandlePair<discovery.ScannerProxy>(s, response.handle);
+ print('Scanning begins!');
+ return s.ptr
+ .scan(query, shs)
+ .then((discovery.ScannerScanResponseParams response) {
+ print(
+ "${key} scanning started. The cancel handle is ${response.handle}.");
+
+ return new ProxyHandlePair<discovery.ScannerProxy>(s, response.handle);
+ });
+ }
+
+ // Otherwise, set _scanners[key] and do the preparation inside the future
+ // so that stopScan can stop it if the two are called back to back.
+ _scanners[key] = _scanHelper();
+
+ return _scanners[key];
+ }
+
+ // This sends a stop signal to the scanner. Handles repeated stop calls on
+ // the same key by returning the same Future.
+ Future stopScan(String key) {
+ if (!_scanners.containsKey(key)) {
+ return new Future.value();
+ }
+ if (_stoppingScanners.containsKey(key)) {
+ return _stoppingScanners[key];
+ }
+
+ _stoppingScanners[key] = _stopScanHelper(key);
+
+ return _stoppingScanners[key].then((_) {
+ // Success! Let's clean up both _scanners and _stoppingScanners.
+ _scanners.remove(key);
+ _stoppingScanners.remove(key);
+ }).catchError((e) {
+ // Failure. We can only clean up _stoppingScanners.
+ _stoppingScanners.remove(key);
+ throw e;
});
}
- // This sends a stop signal to the scanner. Since it is non-blocking, the
- // scan handle may not stop instantaneously.
- void stopScan(String key) {
- if (scanners[key] != null) {
- print("Stopping scan for ${key}.");
- scanners[key].proxy.ptr.stop(scanners[key].handle);
- scanners[key].proxy.close(); // don't wait for this future.
- scanners.remove(key);
- }
+ Future _stopScanHelper(String key) async {
+ ProxyHandlePair<discovery.ScannerProxy> sp = await _scanners[key];
+ await sp.proxy.ptr.stop(sp.handle);
+ await sp.proxy.close();
+ print("Scan was stopped for ${key}!");
}
// Advertises the given service information. Keeps track of the advertiser
// handle via the key.
Future advertise(String key, discovery.Service serviceInfo,
{List<String> visibility}) async {
- // Cancel the advertisement if one is already going for this key.
- if (advertisers.containsKey(key)) {
- stopAdvertise(key);
+ // Return the existing advertisement if one is already going.
+ if (_advertisers.containsKey(key)) {
+ return _advertisers[key];
}
- discovery.AdvertiserProxy a = new discovery.AdvertiserProxy.unbound();
+ Future _advertiseHelper() async {
+ discovery.AdvertiserProxy a = new discovery.AdvertiserProxy.unbound();
- print(
- 'Starting up discovery advertiser ${key}. Broadcasting for ${serviceInfo.instanceName}');
-
- shell.connectToService(discoveryUrl, a);
-
- return a.ptr
- .advertise(serviceInfo, visibility ?? <String>[])
- .then((discovery.AdvertiserAdvertiseResponseParams response) {
print(
- "${key} advertising started. The cancel handle is ${response.handle}.");
- advertisers[key] =
- new ProxyHandlePair<discovery.AdvertiserProxy>(a, response.handle);
+ 'Starting up discovery advertiser ${key}. Broadcasting for ${serviceInfo.instanceName}');
+
+ shell.connectToService(discoveryUrl, a);
+
+ return a.ptr
+ .advertise(serviceInfo, visibility ?? <String>[])
+ .then((discovery.AdvertiserAdvertiseResponseParams response) {
+ print(
+ "${key} advertising started. The cancel handle is ${response.handle}.");
+
+ return new ProxyHandlePair<discovery.AdvertiserProxy>(
+ a, response.handle);
+ });
+ }
+
+ // Otherwise, set _advertisers[key] and do the preparation inside the future
+ // so that stopAdvertise can stop it if the two are called back to back.
+ _advertisers[key] = _advertiseHelper();
+
+ return _advertisers[key];
+ }
+
+ // This sends a stop signal to the advertiser. Handles repeated stop calls on
+ // the same key by returning the same Future.
+ Future stopAdvertise(String key) {
+ if (!_advertisers.containsKey(key)) {
+ return new Future.value();
+ }
+ if (_stoppingAdvertisers.containsKey(key)) {
+ return _stoppingAdvertisers[key];
+ }
+
+ _stoppingAdvertisers[key] = _stopAdvertiseHelper(key);
+
+ return _stoppingAdvertisers[key].then((_) {
+ // Success! Let's clean up both _advertisers and _stoppingAdvertisers.
+ _advertisers.remove(key);
+ _stoppingAdvertisers.remove(key);
+ }).catchError((e) {
+ // Failure. We can only clean up _stoppingAdvertisers.
+ _stoppingAdvertisers.remove(key);
+ throw e;
});
}
- // This sends a stop signal to the advertiser. Since it is non-blocking, the
- // advertise handle may not stop instantaneously.
- void stopAdvertise(String key) {
- if (advertisers[key] != null) {
- print("Stopping advertise for ${key}.");
- advertisers[key].proxy.ptr.stop(advertisers[key].handle);
- advertisers[key].proxy.close(); // don't wait for this future.
- advertisers.remove(key);
- }
+ Future _stopAdvertiseHelper(String key) async {
+ ProxyHandlePair<discovery.AdvertiserProxy> ap = await _advertisers[key];
+ await ap.proxy.ptr.stop(ap.handle);
+ await ap.proxy.close();
+ print("Advertise was stopped for ${key}!");
}
}
diff --git a/lib/src/syncbase/log_writer.dart b/lib/src/syncbase/log_writer.dart
index a588731..a146839 100644
--- a/lib/src/syncbase/log_writer.dart
+++ b/lib/src/syncbase/log_writer.dart
@@ -30,7 +30,7 @@
class LogWriter {
// This callback is called on each watch update, passing the key and value.
- final util.updateCallbackT updateCallback;
+ final util.keyValueCallback updateCallback;
// The users that we should look for when coming to a proposal consensus.
final List<int> users;
diff --git a/lib/src/syncbase/settings_manager.dart b/lib/src/syncbase/settings_manager.dart
index 66fccee..7c11939 100644
--- a/lib/src/syncbase/settings_manager.dart
+++ b/lib/src/syncbase/settings_manager.dart
@@ -27,9 +27,9 @@
import 'package:syncbase/syncbase_client.dart' as sc;
class SettingsManager {
- final util.updateCallbackT updateSettingsCallback;
- final util.updateCallbackT updateGamesCallback;
- final util.updateCallbackT updatePlayerFoundCallback;
+ final util.keyValueCallback updateSettingsCallback;
+ final util.keyValueCallback updateGamesCallback;
+ final util.keyValueCallback updatePlayerFoundCallback;
final CroupierClient _cc;
sc.SyncbaseTable tb;
@@ -59,8 +59,8 @@
tb = await _cc.createTable(db, util.tableNameSettings);
// Start to watch the stream for the shared settings table.
- Stream<sc.WatchChange> watchStream = db.watch(util.tableNameSettings,
- _settingsWatchSyncPrefix, UTF8.encode("now"));
+ Stream<sc.WatchChange> watchStream = db.watch(
+ util.tableNameSettings, _settingsWatchSyncPrefix, UTF8.encode("now"));
_startWatchSettings(watchStream); // Don't wait for this future.
_loadSettings(tb); // Don't wait for this future.
}
@@ -198,7 +198,7 @@
new logic_game.GameStartData(type, 0, gameID, id);
await _cc.createSyncgroup(
- _cc.makeSyncgroupName(util.syncgameSuffix(gsd.toJSONString())),
+ _cc.makeSyncgroupName(util.syncgameSuffix("${gsd.gameID}")),
util.tableNameGames,
prefix: util.syncgamePrefix(gameID));
@@ -250,30 +250,31 @@
Future scanSettings() async {
SettingsScanHandler ssh =
new SettingsScanHandler(_cc, this.updateGamesCallback);
- _cc.discoveryClient
- .scan(_discoverySettingsKey, 'v.InterfaceName="${util.discoveryInterfaceName}"', ssh);
+ return _cc.discoveryClient.scan(_discoverySettingsKey,
+ 'v.InterfaceName="${util.discoveryInterfaceName}"', ssh);
}
- void stopScanSettings() {
- _cc.discoveryClient.stopScan(_discoverySettingsKey);
+ Future stopScanSettings() {
+ return _cc.discoveryClient.stopScan(_discoverySettingsKey);
}
// Someone who wants to join a game should advertise their presence.
Future advertiseSettings(logic_game.GameStartData gsd) async {
String suffix = await _syncSuffix();
- String gameSuffix = util.syncgameSuffix(gsd.toJSONString());
- _cc.discoveryClient.advertise(
+ String gameSuffix = util.syncgameSuffix("${gsd.gameID}");
+ return _cc.discoveryClient.advertise(
_discoverySettingsKey,
DiscoveryClient.serviceMaker(
interfaceName: util.discoveryInterfaceName,
- addrs: <String>[
- _cc.makeSyncgroupName(suffix),
- _cc.makeSyncgroupName(gameSuffix)
- ]));
+ attrs: <String, String>{
+ util.syncgameSettingsAttr: _cc.makeSyncgroupName(suffix),
+ util.syncgameGameStartDataAttr: gsd.toJSONString()
+ },
+ addrs: <String>[_cc.makeSyncgroupName(gameSuffix)]));
}
- void stopAdvertiseSettings() {
- _cc.discoveryClient.stopAdvertise(_discoverySettingsKey);
+ Future stopAdvertiseSettings() {
+ return _cc.discoveryClient.stopAdvertise(_discoverySettingsKey);
}
Future<int> _getUserID() async {
@@ -306,7 +307,7 @@
CroupierClient _cc;
Map<String, String> settingsAddrs;
Map<String, String> gameAddrs;
- util.updateCallbackT updateGamesCallback;
+ util.keyValueCallback updateGamesCallback;
SettingsScanHandler(this._cc, this.updateGamesCallback) {
settingsAddrs = new Map<String, String>();
@@ -317,15 +318,15 @@
util.log(
"SettingsScanHandler Found ${s.instanceId} ${s.instanceName} ${s.addrs}");
- if (s.addrs.length == 2) {
- // Note: Assumes 2 addresses.
- settingsAddrs[s.instanceId] = s.addrs[0];
- gameAddrs[s.instanceId] = s.addrs[1];
+ if (s.addrs.length == 1 && s.attrs != null) {
+ // Note: Assumes 1 address and attributes for the game.
+ settingsAddrs[s.instanceId] = s.attrs[util.syncgameSettingsAttr];
+ gameAddrs[s.instanceId] = s.addrs[0];
- String json = _getPartFromBack(s.addrs[1], "-", 0);
- updateGamesCallback(s.addrs[1], json);
+ String gameSettingsJSON = s.attrs[util.syncgameGameStartDataAttr];
+ updateGamesCallback(gameAddrs[s.instanceId], gameSettingsJSON);
- _cc.joinSyncgroup(s.addrs[0]);
+ _cc.joinSyncgroup(settingsAddrs[s.instanceId]);
} else {
// An unexpected service was found. Who is advertising it?
// https://github.com/vanadium/issues/issues/846
diff --git a/lib/src/syncbase/util.dart b/lib/src/syncbase/util.dart
index 1853bbe..4bc8bb2 100644
--- a/lib/src/syncbase/util.dart
+++ b/lib/src/syncbase/util.dart
@@ -4,25 +4,25 @@
import 'package:syncbase/syncbase_client.dart' show Perms, SyncbaseClient;
-const appName = 'app';
-const dbName = 'db';
-const tableNameGames = 'games';
-const tableNameSettings = 'table_settings';
+const String appName = 'app';
+const String dbName = 'db';
+const String tableNameGames = 'games';
+const String tableNameSettings = 'table_settings';
// TODO(alexfandrianto): This may need to be the global mount table with a
// proxy. Otherwise, it will be difficult for other users to run.
// https://github.com/vanadium/issues/issues/782
-const mtAddr = '/192.168.86.254:8101';
-const sgPrefix = 'croupierAlex/%%sync';
-const sgSuffix = 'discovery';
-const sgSuffixGame = 'gaming';
+const String mtAddr = '/192.168.86.254:8101';
+const String sgPrefix = 'croupierAlex/%%sync';
+const String sgSuffix = 'discovery';
+const String sgSuffixGame = 'gaming';
-const discoveryInterfaceName = 'CroupierSettingsAndGame2';
+const String discoveryInterfaceName = 'CroupierSettingsAndGame2';
typedef void NoArgCb();
-typedef void updateCallbackT(String key, String value);
+typedef void keyValueCallback(String key, String value);
-String openPermsJson =
+const String openPermsJson =
'{"Admin":{"In":["..."]},"Write":{"In":["..."]},"Read":{"In":["..."]},"Resolve":{"In":["..."]},"Debug":{"In":["..."]}}';
Perms openPerms = SyncbaseClient.perms(openPermsJson);
@@ -40,3 +40,5 @@
return "${gameID}";
}
+const String syncgameSettingsAttr = "settings_sgname";
+const String syncgameGameStartDataAttr = "game_start_data";
