fix(cospan-node): remove blocking_lock from diff_commits async context The structural-diff helper was calling Mutex::blocking_lock from within the async diff_commits handler, which panics under tokio. The bug pre-dated this branch but only surfaced now that v0.29.0's clippy fix unblocked the Test job in CI. Refactor: take the lock once at the top of the handler, hand the loaded import marks and an FsStore handle into the synchronous helper, and run the rest of the body lock-free.
Author: Aaron Steven White
Commit
d741509605e4346f8afca8cca529b1a17b2658deParent: 483cb11af8
Structural diff unavailable
These commits were pushed via plain git push, so no pre-parsed
schemas are available. Install git-remote-cospan and re-push via panproto:// to
see scope-level changes, breaking change detection, and semantic diffs.
brew install panproto/tap/git-remote-cospan1 file changed +19 -19
@@ -54,6 +54,10 @@ pub async fn diff_commits(
5454 State(state): State<Arc<NodeState>>, 5555 Query(params): Query<DiffCommitsParams>, 5656 ) -> Result<Json<DiffCommitsResult>, NodeError> { 57+ // Acquire everything we need from the shared store under one async 58+ // lock acquisition; once the guard is dropped the rest of the handler 59+ // is fully synchronous and can hold non-`Send` git2 types without 60+ // making the handler future itself non-`Send`. 5761 let store = state.store.lock().await; 5862 if !store.has_git_mirror(¶ms.did, ¶ms.repo) { 5963 return Err(NodeError::RefNotFound(format!(
@@ -64,6 +68,8 @@ pub async fn diff_commits(
6468 let mirror = store 6569 .open_or_init_git_mirror(¶ms.did, ¶ms.repo) 6670 .map_err(|e| NodeError::Internal(format!("open mirror: {e}")))?; 71+ let import_marks = store.load_import_marks(¶ms.did, ¶ms.repo); 72+ let vcs_store = store.open(¶ms.did, ¶ms.repo).ok(); 6773 drop(store); 6874 6975 let from_oid = git2::Oid::from_str(¶ms.from)
@@ -254,14 +260,9 @@ pub async fn diff_commits(
254260 // See panproto/panproto#28 (distribute git-remote-cospan binary). 255261 let file_paths: Vec<(String, bool)> = 256262 entries.iter().map(|e| (e.path.clone(), e.binary)).collect(); 257- let structural_diffs = try_load_structural_diffs_from_vcs( 258- &state, 259- ¶ms.did, 260- ¶ms.repo, 261- from_oid, 262- to_oid, 263- &file_paths, 264- ); 263+ let structural_diffs = vcs_store.as_ref().and_then(|store| { 264+ try_load_structural_diffs_from_vcs(store, &import_marks, from_oid, to_oid, &file_paths) 265+ }); 265266 266267 let files: Vec<FileDiff> = entries 267268 .iter()
@@ -304,22 +305,21 @@ pub async fn diff_commits(
304305 /// 305306 /// Walks each commit's `SchemaTree` (panproto issue #49) into a 306307 /// `path -> FileSchemaObject` map, then diffs per-file schemas natively. 308+/// 309+/// Synchronous: callers obtain `vcs_store` and `import_marks` from the 310+/// shared `NodeState.store` lock once at the top of the handler, then 311+/// pass them in here so this function does not need to await. 307312 fn try_load_structural_diffs_from_vcs( 308- state: &Arc<NodeState>, 309- did: &str, 310- repo: &str, 313+ vcs_store: &panproto_core::vcs::FsStore, 314+ import_marks: &std::collections::HashMap<git2::Oid, panproto_core::vcs::ObjectId>, 311315 from_git_oid: git2::Oid, 312316 to_git_oid: git2::Oid, 313317 file_paths: &[(String, bool)], 314318 ) -> Option<std::collections::HashMap<String, serde_json::Value>> { 315319 use panproto_core::vcs::{self, FileSchemaObject, Object, Store}; 316320 317- let store_guard = state.store.blocking_lock(); 318- let marks = store_guard.load_import_marks(did, repo); 319- let from_pp = marks.get(&from_git_oid).copied()?; 320- let to_pp = marks.get(&to_git_oid).copied()?; 321- let vcs_store = store_guard.open(did, repo).ok()?; 322- drop(store_guard); 321+ let from_pp = import_marks.get(&from_git_oid).copied()?; 322+ let to_pp = import_marks.get(&to_git_oid).copied()?; 323323 324324 fn collect_leaves<S: Store>( 325325 store: &S,
@@ -338,8 +338,8 @@ fn try_load_structural_diffs_from_vcs(
338338 Some(out) 339339 } 340340 341- let from_leaves = collect_leaves(&vcs_store, &from_pp)?; 342- let to_leaves = collect_leaves(&vcs_store, &to_pp)?; 341+ let from_leaves = collect_leaves(vcs_store, &from_pp)?; 342+ let to_leaves = collect_leaves(vcs_store, &to_pp)?; 343343 344344 // Produce an empty schema matching the shape of `template`, used when a 345345 // file is pure-add or pure-delete (only one side has a leaf).