From 83420a40007677f6ca1b3fb27c95c6686fc9c554 Mon Sep 17 00:00:00 2001
From: Zeke Foppa <github.com/bfops>
Date: Wed, 5 Jun 2024 11:51:13 -0700
Subject: [PATCH 1/9] [bfops/fix-config-lock]: do thing

---
 crates/cli/src/config.rs                   | 75 +++++-----------------
 crates/cli/src/lib.rs                      |  6 +-
 crates/cli/src/subcommands/build.rs        |  5 +-
 crates/cli/src/subcommands/generate/mod.rs |  5 +-
 crates/cli/src/subcommands/init.rs         |  5 +-
 crates/cli/src/subcommands/local.rs        |  5 +-
 crates/cli/src/subcommands/logs.rs         |  1 -
 crates/cli/src/subcommands/upgrade.rs      |  5 +-
 crates/cli/src/subcommands/version.rs      |  5 +-
 9 files changed, 23 insertions(+), 89 deletions(-)

diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs
index 99e05a8e01b..4360c442aab 100644
--- a/crates/cli/src/config.rs
+++ b/crates/cli/src/config.rs
@@ -3,12 +3,13 @@ use anyhow::Context;
 use jsonwebtoken::DecodingKey;
 use serde::{Deserialize, Serialize};
 use spacetimedb::auth::identity::decode_token;
-use spacetimedb_fs_utils::{create_parent_dir, lockfile::Lockfile};
+use spacetimedb_fs_utils::create_parent_dir;
 use spacetimedb_lib::Identity;
 use std::{
     fs,
     path::{Path, PathBuf},
 };
+use tempfile::NamedTempFile;
 
 #[derive(Serialize, Deserialize, Clone, Debug)]
 pub struct IdentityConfig {
@@ -109,20 +110,6 @@ pub struct RawConfig {
 pub struct Config {
     proj: RawConfig,
     home: RawConfig,
-
-    /// The path from which we loaded the system config `home`,
-    /// and a [`Lockfile`] which guarantees us exclusive access to it.
-    ///
-    /// The lockfile is created before reading or creating the home config,
-    /// and closed upon dropping the config.
-    /// This allows us to mutate the config via [`Config::save`] without worrying about other processes.
-    ///
-    /// None only in tests, which are allowed to concurrently mutate configurations as much as they want,
-    /// since they use a hardcoded configuration rather than loading from a file.
-    ///
-    /// Note that it's not necessary to lock or remember the path of the project config,
-    /// since we never write to that file.
-    home_file: Option<(PathBuf, Lockfile)>,
 }
 
 const HOME_CONFIG_DIR: &str = ".spacetime";
@@ -594,21 +581,16 @@ Fetch the server's fingerprint with:
     }
 }
 
