From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Fri, 11 Dec 2020 23:25:20 +0000 (+0000)
Subject: config etc.: avoid accessing the save path without locking it
X-Git-Tag: otter-0.2.0~201
X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=0737c7eb1b2017926ce69bad9d0b48a01d7c2471;p=otter.git

config etc.: avoid accessing the save path without locking it

We panic instead!

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

diff --git a/src/accounts.rs b/src/accounts.rs
index 4eaea255..38fa32aa 100644
--- a/src/accounts.rs
+++ b/src/accounts.rs
@@ -401,7 +401,7 @@ impl AccountsGuard {
 //---------- load/save ----------
 
 fn save_path() -> String {
-  format!("{}/{}", config().save_dir, &ACCOUNTS_FILE)
+  format!("{}/{}", config().save_dir(), &ACCOUNTS_FILE)
 }
 
 #[throws(StartupError)]
diff --git a/src/bin/daemon-otter.rs b/src/bin/daemon-otter.rs
index b397c816..7241aea3 100644
--- a/src/bin/daemon-otter.rs
+++ b/src/bin/daemon-otter.rs
@@ -198,6 +198,7 @@ fn main() {
   nwtemplates::init()?;
   shapelib::load()?;
 
+  c.lock_save_area()?;
   load_accounts()?;
   load_games(&mut AccountsGuard::lock(), &mut games_lock())?;
 
diff --git a/src/config.rs b/src/config.rs
index 6cd59259..bb8bf4c9 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -36,7 +36,7 @@ pub struct ServerConfigSpec {
 
 #[derive(Debug,Clone)]
 pub struct ServerConfig {
-  pub save_dir: String,
+  save_dir: String,
   pub command_socket: String,
   pub debug: bool,
   pub http_port: Option<u16>,
@@ -151,6 +151,26 @@ impl ServerConfig {
     let config = config.try_into()?;
     set_config(config);
   }
+
+  #[throws(AE)]
+  pub fn lock_save_area(&self) {
+    let mut st = GLOBAL.save_area_lock.lock().unwrap();
+    let st = &mut *st;
+    if st.is_none() {
+      let lockfile = format!("{}/lock", config().save_dir);
+      *st = Some((||{
+        let file = File::create(&lockfile).context("open")?;
+        file.try_lock_exclusive().context("lock")?;
+        Ok::<_,AE>(file)
+      })().context(lockfile).context("lock global save area")?);
+    }
+  }
+
+  pub fn save_dir(&self) -> &String {
+    let st = GLOBAL.save_area_lock.lock().unwrap();
+    let mut _f : &File = st.as_ref().unwrap();
+    &self.save_dir
+  }
 }
 
 impl Default for ServerConfig {
diff --git a/src/global.rs b/src/global.rs
index 184c25e1..b957ac4f 100644
--- a/src/global.rs
+++ b/src/global.rs
@@ -187,7 +187,7 @@ pub struct Global {
   // (perf impact); outermost first, innermost last)
 
   // slow global locks:
-  save_area_lock: Mutex<Option<File>>,
+  pub save_area_lock: Mutex<Option<File>>,
   // <- accounts::accounts ->
   games_table: RwLock<GamesTable>,
 
@@ -831,7 +831,7 @@ enum SavefilenameParseResult {
 }
 
 fn savefilename(name: &InstanceName, prefix: &str, suffix: &str) -> String {
-  [ config().save_dir.as_str(), &"/", prefix ]
+  [ config().save_dir().as_str(), &"/", prefix ]
     .iter().map(Deref::deref)
     .chain(iter::once( name.to_string().as_str() ))
     .chain([ suffix ].iter().map(Deref::deref))
@@ -939,19 +939,6 @@ impl InstanceGuard<'_> {
   fn load_game(accounts: &AccountsGuard,
                games: &mut GamesGuard,
                name: InstanceName) -> Option<InstanceRef> {
-    {
-      let mut st = GLOBAL.save_area_lock.lock().unwrap();
-      let st = &mut *st;
-      if st.is_none() {
-        let lockfile = format!("{}/lock", config().save_dir);
-        *st = Some((||{
-          let file = File::create(&lockfile).context("open")?;
-          file.try_lock_exclusive().context("lock")?;
-          Ok::<_,AE>(file)
-        })().context(lockfile).context("lock global save area")?);
-      }
-    }
-
     let InstanceSaveAccesses::<String,ActualPiecesLoaded>
     { tokens_players, mut ipieces, mut aplayers, acl }
     = match Self::load_something(&name, "a-") {
@@ -1060,7 +1047,7 @@ pub fn load_games(accounts: &mut AccountsGuard,
   use AFState::*;
   use SavefilenameParseResult::*;
   let mut a_leaves = HashMap::new();
-  for de in fs::read_dir(&config().save_dir)? {
+  for de in fs::read_dir(&config().save_dir())? {
     let de = de?;
     let leaf = de.file_name();
     (||{