From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Tue, 28 Jul 2020 18:34:35 +0000 (+0100)
Subject: wip destroy game
X-Git-Tag: otter-0.2.0~1226
X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=896dc66f9376b400254782c32b3ed11752d5cf69;p=otter.git

wip destroy game
---

diff --git a/src/cmdlistener.rs b/src/cmdlistener.rs
index 54ed415c..62137214 100644
--- a/src/cmdlistener.rs
+++ b/src/cmdlistener.rs
@@ -230,7 +230,11 @@ fn execute(cs: &mut CommandStream, cmd: MgmtCommand) -> MgmtResponse {
 
       execute_for_game(cs, &mut ig, insns, MgmtGameUpdateMode::Bulk)
         .map_err(|e|{
-          Instance::destroy(ig);
+          let name = ig.name.clone();
+          Instance::destroy(ig)
+            .unwrap_or_else(|e|
+ eprintln!("failed to tidy up failecd creation of {:?}: {:?}",
+                                      &name, &e));
           e
         })?;
 
diff --git a/src/global.rs b/src/global.rs
index 435c1307..3fb6290c 100644
--- a/src/global.rs
+++ b/src/global.rs
@@ -147,30 +147,29 @@ impl Instance {
     };
 
     let gref = InstanceRef(Arc::new(Mutex::new(cont)));
+    let mut ig = gref.lock()?;
 
-    {
-      // access without save means it's deleted
-      // save without access menas no accesses
-      // we save first since that's more fallible
-      let mut ig = gref.lock()?;
-      ig.save_access_now()?;
-      ig.save_game_now()?;
-    }
+    // We hold the GLOBAL.games lock while we save the new game.
+    // That lock is not on the hot path.
+    let mut games = GLOBAL.games.write().unwrap();
+    let entry = games.entry(name);
 
-    {
-      let mut games = GLOBAL.games.write().unwrap();
-      let entry = games.entry(name);
+    use hash_map::Entry::*;
+    let entry = match entry {
+      Vacant(ve) => ve,
+      Occupied(_) => throw!(MgmtError::AlreadyExists),
+    };
 
-      use hash_map::Entry::*;
-      let entry = match entry {
-        Vacant(ve) => ve,
-        Occupied(_) => throw!(MgmtError::AlreadyExists),
-      };
+    // access without save means it's deleted
+    // save without access menas no accesses
+    ig.save_access_now()?;
+    ig.save_game_now()?;
 
+    (||{
       entry.insert(gref.clone());
-    }
+    })(); // <- No ?, ensures that IEFE is infallible (barring panics)
 
-    gref
+    ig.gref
   }
 
   #[throws(MgmtError)]
@@ -181,9 +180,14 @@ impl Instance {
       .clone()
   }
 
+  #[throws(MgmtError)]
   pub fn destroy(mut g: InstanceGuard) {
     g.c.live = false;
-    // remove the files
+    (||{
+      fs::remove_file(InstanceGuard::savefile(&g.name, "g-", ""))?;
+      fs::remove_file(InstanceGuard::savefile(&g.name, "a-", ""))?;
+      <Result<_,ServerFailure>>::Ok(())
+    })()?;
     GLOBAL.games.write().unwrap().remove(&g.name);
     InstanceGuard::forget_all_tokens(&mut g.tokens_players);
     InstanceGuard::forget_all_tokens(&mut g.tokens_clients);