-impl Config {
-    /// Release the system configuration `Lockfile` contained in `self`, if it exists,
-    /// allowing other processes to access the configuration.
-    ///
-    /// This is equivalent to dropping `self`, but more explicit in purpose.
-    pub fn release_lock(self) {
-        // Just drop `self`, cleaning up its lockfile.
-        //
-        // If we had CLI subcommands which wanted to read a `Config` but never `Config::save` it,
-        // we could have this message take `&mut self` and set the `home_lock` to `None`.
-        // This seems unlikely to be useful,
-        // as many accesses to the `Config` will implicitly mutate and `save`,
-        // e.g. by creating a new identity if none exists.
-    }
+fn atomic_write(file_path: &PathBuf, data: String) -> anyhow::Result<()> {
+    let temp_file = NamedTempFile::new()?;
+    // Close the file, but keep the path to it around.
+    let temp_path = temp_file.into_temp_path();
+    std::fs::write(&temp_path, data)?;
+    std::fs::rename(&temp_path, file_path)?;
+    Ok(())
+}
 
+impl Config {
     pub fn default_server_name(&self) -> Option<&str> {
         self.proj
             .default_server
@@ -850,7 +832,6 @@ impl Config {
 
     pub fn load() -> Self {
         let home_path = Self::system_config_path();
-        let home_lock = Lockfile::for_file(&home_path).unwrap();
         let home = if home_path.exists() {
             Self::load_from_file(&home_path)
                 .inspect_err(|e| eprintln!("config file {home_path:?} is invalid: {e:#?}"))
@@ -861,50 +842,26 @@ impl Config {
 
         let proj = Self::load_proj_config();
 
-        Self {
-            home,
-            proj,
-
-            home_file: Some((home_path, home_lock)),
-        }
+        Self { home, proj }
     }
 
     #[doc(hidden)]
-    /// Used in tests; not backed by a file.
+    /// Used in tests.
     pub fn new_with_localhost() -> Self {
         Self {
             home: RawConfig::new_with_localhost(),
             proj: RawConfig::default(),
-
-            home_file: None,
-        }
-    }
-
-    #[doc(hidden)]
-    pub fn clone_for_test(&self) -> Self {
-        assert!(
-            self.home_file.is_none(),
-            "Cannot clone_for_test a Config derived from file {:?}",
-            self.home_file,
-        );
-        Config {
-            home: self.home.clone(),
-            proj: self.proj.clone(),
-            home_file: None,
         }
     }
 
     pub fn save(&self) {
-        let Some((home_path, _)) = &self.home_file else {
-            return;
-        };
-
+        let home_path = Self::system_config_path();
         // If the `home_path` is in a directory, ensure it exists.
-        create_parent_dir(home_path).unwrap();
+        create_parent_dir(home_path.as_ref()).unwrap();
 
         let config = toml::to_string_pretty(&self.home).unwrap();
 
-        if let Err(e) = std::fs::write(home_path, config) {
+        if let Err(e) = atomic_write(&home_path, config) {
             eprintln!("could not save config file: {e}")
         }
     }
diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs
index dbea04ee3ba..3d5039c34bb 100644
--- a/crates/cli/src/lib.rs
+++ b/crates/cli/src/lib.rs
@@ -59,11 +59,7 @@ pub async fn exec_subcommand(config: Config, cmd: &str, args: &ArgMatches) -> Re
         "build" => build::exec(config, args).await,
         "server" => server::exec(config, args).await,
         #[cfg(feature = "standalone")]
-        "start" => {
-            // Release the lockfile on the config, since we don't need it.
-            config.release_lock();
-            start::exec(args).await
-        }
+        "start" => start::exec(args).await,
         "upgrade" => upgrade::exec(config, args).await,
         unknown => Err(anyhow::anyhow!("Invalid subcommand: {}", unknown)),
     }
diff --git a/crates/cli/src/subcommands/build.rs b/crates/cli/src/subcommands/build.rs
index a8e1fdeca2e..0e45f15aa84 100644
--- a/crates/cli/src/subcommands/build.rs
+++ b/crates/cli/src/subcommands/build.rs
@@ -32,10 +32,7 @@ pub fn cli() -> clap::Command {
         )
 }
 
-pub async fn exec(config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
-    // Release the lockfile on the config, since we don't need it.
-    config.release_lock();
-
+pub async fn exec(_config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
     let project_path = args.get_one::<PathBuf>("project_path").unwrap();
     let skip_clippy = args.get_flag("skip_clippy");
     let build_debug = args.get_flag("debug");
diff --git a/crates/cli/src/subcommands/generate/mod.rs b/crates/cli/src/subcommands/generate/mod.rs
index 897053f685d..9d0dac5dc0a 100644
--- a/crates/cli/src/subcommands/generate/mod.rs
+++ b/crates/cli/src/subcommands/generate/mod.rs
@@ -100,10 +100,7 @@ pub fn cli() -> clap::Command {
         .after_help("Run `spacetime help publish` for more detailed information.")
 }
 
-pub fn exec(config: Config, args: &clap::ArgMatches) -> anyhow::Result<()> {
-    // Release the lockfile on the config, since we don't need it.
-    config.release_lock();
-
+pub fn exec(_config: Config, args: &clap::ArgMatches) -> anyhow::Result<()> {
     let project_path = args.get_one::<PathBuf>("project_path").unwrap();
     let wasm_file = args.get_one::<PathBuf>("wasm_file").cloned();
     let out_dir = args.get_one::<PathBuf>("out_dir").unwrap();
diff --git a/crates/cli/src/subcommands/init.rs b/crates/cli/src/subcommands/init.rs
index 7a410fb7632..da054a0418d 100644
--- a/crates/cli/src/subcommands/init.rs
+++ b/crates/cli/src/subcommands/init.rs
@@ -112,10 +112,7 @@ fn check_for_git() -> bool {
     false
 }
 
-pub async fn exec(config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
-    // Release the lockfile on the config, since we don't need it.
-    config.release_lock();
-
+pub async fn exec(_config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
     let project_path = args.get_one::<PathBuf>("project-path").unwrap();
     let project_lang = *args.get_one::<ModuleLanguage>("lang").unwrap();
 
diff --git a/crates/cli/src/subcommands/local.rs b/crates/cli/src/subcommands/local.rs
index 5c028f0ed4f..b9240ec22e0 100644
--- a/crates/cli/src/subcommands/local.rs
+++ b/crates/cli/src/subcommands/local.rs
@@ -37,10 +37,7 @@ async fn exec_subcommand(config: Config, cmd: &str, args: &ArgMatches) -> Result
     }
 }
 
-async fn exec_clear(config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
-    // Release the lockfile on the config, since we don't need it.
-    config.release_lock();
-
+async fn exec_clear(_config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
     let force = args.get_flag("force");
     if std::env::var_os("STDB_PATH").map(PathBuf::from).is_none() {
         let mut path = dirs::home_dir().unwrap_or_default();
diff --git a/crates/cli/src/subcommands/logs.rs b/crates/cli/src/subcommands/logs.rs
index 4d54a1140a2..405378dd34c 100644
--- a/crates/cli/src/subcommands/logs.rs
+++ b/crates/cli/src/subcommands/logs.rs
@@ -114,7 +114,6 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E
     let query_parms = LogsParams { num_lines, follow };
 
     let host_url = config.get_host_url(server)?;
-    config.release_lock();
 
     let builder = reqwest::Client::new().get(format!("{}/database/logs/{}", host_url, address));
     let builder = add_auth_header_opt(builder, &auth_header);
diff --git a/crates/cli/src/subcommands/upgrade.rs b/crates/cli/src/subcommands/upgrade.rs
index 35435843252..6af1fea1066 100644
--- a/crates/cli/src/subcommands/upgrade.rs
+++ b/crates/cli/src/subcommands/upgrade.rs
@@ -121,10 +121,7 @@ async fn download_with_progress(client: &reqwest::Client, url: &str, temp_path:
     Ok(())
 }
 
-pub async fn exec(config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
-    // Release the lockfile on the config, since we don't need it.
-    config.release_lock();
-
+pub async fn exec(_config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
     let version = args.get_one::<String>("version");
     let current_exe_path = env::current_exe()?;
     let force = args.get_flag("force");
diff --git a/crates/cli/src/subcommands/version.rs b/crates/cli/src/subcommands/version.rs
index a77a2243bbd..646cdda85a4 100644
--- a/crates/cli/src/subcommands/version.rs
+++ b/crates/cli/src/subcommands/version.rs
@@ -17,10 +17,7 @@ pub fn cli() -> clap::Command {
         )
 }
 
-pub async fn exec(config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
-    // Release the lockfile on the config, since we don't need it.
-    config.release_lock();
-
+pub async fn exec(_config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
     if args.get_flag("cli") {
         println!("{}", CLI_VERSION);
         return Ok(());

From 4b7155482e817a140bef82f897981452b8469fac Mon Sep 17 00:00:00 2001
From: Zeke Foppa <github.com/bfops>
Date: Wed, 5 Jun 2024 12:57:15 -0700
Subject: [PATCH 2/9] [bfops/fix-config-lock]: review

---
 Cargo.lock                 |  2 ++
 crates/cli/src/config.rs   | 12 +-----------
 crates/fs-utils/Cargo.toml |  2 ++
 crates/fs-utils/src/lib.rs | 12 +++++++++++-
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 38a9e7f9953..79bd363046b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4463,8 +4463,10 @@ dependencies = [
 name = "spacetimedb-fs-utils"
 version = "0.9.3"
 dependencies = [
+ "anyhow",
  "hex",
  "tempdir",
+ "tempfile",
  "thiserror",
 ]
 
diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs
index 4360c442aab..2f5d52fccfb 100644
--- a/crates/cli/src/config.rs
+++ b/crates/cli/src/config.rs
@@ -3,13 +3,12 @@ use anyhow::Context;
 use jsonwebtoken::DecodingKey;
 use serde::{Deserialize, Serialize};
 use spacetimedb::auth::identity::decode_token;
-use spacetimedb_fs_utils::create_parent_dir;
+use spacetimedb_fs_utils::{atomic_write, create_parent_dir};
 use spacetimedb_lib::Identity;
 use std::{
     fs,
     path::{Path, PathBuf},
 };
-use tempfile::NamedTempFile;
 
 #[derive(Serialize, Deserialize, Clone, Debug)]
 pub struct IdentityConfig {
@@ -581,15 +580,6 @@ Fetch the server's fingerprint with:
     }
 }
 
-fn atomic_write(file_path: &PathBuf, data: String) -> anyhow::Result<()> {
-    let temp_file = NamedTempFile::new()?;
-    // Close the file, but keep the path to it around.
-    let temp_path = temp_file.into_temp_path();
-    std::fs::write(&temp_path, data)?;
-    std::fs::rename(&temp_path, file_path)?;
-    Ok(())
-}
-
 impl Config {
     pub fn default_server_name(&self) -> Option<&str> {
         self.proj
diff --git a/crates/fs-utils/Cargo.toml b/crates/fs-utils/Cargo.toml
index 9e21a90ccf8..7968bca0cf9 100644
--- a/crates/fs-utils/Cargo.toml
+++ b/crates/fs-utils/Cargo.toml
@@ -7,8 +7,10 @@ license-file = "LICENSE"
 description = "Assorted utilities for filesystem operations used in SpacetimeDB"
 
 [dependencies]
+anyhow.workspace = true
 thiserror.workspace = true
 hex.workspace = true
+tempfile.workspace = true
 
 [dev-dependencies]
 tempdir.workspace = true
diff --git a/crates/fs-utils/src/lib.rs b/crates/fs-utils/src/lib.rs
index 2123b3808f5..6827b848112 100644
--- a/crates/fs-utils/src/lib.rs
+++ b/crates/fs-utils/src/lib.rs
@@ -1,4 +1,5 @@
-use std::path::Path;
+use std::path::{Path, PathBuf};
+use tempfile::NamedTempFile;
 
 pub mod dir_trie;
 pub mod lockfile;
@@ -31,3 +32,12 @@ pub fn create_parent_dir(file: &Path) -> Result<(), std::io::Error> {
     // If `parent` already exists as a directory, this is a no-op.
     std::fs::create_dir_all(parent)
 }
+
+pub fn atomic_write(file_path: &PathBuf, data: String) -> anyhow::Result<()> {
+    let temp_file = NamedTempFile::new()?;
+    // Close the file, but keep the path to it around.
+    let temp_path = temp_file.into_temp_path();
+    std::fs::write(&temp_path, data)?;
+    std::fs::rename(&temp_path, file_path)?;
+    Ok(())
+}

From b86d15a9010a4d4370a8d0a74aa8fc9cbf23d022 Mon Sep 17 00:00:00 2001
From: Zeke Foppa <github.com/bfops>
Date: Wed, 5 Jun 2024 13:04:50 -0700
Subject: [PATCH 3/9] [bfops/fix-config-lock]: review

---
 crates/testing/src/lib.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/testing/src/lib.rs b/crates/testing/src/lib.rs
index 77cc202d553..d58e0268cb6 100644
--- a/crates/testing/src/lib.rs
+++ b/crates/testing/src/lib.rs
@@ -34,6 +34,6 @@ pub fn invoke_cli(args: &[&str]) {
     let (cmd, args) = args.subcommand().expect("Could not split subcommand and args");
 
     RUNTIME
-        .block_on(spacetimedb_cli::exec_subcommand((*CONFIG).clone_for_test(), cmd, args))
+        .block_on(spacetimedb_cli::exec_subcommand((*CONFIG).clone(), cmd, args))
         .unwrap()
 }

From 487e080d65a109b2e46f6a301ccec247cc356b28 Mon Sep 17 00:00:00 2001
From: Zeke Foppa <github.com/bfops>
Date: Wed, 5 Jun 2024 13:06:05 -0700
Subject: [PATCH 4/9] [bfops/fix-config-lock]: fix

---
 crates/cli/src/config.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs
index 2f5d52fccfb..f784d150c4c 100644
--- a/crates/cli/src/config.rs
+++ b/crates/cli/src/config.rs
@@ -106,6 +106,7 @@ pub struct RawConfig {
     server_configs: Vec<ServerConfig>,
 }
 
+#[derive(Debug, Clone)]
 pub struct Config {
     proj: RawConfig,
     home: RawConfig,

From 51a44489c93b94012fde5be1b5597500d3175fa5 Mon Sep 17 00:00:00 2001
From: Zeke Foppa <github.com/bfops>
Date: Wed, 5 Jun 2024 15:13:33 -0700
Subject: [PATCH 5/9] [bfops/fix-config-lock]: TODOs

---
 crates/cli/src/config.rs        | 11 +++++++++++
 crates/fs-utils/src/lockfile.rs |  4 ++++
 2 files changed, 15 insertions(+)

diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs
index f784d150c4c..6652cc9252a 100644
--- a/crates/cli/src/config.rs
+++ b/crates/cli/src/config.rs
@@ -852,6 +852,17 @@ impl Config {
 
         let config = toml::to_string_pretty(&self.home).unwrap();
 
+        // TODO: We currently have a race condition if multiple processes are modifying the config.
+        // If process X and process Y read the config, each make independent changes, and then save
+        // the config, the first writer will have its changes clobbered by the second writer.
+        //
+        // We used to use `Lockfile` to prevent this from happening, but we had other issues with
+        // that approach (see https://github.com/clockworklabs/SpacetimeDB/issues/1339).
+        //
+        // We should eventually reintroduce `Lockfile`, with further fixes including OS locks (see
+        // the TODO in `lockfile.rs`). In a perfect world, we could distinguish a read lock from a
+        // write lock, so that multiple parallel processes could continue to read the config file,
+        // which the current `Lockfile` does not allow.
         if let Err(e) = atomic_write(&home_path, config) {
             eprintln!("could not save config file: {e}")
         }
diff --git a/crates/fs-utils/src/lockfile.rs b/crates/fs-utils/src/lockfile.rs
index 8f3b474faf0..c5bcf408cb6 100644
--- a/crates/fs-utils/src/lockfile.rs
+++ b/crates/fs-utils/src/lockfile.rs
@@ -37,6 +37,10 @@ impl Lockfile {
     ///
     /// `file_path` should be the full path of the file to which to acquire exclusive access.
     pub fn for_file(file_path: &Path) -> Result<Self, LockfileError> {
+        // TODO: Someday, it would be nice to use OS locks to minimize edge cases.
+        // Currently, our files can be left around if a process is unceremoniously killed (most
+        // commonly with Ctrl-C, but this would also apply to e.g. power failure).
+        // See https://github.com/clockworklabs/SpacetimeDB/pull/1341#issuecomment-2151018992.
         let path = Self::lock_path(file_path);
 
         let fail = |cause| LockfileError::Acquire {

From 86c0730dcc5811d63b6e47b9b1c7115838a347cd Mon Sep 17 00:00:00 2001
From: Zeke Foppa <github.com/bfops>
Date: Wed, 5 Jun 2024 15:18:54 -0700
Subject: [PATCH 6/9] [bfops/fix-config-lock]: review

---
 crates/cli/src/config.rs        | 9 +++++----
 crates/fs-utils/src/lockfile.rs | 6 ++++--
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs
index 6652cc9252a..d6b67657b0b 100644
--- a/crates/cli/src/config.rs
+++ b/crates/cli/src/config.rs
@@ -858,11 +858,12 @@ impl Config {
         //
         // We used to use `Lockfile` to prevent this from happening, but we had other issues with
         // that approach (see https://github.com/clockworklabs/SpacetimeDB/issues/1339).
-        //
         // We should eventually reintroduce `Lockfile`, with further fixes including OS locks (see
-        // the TODO in `lockfile.rs`). In a perfect world, we could distinguish a read lock from a
-        // write lock, so that multiple parallel processes could continue to read the config file,
-        // which the current `Lockfile` does not allow.
+        // the TODO in `lockfile.rs`).
+        //
+        // (In a perfect world, we would also distinguish a read lock from a write lock, so that
+        // multiple parallel processes could read the config file. The current `Lockfile` doesn't
+        // allow this).
         if let Err(e) = atomic_write(&home_path, config) {
             eprintln!("could not save config file: {e}")
         }
diff --git a/crates/fs-utils/src/lockfile.rs b/crates/fs-utils/src/lockfile.rs
index c5bcf408cb6..f77828d9efb 100644
--- a/crates/fs-utils/src/lockfile.rs
+++ b/crates/fs-utils/src/lockfile.rs
@@ -37,10 +37,12 @@ impl Lockfile {
     ///
     /// `file_path` should be the full path of the file to which to acquire exclusive access.
     pub fn for_file(file_path: &Path) -> Result<Self, LockfileError> {
-        // TODO: Someday, it would be nice to use OS locks to minimize edge cases.
+        // TODO: Someday, it would be nice to use OS locks to minimize edge cases (see
+        // https://github.com/clockworklabs/SpacetimeDB/pull/1341#issuecomment-2151018992).
+        //
         // Currently, our files can be left around if a process is unceremoniously killed (most
         // commonly with Ctrl-C, but this would also apply to e.g. power failure).
-        // See https://github.com/clockworklabs/SpacetimeDB/pull/1341#issuecomment-2151018992.
+        // See https://github.com/clockworklabs/SpacetimeDB/issues/1339.
         let path = Self::lock_path(file_path);
 
         let fail = |cause| LockfileError::Acquire {

From bdab136a884c57c191d362349bafeaa0cd369f63 Mon Sep 17 00:00:00 2001
From: Zeke Foppa <github.com/bfops>
Date: Wed, 5 Jun 2024 15:20:35 -0700
Subject: [PATCH 7/9] [bfops/fix-config-lock]: review

---
 crates/cli/src/config.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs
index d6b67657b0b..b85e5fbe0c2 100644
--- a/crates/cli/src/config.rs
+++ b/crates/cli/src/config.rs
@@ -858,12 +858,12 @@ impl Config {
         //
         // We used to use `Lockfile` to prevent this from happening, but we had other issues with
         // that approach (see https://github.com/clockworklabs/SpacetimeDB/issues/1339).
-        // We should eventually reintroduce `Lockfile`, with further fixes including OS locks (see
-        // the TODO in `lockfile.rs`).
+        // We should eventually reintroduce `Lockfile`, with further fixes (see the TODO in
+        // `lockfile.rs`).
         //
-        // (In a perfect world, we would also distinguish a read lock from a write lock, so that
+        // In a perfect world, we would also distinguish a read lock from a write lock, so that
         // multiple parallel processes could read the config file. The current `Lockfile` doesn't
-        // allow this).
+        // allow this, but it's not the end of the world.
         if let Err(e) = atomic_write(&home_path, config) {
             eprintln!("could not save config file: {e}")
         }

From 62dfd548e2cb779c7f52d9119e7cc56e4b662005 Mon Sep 17 00:00:00 2001
From: Zeke Foppa <github.com/bfops>
Date: Wed, 5 Jun 2024 15:26:13 -0700
Subject: [PATCH 8/9] [bfops/fix-config-lock]: review

---
 crates/cli/src/config.rs | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs
index b85e5fbe0c2..99cbb8cd259 100644
--- a/crates/cli/src/config.rs
+++ b/crates/cli/src/config.rs
@@ -857,13 +857,11 @@ impl Config {
         // the config, the first writer will have its changes clobbered by the second writer.
         //
         // We used to use `Lockfile` to prevent this from happening, but we had other issues with
-        // that approach (see https://github.com/clockworklabs/SpacetimeDB/issues/1339).
-        // We should eventually reintroduce `Lockfile`, with further fixes (see the TODO in
-        // `lockfile.rs`).
+        // that approach (see https://github.com/clockworklabs/SpacetimeDB/issues/1339, and the
+        // TODO in `lockfile.rs`).
         //
-        // In a perfect world, we would also distinguish a read lock from a write lock, so that
-        // multiple parallel processes could read the config file. The current `Lockfile` doesn't
-        // allow this, but it's not the end of the world.
+        // We should address this issue, but we currently don't expect it to arise very frequently.
+        // (https://github.com/clockworklabs/SpacetimeDB/pull/1341#issuecomment-2150857432)
         if let Err(e) = atomic_write(&home_path, config) {
             eprintln!("could not save config file: {e}")
         }

From 09c23f2da39b295dfe76a3a88f244ee41712c654 Mon Sep 17 00:00:00 2001
From: Zeke Foppa <github.com/bfops>
Date: Wed, 5 Jun 2024 15:28:03 -0700
Subject: [PATCH 9/9] [bfops/fix-config-lock]: review

---
 crates/cli/src/config.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs
index 99cbb8cd259..ae541409d7c 100644
--- a/crates/cli/src/config.rs
+++ b/crates/cli/src/config.rs
@@ -860,8 +860,8 @@ impl Config {
         // that approach (see https://github.com/clockworklabs/SpacetimeDB/issues/1339, and the
         // TODO in `lockfile.rs`).
         //
-        // We should address this issue, but we currently don't expect it to arise very frequently.
-        // (https://github.com/clockworklabs/SpacetimeDB/pull/1341#issuecomment-2150857432)
+        // We should address this issue, but we currently don't expect it to arise very frequently
+        // (see https://github.com/clockworklabs/SpacetimeDB/pull/1341#issuecomment-2150857432).
         if let Err(e) = atomic_write(&home_path, config) {
             eprintln!("could not save config file: {e}")
         }