From: Ian Jackson Date: Wed, 28 Oct 2020 23:36:25 +0000 (+0000) Subject: accounts: before change accounts locking arrangements X-Git-Tag: otter-0.2.0~584 X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=e3738af6c77f1642faf89dfc0e0c476e02e214da;p=otter.git accounts: before change accounts locking arrangements Signed-off-by: Ian Jackson --- diff --git a/src/api.rs b/src/api.rs index 72edb05f..353d1bba 100644 --- a/src/api.rs +++ b/src/api.rs @@ -18,7 +18,7 @@ struct ApiPiece { struct ApiPieceOpArgs<'a> { gs: &'a mut GameState, player: PlayerId, - pst: &'a PlayerState, + pst: &'a IPlayerState, piece: PieceId, p: &'a dyn Piece, iplayers: &'a SecondarySlotMap, diff --git a/src/cmdlistener.rs b/src/cmdlistener.rs index 4d5cb91a..376199e2 100644 --- 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 43dd12cb..65848b8b 100644 --- a/src/gamestate.rs +++ b/src/gamestate.rs @@ -46,7 +46,12 @@ pub struct GameState { pub gen : Generation, pub log : VecDeque<(Generation, Arc)>, pub max_z : ZCoord, - pub players : DenseSlotMap, + pub players : DenseSlotMap, +} + +#[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 8e5b1be5..dd4eb34e 100644 --- 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>, + // <- accounts::accounts -> games : RwLock,InstanceRef>>, + + // per-game lock: + // <- InstanceContainer -> + pub shapelibs : RwLock, + + // inner locks which the game needs: + dirty : Mutex>, + config : RwLock>, + + // fast global lookups players : RwLock>, clients : RwLock>, - config : RwLock>, - dirty : Mutex>, - save_area_lock : Mutex>, - pub shapelibs : RwLock, } #[derive(Debug)] @@ -203,7 +214,7 @@ pub struct InstanceContainer { struct InstanceSaveAccesses { ipieces: PiecesLoadedRef, tokens_players: Vec<(RawTokenStr, PlayerId)>, - aplayers: SecondarySlotMap, + aplayers: SecondarySlotMap, acl: Acl, } @@ -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, Option), + -> Result<(Option, Option), InternalError> { // We have to filter this player out of everything // Then save diff --git a/src/session.rs b/src/session.rs index b647d742..33de1c4b 100644 --- a/src/session.rs +++ b/src/session.rs @@ -94,7 +94,7 @@ fn session(form : Json) -> Result { }); } - 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) -> Result { 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 63a389d1..aca80796 100644 --- 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, 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); } }