Fix to very rare deadlock with suibase-daemon

rust/demo-app/move/Move.lock

@@ -49,6 +49,6 @@ published-version = "1"
 
 [env.testnet_proxy]
 chain-id = "4c78adac"
-original-published-id = "0x8f54aa36806fd4081713a75056d3a89a58f8c8977ca8e0a12b32ae587c55c7a9"
-latest-published-id = "0x8f54aa36806fd4081713a75056d3a89a58f8c8977ca8e0a12b32ae587c55c7a9"
+original-published-id = "0x5654d4ac002cf575c632ff1e11a14730b3cb00ef1d3abcc5095c4e7d5b558b42"
+latest-published-id = "0x5654d4ac002cf575c632ff1e11a14730b3cb00ef1d3abcc5095c4e7d5b558b42"
 published-version = "1"

rust/suibase/Cargo.toml

@@ -10,7 +10,7 @@ members = ["crates/suibase-daemon",
 [workspace.package]
 # Bump 'version' for the daemon to self-restart after an update.
 # (this is not the Suibase package version, it is specifically for the Rust crates).
-version = "0.0.8"
+version = "0.0.9"
 edition = "2021"
 
 [workspace.dependencies]
@@ -32,7 +32,7 @@ axum-server = { version = "0.5.1", default-features = false, features = [
  "tls-rustls",
 ] }
 bcs = "0.1.4"
-chrono = "0.4.31" 
+chrono = "0.4.31"
 clap = { version = "3.2.22", features = ["derive"] } # No upgrade to v4 until color are back.
 colored = "2.0.0"
 data-encoding = "2.4.0"

rust/suibase/crates/dtp-daemon/src/api/impl_general_api.rs

@@ -126,7 +126,7 @@ impl GeneralApiImpl {
  let first_word = words.next();
 
  match first_word {
- Some("localnet") | Some("faucet") | Some("multi-link") | Some("proxy") => {
+ Some("localnet") | Some("faucet") => {
  // Get the 4th word in words.
  let mut service_status = words.nth(2).unwrap_or("").to_string();
 

rust/suibase/crates/suibase-daemon/src/api/impl_general_api.rs

@@ -189,6 +189,7 @@ impl GeneralApiImpl {
  if line_lc.contains("not initialized")
  || line_lc.contains("not found")
  || line_lc.contains("no such")
+ || line_lc.contains("no command")
  {
  resp.status = Some("DISABLED".to_string());
  let status_info = format!("{0} not initialized. Do '{0} start'", workdir_name);
@@ -374,7 +375,7 @@ impl GeneralApiImpl {
  // Get an update with a "<workdir> status" shell call.
  // Map it into the resp.
  let cmd_resp = match self
- .shell_exec(workdir_idx, format!("{} status", workdir))
+ .shell_exec(workdir_idx, format!("{} status --daemoncall", workdir))
  .await
  {
  Ok(cmd_resp) => cmd_resp,
@@ -393,9 +394,9 @@ impl GeneralApiImpl {
  }
 
  {
- // Get the globals for the target workdir_idx.
- let mut globals_read_guard = self.globals.get_status(workdir_idx).write().await;
- let globals = &mut *globals_read_guard;
+ // Update the globals with this potentially new response.
+ let mut globals_write_guard = self.globals.get_status(workdir_idx).write().await;
+ let globals = &mut *globals_write_guard;
  if let Some(ui) = &mut globals.ui {
  // Update globals.ui with resp if different. This will update the uuid_data accordingly.
  let uuids = ui.set(&resp);
@@ -610,19 +611,16 @@ impl GeneralApiServer for GeneralApiImpl {
  let globals = &*globals_read_guard;
 
  if let Some(ui) = &globals.ui {
- if method_uuid.is_some() || data_uuid.is_some() {
+ if let (Some(method_uuid), Some(data_uuid)) = (method_uuid, data_uuid) {
  let mut are_same_version = false;
- if let (Some(method_uuid), Some(data_uuid)) =
- (method_uuid.as_ref(), data_uuid.as_ref())
- {
- let globals_data_uuid = &ui.get_uuid().get_data_uuid();
- if data_uuid == globals_data_uuid {
- let globals_method_uuid = &ui.get_uuid().get_method_uuid();
- if method_uuid == globals_method_uuid {
- are_same_version = true;
- }
+ let globals_data_uuid = ui.get_uuid().get_data_uuid();
+ if data_uuid == globals_data_uuid {
+ let globals_method_uuid = ui.get_uuid().get_method_uuid();
+ if method_uuid == globals_method_uuid {
+ are_same_version = true;
  }
  }
+
  if !are_same_version {
  // Something went wrong, but this could be normal if the globals just got updated
  // and the caller is not yet aware of it (assume the caller will eventually discover

rust/suibase/crates/suibase-daemon/src/api/impl_packages_api.rs

@@ -2,17 +2,21 @@ use std::time::SystemTime;
 
 use axum::async_trait;
 
+use anyhow::Result;
+
+use common::basic_types::WorkdirIdx;
 use jsonrpsee::core::RpcResult;
 use jsonrpsee_types::ErrorObjectOwned as RpcError;
 
 use chrono::Utc;
 
 use crate::admin_controller::{AdminController, AdminControllerTx};
+
 use crate::api::RpcSuibaseError;
 use crate::shared_types::{Globals, GlobalsWorkdirsST};
 
 use super::{
- MoveConfig, PackageInstance, PackagesApiServer, RpcInputError, SuccessResponse, Versioned,
+ Header, MoveConfig, PackageInstance, PackagesApiServer, RpcInputError, SuccessResponse,
  WorkdirPackagesResponse, WorkdirSuiEventsResponse,
 };
 
@@ -151,8 +155,7 @@ impl PackagesApiServer for PackagesApiImpl {
 
  // Create the globals.ui if does not exists.
  if globals.ui.is_none() {
- let versioned_resp = Versioned::new(WorkdirPackagesResponse::new());
- globals.ui = Some(versioned_resp);
+ globals.init_empty_ui(workdir.clone());
  }
 
  if let Some(ui) = &mut globals.ui {
@@ -242,47 +245,55 @@ impl PackagesApiServer for PackagesApiImpl {
  None => return Err(RpcInputError::InvalidParams("workdir".to_string(), workdir).into()),
  };
 
- let mut resp_ready: Option<WorkdirPackagesResponse> = None;
-
  if method_uuid.is_none() && data_uuid.is_none() {
- // Just return what is already built in-memory, or empty.
+ // Best-effort refresh, since user is requesting for the latest.
+
+ // Allow only one API request for a given workdir at the time to avoid race conditions.
+ let mut api_mutex_guard = self.globals.get_api_mutex(workdir_idx).lock().await;
+ let api_mutex = &mut *api_mutex_guard;
+
+ let last_api_call_timestamp = &mut api_mutex.last_get_workdir_status_time;
 
- // Get the globals for the target workdir_idx.
+ let _ = self
+ .update_globals_workdir_packages(
+ workdir.clone(),
+ workdir_idx,
+ last_api_call_timestamp,
+ )
+ .await;
+ }
+
+ {
  let globals_read_guard = self.globals.get_packages(workdir_idx).read().await;
  let globals = &*globals_read_guard;
 
  if let Some(ui) = &globals.ui {
  if let (Some(method_uuid), Some(data_uuid)) = (method_uuid, data_uuid) {
+ let mut are_same_version = false;
  let globals_data_uuid = ui.get_uuid().get_data_uuid();
  if data_uuid == globals_data_uuid {
  let globals_method_uuid = ui.get_uuid().get_method_uuid();
  if method_uuid == globals_method_uuid {
- // The caller requested the same data that it already have a copy of.
- // Respond with the same UUID as a way to say "no change".
- let mut new_resp = WorkdirPackagesResponse::new();
- ui.write_uuids_into_header_param(&mut new_resp.header);
- resp_ready = Some(new_resp);
+ are_same_version = true;
  }
  }
- } else {
- // The caller did not specify a method_uuid or data_uuid and
- // there is an in-memory response ready. Just respond with it.
- let mut existing_resp = ui.get_data().clone();
- ui.write_uuids_into_header_param(&mut existing_resp.header);
- resp_ready = Some(existing_resp);
+  if !are_same_version {
+  // Something went wrong, but this could be normal if the globals just got updated
+  // and the caller is not yet aware of it (assume the caller will eventually discover
+  // the latest version with getVersions).
+  return Err(RpcSuibaseError::OutdatedUUID().into());
+ }
  }
+ // Response with the latest global data.
+ let mut resp = ui.get_data().clone();
+ resp.header.set_from_uuids(ui.get_uuid());
+ return Ok(resp);
+ } else {
+ return Err(
+ RpcSuibaseError::InternalError("globals.ui was None".to_string()).into(),
+ );
  }
  }
-
- if resp_ready.is_none() {
- // There was no data in the globals, so revert to empty response.
- resp_ready = Some(WorkdirPackagesResponse::new());
- }
- let mut resp_ready = resp_ready.unwrap();
-
- resp_ready.header.method = "getWorkdirPackages".to_string();
- resp_ready.header.key = Some(workdir.clone());
- return Ok(resp_ready);
  }
 }
 
@@ -414,4 +425,48 @@ impl PackagesApiImpl {
 
  Ok((workdir_idx, package_uuid))
  }
+
+ async fn update_globals_workdir_packages(
+ &self,
+ workdir: String,
+ workdir_idx: WorkdirIdx,
+ last_api_call_timestamp: &mut tokio::time::Instant,
+ ) -> Result<Header> {
+ // Debounce excessive refresh request on short period of time.
+ if last_api_call_timestamp.elapsed() < tokio::time::Duration::from_millis(50) {
+ let globals_read_guard = self.globals.get_packages(workdir_idx).read().await;
+ let globals = &*globals_read_guard;
+
+ if let Some(ui) = &globals.ui {
+ return Ok(ui.get_data().header.clone());
+ }
+ };
+ *last_api_call_timestamp = tokio::time::Instant::now();
+
+ // Read the Filesystem to get the published packages.
+
+ // TODO Get latest from filesystem.
+
+ // Merge filesystem findings with globals and create a new resp as needed.
+ // This is a write lock on the globals.
+ let resp_header = {
+ let mut globals_write_guard = self.globals.get_packages(workdir_idx).write().await;
+ let globals = &mut *globals_write_guard;
+
+ if let Some(ui) = &mut globals.ui {
+ // Update globals.ui with resp if different. This will update the uuid_data accordingly.
+ // TODO For now just get what is in the global.
+ // let uuids = ui.set(&resp);
+
+ // Make the header in the response have the proper uuids.
+ //resp.header.set_from_uuids(&uuids);
+ ui.get_data().header.clone()
+ } else {
+ globals.init_empty_ui(workdir.clone());
+ globals.ui.as_ref().unwrap().get_data().header.clone()
+ }
+ };
+
+ Ok(resp_header)
+ }
 }

rust/suibase/crates/suibase-daemon/src/shared_types/packages.rs

@@ -1,4 +1,3 @@
-
 use crate::api::{Versioned, WorkdirPackagesResponse};
 
 //use common::basic_types::{AutoSizeVec, WorkdirIdx};
@@ -40,63 +39,18 @@ impl GlobalsWorkdirPackagesST {
  Self { ui: None }
  }
 
- // Convenient access to the move_configs for a given workdir.
-/* 
- pub fn get_move_configs(
- workdirs: &AutoSizeVec<PackagesWorkdirConfig>,
- workdir_idx: WorkdirIdx,
- ) -> Option<&HashMap<String, MoveConfig>> {
- // Caller must hold a read lock on workdirs.
- // Will return None if an object is missing while trying to reach the MoveConfig (should not happen).
- let config_resp = Self::get_config_resp(workdirs, workdir_idx)?;
- Some(config_resp.move_configs.as_ref().unwrap())
- }
-
- pub fn get_mut_move_configs(
- workdirs: &mut AutoSizeVec<PackagesWorkdirConfig>,
- workdir_idx: WorkdirIdx,
- ) -> &mut HashMap<String, MoveConfig> {
- // Caller must hold a write lock on workdirs.
- // Will create the move_configs if does not exists.
- let config_resp = Self::get_mut_config_resp(workdirs, workdir_idx);
-
- config_resp.move_configs.as_mut().unwrap()
- }
-
- #[allow(clippy::question_mark)]
- pub fn get_config_resp(
- workdirs: &AutoSizeVec<PackagesWorkdirConfig>,
- workdir_idx: WorkdirIdx,
- ) -> Option<&WorkdirPackagesResponse> {
- // Will return None if an object is missing while trying to reach the PackageConfigResponse (should not happen).
- let packages_workdir_config = workdirs.get_if_some(workdir_idx)?;
- let ui = packages_workdir_config.ui.as_ref()?;
- let config_resp = ui.get_data();
+ pub fn init_empty_ui(&mut self, workdir: String) {
+ // As needed, initialize globals.ui with resp.
+ let mut empty_resp = WorkdirPackagesResponse::new();
+ empty_resp.header.method = "getWorkdirPackages".to_string();
+ empty_resp.header.key = Some(workdir);
 
- if config_resp.move_configs.is_none() {
- return None;
- }
-
- Some(config_resp)
+ let new_versioned_resp = Versioned::new(empty_resp.clone());
+ // Copy the newly created UUID in the inner response header (so the caller can use these also).
+ new_versioned_resp.write_uuids_into_header_param(&mut empty_resp.header);
+ self.ui = Some(new_versioned_resp);
  }
 
- pub fn get_mut_config_resp(
- workdirs: &mut AutoSizeVec<PackagesWorkdirConfig>,
- workdir_idx: WorkdirIdx,
- ) -> &mut WorkdirPackagesResponse {
- let packages_workdir_config = workdirs.get_mut(workdir_idx);
- if packages_workdir_config.ui.is_none() {
- packages_workdir_config.ui = Some(Versioned::new(WorkdirPackagesResponse::new()));
- }
- let ui = packages_workdir_config.ui.as_mut().unwrap();
- let config_resp = ui.get_mut_data();
-
- if config_resp.move_configs.is_none() {
- config_resp.move_configs = Some(HashMap::new());
- }
-
- config_resp
- }*/
 }
 
 impl Default for GlobalsWorkdirPackagesST {