accounts: before change accounts locking arrangements

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

diff --git a/src/api.rs b/src/api.rs
index 72edb05f8664e2c2cd2ec8bd78f6140861ee28db..353d1bba129fec014bd008b84169d9eb8d85f850 100644 (file)
--- a/src/api.rs
+++ b/src/api.rs
@@ -18,7 +18,7 @@ struct ApiPiece<O : ApiPieceOp> {
 struct ApiPieceOpArgs<'a> {
   gs: &'a mut GameState,
   player: PlayerId,
-  pst: &'a PlayerState,
+  pst: &'a IPlayerState,
   piece: PieceId,
   p: &'a dyn Piece,
   iplayers: &'a SecondarySlotMap<PlayerId, PlayerRecord>,

diff --git a/src/cmdlistener.rs b/src/cmdlistener.rs
index 4d5cb91a891ecb1f97a27f7bf3144f099d48b7bc..376199e28ca5453c3a5cb4fa07d5e205f2e4e14e 100644 (file)
--- a/src/cmdlistener.rs
+++ b/src/cmdlistener.rs
@@ -268,6 +268,7 @@ fn execute_game_insn<'ig>(
     Insn::AddPlayer(add) => {
       // todo some kind of permissions check for player too
       let ig = cs.check_acl(ig, PCH::Instance, &[TP::AddPlayer])?.0;
+      let record = AccountRecord::lookup(&add.account)?;
       let nick = add.nick.ok_or(ME::ParameterMissing)?;
       let logentry = LogEntry {
         html: Html(format!("{} added a player: {}", &who,
@@ -279,11 +280,13 @@ fn execute_game_insn<'ig>(
         Ok(tz) => tz,
         Err(x) => match x { },
       };
-      let st = PlayerState {
-        tz,
-        account: add.account,
+      let gpl = GPlayerState {
         nick: nick.to_string(),
       };
+      let ipl = IPlayerState {
+        account: add.account,
+        tz,
+      };
       let (player, logentry) = ig.player_new(st, tz, logentry)?;
       (U{ pcs: vec![],
           log: vec![ logentry ],

diff --git a/src/gamestate.rs b/src/gamestate.rs
index 43dd12cb5c2b12da59c846250a36e0b207891fd6..65848b8be04f93cc99e8c4f9e32fb3373d18f493 100644 (file)
--- a/src/gamestate.rs
+++ b/src/gamestate.rs
@@ -46,7 +46,12 @@ pub struct GameState {
   pub gen : Generation,
   pub log : VecDeque<(Generation, Arc<CommittedLogEntry>)>,
   pub max_z : ZCoord,
-  pub players : DenseSlotMap<PlayerId, String /* nick */>,
+  pub players : DenseSlotMap<PlayerId, GPlayerState>,
+}
+
+#[derive(Debug,Serialize,Deserialize,Clone)]
+pub struct GPlayerState {
+  pub nick: String,
 }
 
 #[derive(Debug,Serialize,Deserialize)]

diff --git a/src/global.rs b/src/global.rs
index 8e5b1be5aac2e0efa5dfa87dfaad0d558c3b1a83..dd4eb34e9d00c84b74c85ae8ced6890ef3dee313 100644 (file)
--- a/src/global.rs
+++ b/src/global.rs
@@ -50,11 +50,11 @@ pub struct Instance {
 
 pub struct PlayerRecord {
   pub u: PlayerUpdates,
-  pub pst: PlayerState,
+  pub pst: IPlayerState,
 }
 
 #[derive(Debug,Clone,Serialize,Deserialize)]
-pub struct PlayerState {
+pub struct IPlayerState {
   pub acctid: AccountId,
   pub tz: Timezone,
 }
@@ -180,15 +180,26 @@ lazy_static! {
 
 #[derive(Default)]
 pub struct Global {
-  // lock hierarchy: games < InstanceContainer < {players, clients}
-  // (in order of criticality (perf impact); outermost first, innermost last)
+  // lock hierarchy: all of these are in order of acquisition
+  // (in order of lock acquisition (L first), so also in order of criticality
+  // (perf impact); outermost first, innermost last)
+
+  // slow global locks:
+  save_area_lock : Mutex<Option<File>>,
+  // <- accounts::accounts ->
   games   : RwLock<HashMap<Arc<InstanceName>,InstanceRef>>,
+
+  // per-game lock:
+  // <- InstanceContainer ->
+  pub shapelibs : RwLock<shapelib::Registry>,
+
+  // inner locks which the game needs:
+  dirty   : Mutex<VecDeque<InstanceRef>>,
+  config  : RwLock<Arc<ServerConfig>>,
+
+  // fast global lookups
   players : RwLock<TokenTable<PlayerId>>,
   clients : RwLock<TokenTable<ClientId>>,
-  config  : RwLock<Arc<ServerConfig>>,
-  dirty   : Mutex<VecDeque<InstanceRef>>,
-  save_area_lock : Mutex<Option<File>>,
-  pub shapelibs : RwLock<shapelib::Registry>,
 }
 
 #[derive(Debug)]
@@ -203,7 +214,7 @@ pub struct InstanceContainer {
 struct InstanceSaveAccesses<RawTokenStr, PiecesLoadedRef> {
   ipieces: PiecesLoadedRef,
   tokens_players: Vec<(RawTokenStr, PlayerId)>,
-  aplayers: SecondarySlotMap<PlayerId, PlayerState>,
+  aplayers: SecondarySlotMap<PlayerId, IPlayerState>,
   acl: Acl<TablePermission>,
 }
 
@@ -417,7 +428,7 @@ impl InstanceGuard<'_> {
   /// caller is responsible for logging; threading it through
   /// proves the caller has a log entry.
   #[throws(MgmtError)]
-  pub fn player_new(&mut self, newnick: String, newplayer: PlayerState,
+  pub fn player_new(&mut self, newnick: String, newplayer: IPlayerState,
                     logentry: LogEntry) -> (PlayerId, LogEntry) {
     // saving is fallible, but we can't attempt to save unless
     // we have a thing to serialise with the player in it
@@ -476,7 +487,7 @@ impl InstanceGuard<'_> {
   //  #[throws(ServerFailure)]
   //  https://github.com/withoutboats/fehler/issues/62
   pub fn player_remove(&mut self, oldplayer: PlayerId)
-                       -> Result<(Option<String>, Option<PlayerState>),
+                       -> Result<(Option<String>, Option<IPlayerState>),
                                  InternalError> {
     // We have to filter this player out of everything
     // Then save

diff --git a/src/session.rs b/src/session.rs
index b647d742796c6af12f40616602db532ec27855b9..33de1c4b4ae64cdc97d52ab20323ddec44d651f2 100644 (file)
--- a/src/session.rs
+++ b/src/session.rs
@@ -94,7 +94,7 @@ fn session(form : Json<SessionForm>) -> Result<Template,OE> {
       });
     }
 
-    let pl = ig.gs.players.byid_mut(player)?;
+    let gpl = ig.gs.players.byid_mut(player)?;
     let ipl = ig.iplayers.byid(player)?;
     let tz = &ipl.pst.tz;
     let mut pieces : Vec<_> = ig.gs.pieces.iter().collect();
@@ -139,7 +139,7 @@ fn session(form : Json<SessionForm>) -> Result<Template,OE> {
       player,
       defs : alldefs,
       uses,
-      nick : ipl.pst.nick.clone(),
+      nick : gpl.nick.clone(),
       load : serde_json::to_string(&DataLoad {
         players : load_players,
       }).map_err(|e| InternalError::JSONEncode(e))?,

diff --git a/src/spec.rs b/src/spec.rs
index 63a389d1632292a62a92b07cf2e0a046ec4a2a41..aca807967e671b39671e4c4d50ed19a8d8626956 100644 (file)
--- a/src/spec.rs
+++ b/src/spec.rs
@@ -276,12 +276,16 @@ pub mod implementation {
       // xxx check this on setting access
       None
     }
-    fn server_deliver<'t>(&self, pst: &PlayerState,
+    fn server_deliver<'t>(&self,
+                          ipl: &IPlayerState,
+                          gpl: &GPlayerState,
                           token: &'t AccessTokenReport)
                           -> Result<Option<&'t AccessTokenReport>, TDE> {
       Ok(None)
     }
-    fn client_deliver(&self, pst: &PlayerState, token: &AccessTokenReport)
+    fn client_deliver(&self,
+                      pd: &MgmtPlayerDetails,
+                      token: &AccessTokenReport)
                       -> Result<(), TDE> {
       panic!()
     }
@@ -305,15 +309,19 @@ pub mod implementation {
   #[typetag::serde]
   impl PlayerAccessSpec for UrlOnStdout {
     #[throws(TDE)]
-    fn server_deliver<'t>(&self, ps: &PlayerState,
+    fn server_deliver<'t>(&self,
+                          ipl: &IPlayerState,
+                          gpl: &GPlayerState,
                           token: &'t AccessTokenReport)
                           -> Option<&'t AccessTokenReport> {
       Some(token)
     }
     #[throws(TDE)]
-    fn client_deliver(&self, ps: &PlayerState, token: &AccessTokenReport) {
+    fn client_deliver(&self,
+                      pd: &MgmtPlayerDetails,
+                      token: &AccessTokenReport) {
       println!("access account={} nick={:?} url:\n{}",
-               &ps.account, &ps.nick, token.url);
+               &pd.account, &pd.nick, token.url);
     }
   }