authorgravatar for git@paperclover.netclover caruso <git@paperclover.net> 2026-10-01 10:54:48-07:00
committergravatar for git@paperclover.netclover caruso <git@paperclover.net> 2026-10-01 11:07:13-07:00
log932ef789ec44c35ed71fda1ec07adcd7365eb416
tree91eda547cbe0830d5cfbb7910755c7cd14c4c61d
parent23a1ac633199364044fafb816fa6026524ff62f7
signature Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU

fix: open a section's cache another thread created meanwhile

A section opening as its notebook first opened could fail with "File exists": the cache was checked, then created, while the background made the same section's offline copy. Replica::open_or_create opens a cache created meanwhile instead, and every opening path goes through it. Assisted-by: claude-opus-5.5

5 files changed, 49 insertions(+), 22 deletions(-)

crates/mobile/src/library.rs+10-9
...@@ -603,15 +603,16 @@ impl Library {...@@ -603,15 +603,16 @@ impl Library {
603 .ok_or("The notebook hasn’t listed this section")?;603 .ok_or("The notebook hasn’t listed this section")?;
604 let replicas = self.share_replicas().ok_or("Not a share")?;604 let replicas = self.share_replicas().ok_or("Not a share")?;
605 let cache = replicas.join(format!("{identity}.sqlite"));605 let cache = replicas.join(format!("{identity}.sqlite"));
606 let replica = if cache.exists() {606 std::fs::create_dir_all(&replicas)?;
607 Replica::open(&cache)?607 let replica = Replica::open_or_create(&cache, || {
608 } else {608 let client = self.client().ok_or_else(|| {
609 let client = self609 std::io::Error::new(
610 .client()610 std::io::ErrorKind::NotConnected,
611 .ok_or("The server can’t be reached, and this section hasn’t been opened here before.")?;611 "The server can’t be reached, and this section hasn’t been opened here before.",
612 std::fs::create_dir_all(&replicas)?;612 )
613 Replica::create(&cache, &client.read_storage(&file, LIMIT)?)?613 })?;
614 };614 Ok(client.read_storage(&file, LIMIT)?)
615 })?;
615 let server = Arc::clone(server);616 let server = Arc::clone(server);
616 Ok(session::Section::resume_smb(617 Ok(session::Section::resume_smb(
617 file,618 file,
crates/notebook/src/lib.rs+17
...@@ -128,6 +128,23 @@ impl Replica {...@@ -128,6 +128,23 @@ impl Replica {
128 Self::start(cache_connection(path.as_ref())?)128 Self::start(cache_connection(path.as_ref())?)
129 }129 }
130130
131 /// Opens the cache at `path`, first creating it from the image `source` reads where there
132 /// is none. A cache another thread creates meanwhile, as the background makes an offline
133 /// copy, is opened instead.
134 pub fn open_or_create(
135 path: impl AsRef<Path>,
136 source: impl FnOnce() -> Result<Vec<u8>>,
137 ) -> Result<Self> {
138 let path = path.as_ref();
139 if !path.exists() {
140 match Self::seed(path, &source()?) {
141 Err(Error::Io(error)) if error.kind() == io::ErrorKind::AlreadyExists => {}
142 seeded => seeded?,
143 }
144 }
145 Self::open(path)
146 }
147
131 /// `create` without opening the cache it made, as for an offline copy.148 /// `create` without opening the cache it made, as for an offline copy.
132 pub(crate) fn seed(path: &Path, source: &[u8]) -> Result<()> {149 pub(crate) fn seed(path: &Path, source: &[u8]) -> Result<()> {
133 validate(source)?;150 validate(source)?;
crates/notebook/src/session.rs+1-5
...@@ -1650,11 +1650,7 @@ impl Section {...@@ -1650,11 +1650,7 @@ impl Section {
1650 let store = Store::parse(&source)?;1650 let store = Store::parse(&source)?;
1651 let identity = RevisionIndex::parse(&store)?.root;1651 let identity = RevisionIndex::parse(&store)?.root;
1652 let cache = replica(&identity.guid, &source)?;1652 let cache = replica(&identity.guid, &source)?;
1653 let replica = if cache.exists() {1653 let replica = Replica::open_or_create(&cache, || Ok(source))?;
1654 Replica::open(&cache)?
1655 } else {
1656 Replica::create(&cache, &source)?
1657 };
1658 let remote = file.clone();1654 let remote = file.clone();
1659 Self::start(file, replica, move || connect(&remote), notify)1655 Self::start(file, replica, move || connect(&remote), notify)
1660 }1656 }
crates/notebook/tests/cache.rs+18
...@@ -59,6 +59,24 @@ fn typed(space: ExGuid, text: ExGuid, with: &str) -> Edit {...@@ -59,6 +59,24 @@ fn typed(space: ExGuid, text: ExGuid, with: &str) -> Edit {
59 }59 }
60}60}
6161
62/// A cache another thread creates while this one reads its section, as the background
63/// makes a section's offline copy while the section opens, is opened instead of refused.
64#[test]
65fn a_cache_created_meanwhile_opens() {
66 let source = onestore::create_section("Raced.one", "Raced", "Fixture").unwrap();
67 let directory = tempfile::tempdir().unwrap();
68 let path = directory.path().join("raced.sqlite");
69 let replica = Replica::open_or_create(&path, || {
70 drop(Replica::create(&path, &source)?);
71 Ok(source.clone())
72 })
73 .unwrap();
74 assert_eq!(replica.pages().unwrap()[0].1, "Raced");
75 drop(replica);
76 let reopened = Replica::open_or_create(&path, || unreachable!()).unwrap();
77 assert_eq!(reopened.pages().unwrap()[0].1, "Raced");
78}
79
62#[test]80#[test]
63fn cache_reopen_preserves_the_base_and_queued_edits() {81fn cache_reopen_preserves_the_base_and_queued_edits() {
64 let dir = tempfile::tempdir().unwrap();82 let dir = tempfile::tempdir().unwrap();
crates/snowbound/src/library.rs+3-8
...@@ -671,14 +671,9 @@ impl Library {...@@ -671,14 +671,9 @@ impl Library {
671 };671 };
672 let cache = notebook.replica_path(path)?;672 let cache = notebook.replica_path(path)?;
673 std::fs::create_dir_all(cache.parent().unwrap_or(&self.cache))?;673 std::fs::create_dir_all(cache.parent().unwrap_or(&self.cache))?;
674 let replica = if cache.exists() {674 let replica = notebook::Replica::open_or_create(&cache, || {
675 notebook::Replica::open(&cache)675 Ok(server.connect()?.read_storage(&file, LIMIT)?)
676 } else {676 });
677 notebook::Replica::create(
678 &cache,
679 &server.connect()?.read_storage(&file, LIMIT)?,
680 )
681 };
682 replica.and_then(|replica| {677 replica.and_then(|replica| {
683 let server = Arc::clone(server);678 let server = Arc::clone(server);
684 let connect = move || server.connect();679 let connect = move || server.connect();