From: Ian Jackson Date: Thu, 29 Oct 2020 00:01:54 +0000 (+0000) Subject: wip new accounts lock X-Git-Tag: otter-0.2.0~583 X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=f841271369b8f4c7d427d3f63bb651233bf7eff7;p=otter.git wip new accounts lock Signed-off-by: Ian Jackson --- diff --git a/src/accounts.rs b/src/accounts.rs index feb6e7e4..b7c60a0a 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -4,9 +4,9 @@ use crate::imports::*; -use parking_lot::{RwLock, const_rwlock, - RwLockReadGuard, RwLockWriteGuard, - MappedRwLockReadGuard, MappedRwLockWriteGuard}; +use parking_lot::{Mutex, const_mutex, + MutexGuard, + MappedMutexGuard}; slotmap::new_key_type!{ // #[derive(Serialize,Deserialize,Debug)] xxx @@ -164,7 +164,10 @@ struct Accounts { records: DenseSlotMap, } -static ACCOUNTS : RwLock> = const_rwlock(None); +static ACCOUNTS : Mutex> = const_mutex(None); + +#[derive(Debug)] +pub struct AccountsGuard (MutexGuard<'static, Accounts>); impl Deref for AccessRecord { type Target = Arc; @@ -174,7 +177,7 @@ impl Deref for AccessRecord { // xxx load, incl reveleation expiry // xxx periodic token reveleation expiry -pub fn save_accounts_now() -> Result<(), InternalError> { +pub fn save_accounts_now(_: &AccountsGuard) -> Result<(), InternalError> { panic!("xxx") } @@ -218,8 +221,11 @@ impl LookupHelper { } } -impl AccountRecord { - pub fn bulk_check(keys: &[K]) -> Vec> { +impl AccountsGuard { + pub fn bulk_check( + &self, + keys: &[K] + ) -> Vec> { let accounts = ACCOUNTS.read(); keys.iter().map(|&key|{ let accounts = accounts.as_ref()?; @@ -229,9 +235,11 @@ impl AccountRecord { }).collect() } - pub fn lookup(key: K, _: Authorisation) - -> Option<(MappedRwLockReadGuard<'static, AccountRecord>, - AccountId)> + pub fn lookup( + &self, + key: K, _: Authorisation + ) -> Option<(MappedMutexGuard<'static, AccountRecord>, + AccountId)> { let mut helper : LookupHelper = default(); helper.wrap( @@ -243,14 +251,15 @@ impl AccountRecord { ) } - pub fn lookup_mut_caller_must_save - ( key: &K, _auth: Authorisation) - -> Option<(MappedRwLockWriteGuard<'static, AccountRecord>, + pub fn lookup_mut_caller_must_save<'a, K: AccountNameOrId>( + self: &'a mut AccountsGuard, + key: &K, _auth: Authorisation + ) -> Option<(MappedMutexGuard<'a, AccountRecord>, AccountId)> { let mut helper : LookupHelper = default(); helper.wrap( - RwLockWriteGuard::try_map(ACCOUNTS.write(), |accounts| { + MutexGuard::try_map(ACCOUNTS.write(), |accounts| { let accounts = accounts.as_mut()?; let acctid = helper.get(accounts, key)?; accounts.records.get_mut(acctid) @@ -264,6 +273,7 @@ impl AccountRecord { K: AccountNameOrId, F: FnOnce(&mut AccountRecord, AccountId) -> T >( + &mut self, key: K, auth: Authorisation, set_access: Option>, @@ -271,8 +281,7 @@ impl AccountRecord { ) -> Result { - let (entry, acctid) - = AccountRecord::lookup_mut_caller_must_save(key, auth) + let (entry, acctid) = self.lookup_mut_caller_must_save(key, auth) .ok_or(MgmtError::AccountNotFound)?; if let Some(new_access) = set_access() { @@ -289,13 +298,13 @@ impl AccountRecord { } #[throws(MgmtError)] - pub fn insert_entry(account: AccountName, + pub fn insert_entry(&mut self, + account: AccountName, _auth: Authorisation, new_record: AccountRecord) { - let accounts = ACCOUNTS.write(); use hash_map::Entry::*; - let accounts = accounts.get_or_insert_with(default); + let accounts = self.get_or_insert_with(default); let name_entry = accounts.names.entry(account); if_chain!{ if let Occupied(oe) = name_entry; @@ -315,7 +324,8 @@ impl AccountRecord { } #[throws(MgmtError)] - pub fn remove_entry(account: &AccountName, + pub fn remove_entry(&mut self, + account: &AccountName, _auth: Authorisation) { let accounts = ACCOUNTS.write(); @@ -332,7 +342,9 @@ impl AccountRecord { accounts.records.remove(acctid); save_accounts_now()?; } +} +impl AccountRecord { pub fn expire_tokens_revealed(&mut self) { panic!("xxx") } diff --git a/src/api.rs b/src/api.rs index 353d1bba..90ab3a5b 100644 --- a/src/api.rs +++ b/src/api.rs @@ -18,7 +18,6 @@ struct ApiPiece { struct ApiPieceOpArgs<'a> { gs: &'a mut GameState, player: PlayerId, - pst: &'a IPlayerState, piece: PieceId, p: &'a dyn Piece, iplayers: &'a SecondarySlotMap, @@ -119,7 +118,7 @@ fn api_piece_op(form : Json>) let gs = &mut g.gs; let ipieces = &g.ipieces; let iplayers = &g.iplayers; - let pst = &iplayers.byid(player)?.pst; + let ipl = &iplayers.byid(player)?.ipl; let _ = gs.players.byid(player)?; let lens = TransparentLens { }; let piece = lens.decode_visible_pieceid(form.piece, player); @@ -138,7 +137,7 @@ fn api_piece_op(form : Json>) form.op.check_held(pc,player)?; let (wrc, update, logents) = form.op.op(ApiPieceOpArgs { - gs, player, pst, piece, iplayers, + gs, player, piece, iplayers, p: p.as_ref(), lens: &lens, })?; diff --git a/src/cmdlistener.rs b/src/cmdlistener.rs index 376199e2..3a5c498a 100644 --- a/src/cmdlistener.rs +++ b/src/cmdlistener.rs @@ -287,7 +287,7 @@ fn execute_game_insn<'ig>( account: add.account, tz, }; - let (player, logentry) = ig.player_new(st, tz, logentry)?; + let (player, logentry) = ig.player_new(gpl, ipl, tz, logentry)?; (U{ pcs: vec![], log: vec![ logentry ], raw: None }, diff --git a/src/global.rs b/src/global.rs index dd4eb34e..c27e6707 100644 --- a/src/global.rs +++ b/src/global.rs @@ -50,7 +50,7 @@ pub struct Instance { pub struct PlayerRecord { pub u: PlayerUpdates, - pub pst: IPlayerState, + pub ipl: IPlayerState, } #[derive(Debug,Clone,Serialize,Deserialize)] @@ -428,19 +428,19 @@ 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: IPlayerState, + pub fn player_new(&mut self, gnew: GPlayerState, inew: 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 - if self.c.g.gs.players.values().any(|oldnick| oldnick == &newnick) { + if self.c.g.gs.players.values().any(|old| old.nick == gnew.nick) { Err(MgmtError::NickCollision)?; } - if self.c.g.iplayers.values().any(|r| r.pst.acctid == newplayer.acctid) { + if self.c.g.iplayers.values().any(|r| r.ipl.acctid == inew.acctid) { Err(MgmtError::AlreadyExists)?; } - let player = self.c.g.gs.players.insert(newnick); + let player = self.c.g.gs.players.insert(gnew); let u = PlayerUpdates::new_begin(&self.c.g.gs).new(); - let record = PlayerRecord { u, pst: newplayer }; + let record = PlayerRecord { u, ipl: inew }; self.c.g.iplayers.insert(player, record); (||{ @@ -487,7 +487,7 @@ impl InstanceGuard<'_> { // #[throws(ServerFailure)] // https://github.com/withoutboats/fehler/issues/62 pub fn player_remove(&mut self, oldplayer: PlayerId) - -> Result<(Option, Option), + -> Result<(Option, Option), InternalError> { // We have to filter this player out of everything // Then save @@ -495,7 +495,7 @@ impl InstanceGuard<'_> { // We make a copy so if the save fails, we can put everything back let mut players = self.c.g.gs.players.clone(); - let old_nick = players.remove(oldplayer); + let old_gpl = players.remove(oldplayer); // New state let mut gs = GameState { @@ -548,7 +548,7 @@ impl InstanceGuard<'_> { // point of no return mem::drop(undo); - let old_pst = (||{ + let old_ipl = (||{ for &piece in &updated_pieces { (||Some({ self.c.g.gs.pieces.get_mut(piece)?.gen = self.c.g.gs.gen; @@ -566,16 +566,16 @@ impl InstanceGuard<'_> { self.remove_clients(oldplayer, ErrorSignaledViaUpdate::PlayerRemoved); self.tokens_deregister_for_id(|id:PlayerId| id==oldplayer); let iplayer = self.iplayers.remove(oldplayer); - let pst = iplayer.map(|iplayer| iplayer.pst); + let ipl = iplayer.map(|iplayer| iplayer.ipl); self.save_access_now().unwrap_or_else( |e| warn!( "trouble garbage collecting accesses for deleted player: {:?}", &e) ); - pst + ipl })(); // <- No ?, ensures that IEFE is infallible (barring panics) - Ok((old_nick, old_pst)) + Ok((old_gpl, old_ipl)) } #[throws(InternalError)] @@ -599,16 +599,20 @@ impl InstanceGuard<'_> { } #[throws(MgmtError)] - fn player_access_reset_redeliver(&mut self, player: PlayerId, + fn player_access_reset_redeliver(&mut self, + accounts: &mut AccountsGuard, + player: PlayerId, authorised: Authorisation, reset: bool) -> Option { - let pst = self.c.g.iplayers.get(player) + let ipl = self.c.g.iplayers.get(player) .ok_or(MgmtError::PlayerNotFound)? - .pst; + .ipl; + let gpl = self.c.g.gs.players.get(player) + .ok_or(MgmtError::PlayerNotFound)?; - let (access, acctid) = AccountRecord::with_entry_mut( - pst.acctid, authorised, None, + let (access, acctid) = accounts.with_entry_mut( + ipl.acctid, authorised, None, |acct, acctid| { let access = acct.access; @@ -678,23 +682,27 @@ impl InstanceGuard<'_> { url: format!("http://localhost:8000/{}", token.0), // xxx }; let report = access - .server_deliver(&pst, &report)?; + .server_deliver(&gpl, &ipl, &report)?; report.cloned() } #[throws(MgmtError)] - pub fn player_access_reset(&mut self, player: PlayerId, + pub fn player_access_reset(&mut self, + accounts: &mut AccountsGuard, + player: PlayerId, auth: Authorisation) -> Option { // xxx call this function when access method changes - self.player_access_reset_redeliver(player, auth, true)? + self.player_access_reset_redeliver(accounts, player, auth, true)? } #[throws(MgmtError)] - pub fn player_access_redeliver(&mut self, player: PlayerId, + pub fn player_access_redeliver(&mut self, + accounts: &mut AccountsGuard, + player: PlayerId, auth: Authorisation) -> Option { - self.player_access_reset_redeliver(player, auth, false)? + self.player_access_reset_redeliver(accounts, player, auth, false)? } pub fn modify_pieces(&mut self) -> ModifyingPieces { @@ -828,8 +836,8 @@ impl InstanceGuard<'_> { .collect() }; let aplayers = s.c.g.iplayers.iter().map( - |(player, PlayerRecord { pst, .. })| - (player, pst.clone()) + |(player, PlayerRecord { ipl, .. })| + (player, ipl.clone()) ).collect(); let acl = s.c.g.acl.into(); let isa = InstanceSaveAccesses { @@ -853,7 +861,8 @@ impl InstanceGuard<'_> { } #[throws(StartupError)] - fn load_game(name: InstanceName) -> Option { + fn load_game(accounts: &AccountsGuard, + name: InstanceName) -> Option { { let mut st = GLOBAL.save_area_lock.lock().unwrap(); let st = &mut *st; @@ -901,9 +910,9 @@ impl InstanceGuard<'_> { let iplayers : SecondarySlotMap = { let a = aplayers; a.drain() - }.map(|(player, pst)| { + }.map(|(player, ipl)| { let u = pu_bc.new(); - (player, PlayerRecord { u, pst }) + (player, PlayerRecord { u, ipl }) }).collect(); for mut p in gs.pieces.values_mut() { @@ -921,10 +930,10 @@ impl InstanceGuard<'_> { if let Some(record) = iplayers.get(player); then { tokens.push((token, player)); - acctids.push(record.pst.acctid); + acctids.push(record.ipl.acctid); } }} - (tokens, AccountRecord::bulk_check(&acctids)) + (tokens, accounts.bulk_check(&acctids)) }; let g = Instance { @@ -969,7 +978,7 @@ impl InstanceGuard<'_> { } #[throws(anyhow::Error)] -pub fn load_games() { +pub fn load_games(accounts: &mut AccountsGuard) { enum AFState { Found(PathBuf), Used }; use AFState::*; use SavefilenameParseResult::*; @@ -992,7 +1001,7 @@ pub fn load_games() { ); }, GameFile { access_leaf, name } => { - InstanceGuard::load_game(name)?; + InstanceGuard::load_game(accounts, name)?; a_leaves.insert(access_leaf, Used); }, } diff --git a/src/spec.rs b/src/spec.rs index aca80796..6ff27c9b 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -277,8 +277,8 @@ pub mod implementation { None } fn server_deliver<'t>(&self, - ipl: &IPlayerState, gpl: &GPlayerState, + ipl: &IPlayerState, token: &'t AccessTokenReport) -> Result, TDE> { Ok(None) @@ -310,8 +310,8 @@ pub mod implementation { impl PlayerAccessSpec for UrlOnStdout { #[throws(TDE)] fn server_deliver<'t>(&self, - ipl: &IPlayerState, gpl: &GPlayerState, + ipl: &IPlayerState, token: &'t AccessTokenReport) -> Option<&'t AccessTokenReport> { Some(token)