Complete first release candidate audit (#107)

crates/libremetaverse/src/disposal_helper.rs

@@ -0,0 +1,317 @@
+//! Cross-platform resource cleanup helpers.
+
+use std::panic::{AssertUnwindSafe, catch_unwind};
+use std::sync::Mutex;
+use std::time::Duration;
+
+use libremetaverse_types::compat::{CancellationTokenSource, Close, ExternalError, Task, Thread};
+
+use crate::Error;
+
+type ErrorLogger = Box<dyn Fn(String, Option<ExternalError>) + Send + Sync>;
+
+fn panic_error(payload: Box<dyn std::any::Any + Send>) -> ExternalError {
+    if let Some(message) = payload.downcast_ref::<&str>() {
+        ExternalError((*message).to_owned())
+    } else if let Some(message) = payload.downcast_ref::<String>() {
+        ExternalError(message.clone())
+    } else {
+        ExternalError("action panicked with a non-string payload".to_owned())
+    }
+}
+
+fn report(logger: &Option<ErrorLogger>, message: impl Into<String>, error: Option<ExternalError>) {
+    if let Some(logger) = logger {
+        logger(message.into(), error);
+    }
+}
+
+/// Helpers that contain cleanup failures and report them through a caller-owned
+/// logger, matching the upstream disposal contract.
+pub struct DisposalHelper;
+
+impl DisposalHelper {
+    pub fn safe_action(
+        action: Box<dyn Fn() + Send + Sync>,
+        action_name: Option<String>,
+        logger: Option<ErrorLogger>,
+    ) -> Result<(), Error> {
+        if let Err(payload) = catch_unwind(AssertUnwindSafe(action)) {
+            let message = action_name.filter(|name| !name.is_empty()).map_or_else(
+                || "Error executing action".to_owned(),
+                |name| format!("Error executing {name}"),
+            );
+            report(&logger, message, Some(panic_error(payload)));
+        }
+        Ok(())
+    }
+
+    pub fn safe_cancel_and_dispose(
+        mut source: Option<CancellationTokenSource>,
+        logger: Option<ErrorLogger>,
+    ) -> Result<(), Error> {
+        let Some(source) = source.as_mut() else {
+            return Ok(());
+        };
+        source.cancel();
+        if let Err(error) = source.close() {
+            report(
+                &logger,
+                "Error disposing CancellationTokenSource",
+                Some(error),
+            );
+        }
+        Ok(())
+    }
+
+    pub fn safe_dispose(
+        mut resource: Box<dyn Close>,
+        resource_name: Option<String>,
+        logger: Option<ErrorLogger>,
+    ) -> Result<(), Error> {
+        if let Err(error) = resource.close() {
+            let message = resource_name.filter(|name| !name.is_empty()).map_or_else(
+                || "Error disposing resource".to_owned(),
+                |name| format!("Error disposing {name}"),
+            );
+            report(&logger, message, Some(error));
+        }
+        Ok(())
+    }
+
+    pub fn safe_dispose_all(
+        resources: Box<dyn Iterator<Item = Box<dyn Close>>>,
+        logger: Option<ErrorLogger>,
+    ) -> Result<(), Error> {
+        for mut resource in resources {
+            if let Err(error) = resource.close() {
+                report(&logger, "Error disposing resource", Some(error));
+            }
+        }
+        Ok(())
+    }
+
+    pub fn safe_dispose_clear<T: Close>(
+        mut collection: Vec<T>,
+        logger: Option<ErrorLogger>,
+    ) -> Result<(), Error> {
+        for resource in &mut collection {
+            if let Err(error) = resource.close() {
+                report(&logger, "Error disposing resource", Some(error));
+            }
+        }
+        collection.clear();
+        Ok(())
+    }
+
+    pub fn safe_join_thread(
+        thread: Thread,
+        timeout: Duration,
+        logger: Option<ErrorLogger>,
+    ) -> Result<bool, Error> {
+        if !thread.is_alive() {
+            return Ok(true);
+        }
+        match thread.join_timeout(timeout) {
+            Ok(true) => Ok(true),
+            Ok(false) => {
+                report(
+                    &logger,
+                    format!(
+                        "Thread {} did not exit in time",
+                        thread.name().unwrap_or("unnamed")
+                    ),
+                    None,
+                );
+                Ok(false)
+            }
+            Err(error) => {
+                report(
+                    &logger,
+                    format!(
+                        "Error waiting for thread {}",
+                        thread.name().unwrap_or("unnamed")
+                    ),
+                    Some(error),
+                );
+                Ok(false)
+            }
+        }
+    }
+
+    pub fn safe_wait_task_with_task_time_span_action(
+        task: Task<()>,
+        timeout: Duration,
+        logger: Option<ErrorLogger>,
+    ) -> Result<bool, Error> {
+        match task.wait_timeout(timeout) {
+            Ok(Some(())) => Ok(true),
+            Ok(None) => {
+                report(&logger, "Task did not complete in time", None);
+                Ok(false)
+            }
+            Err(error) => {
+                report(
+                    &logger,
+                    "Error waiting for task",
+                    Some(ExternalError(error.to_string())),
+                );
+                Ok(false)
+            }
+        }
+    }
+
+    pub async fn safe_wait_task_with_task_time_span_action_b4795938(
+        task: Task<()>,
+        timeout: Duration,
+        logger: Option<ErrorLogger>,
+    ) -> Result<bool, Error> {
+        // `Task` is executor-neutral; its bounded driver parks only this worker
+        // thread and is woken by the underlying future when progress is ready.
+        Self::safe_wait_task_with_task_time_span_action(task, timeout, logger)
+    }
+
+    pub async fn using_with_func_func<TDisposable: Close, TResult>(
+        factory: Box<dyn Fn() -> TDisposable + Send + Sync>,
+        action: Box<dyn Fn(&mut TDisposable) -> Task<Option<TResult>> + Send + Sync>,
+    ) -> Result<Option<TResult>, Error> {
+        let mut guard = CloseGuard::new(factory());
+        let result = action(guard.resource_mut()).await;
+        guard.close()?;
+        result
+    }
+
+    pub fn using_with_func_func_3fbb9598<TDisposable: Close, TResult>(
+        factory: Box<dyn Fn() -> TDisposable + Send + Sync>,
+        action: Box<dyn Fn(&mut TDisposable) -> Option<TResult> + Send + Sync>,
+    ) -> Result<Option<TResult>, Error> {
+        let mut guard = CloseGuard::new(factory());
+        let result = action(guard.resource_mut());
+        guard.close()?;
+        Ok(result)
+    }
+}
+
+struct CloseGuard<T: Close> {
+    resource: Option<T>,
+}
+
+impl<T: Close> CloseGuard<T> {
+    fn new(resource: T) -> Self {
+        Self {
+            resource: Some(resource),
+        }
+    }
+
+    fn resource_mut(&mut self) -> &mut T {
+        self.resource
+            .as_mut()
+            .expect("resource remains until close")
+    }
+
+    fn close(&mut self) -> Result<(), Error> {
+        let mut resource = self.resource.take().ok_or(Error::InvalidOperation)?;
+        resource.close().map_err(|_| Error::InvalidOperation)
+    }
+}
+
+impl<T: Close> Drop for CloseGuard<T> {
+    fn drop(&mut self) {
+        if let Some(resource) = self.resource.as_mut() {
+            let _ = resource.close();
+        }
+    }
+}
+
+/// Runs one callback exactly once when explicitly disposed or dropped.
+pub struct DisposalHelperDisposalGuard {
+    on_dispose: Mutex<Option<Box<dyn Fn() + Send + Sync>>>,
+}
+
+impl DisposalHelperDisposalGuard {
+    pub fn new(on_dispose: Box<dyn Fn() + Send + Sync>) -> Result<Self, Error> {
+        Ok(Self {
+            on_dispose: Mutex::new(Some(on_dispose)),
+        })
+    }
+
+    pub fn dispose(&self) -> Result<(), Error> {
+        let callback = self
+            .on_dispose
+            .lock()
+            .unwrap_or_else(std::sync::PoisonError::into_inner)
+            .take();
+        let Some(callback) = callback else {
+            return Ok(());
+        };
+        catch_unwind(AssertUnwindSafe(callback)).map_err(|_| Error::InvalidOperation)
+    }
+}
+
+impl Close for DisposalHelperDisposalGuard {
+    fn close(&mut self) -> Result<(), ExternalError> {
+        self.dispose()
+            .map_err(|error| ExternalError(error.to_string()))
+    }
+}
+
+impl Drop for DisposalHelperDisposalGuard {
+    fn drop(&mut self) {
+        let _ = self.dispose();
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::sync::{Arc, atomic::AtomicUsize};
+
+    struct CountedClose(Arc<AtomicUsize>);
+
+    impl Close for CountedClose {
+        fn close(&mut self) -> Result<(), ExternalError> {
+            self.0.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+            Ok(())
+        }
+    }
+
+    #[test]
+    fn disposal_guard_runs_callback_once() {
+        let count = Arc::new(AtomicUsize::new(0));
+        let observed = Arc::clone(&count);
+        let guard = DisposalHelperDisposalGuard::new(Box::new(move || {
+            observed.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+        }))
+        .unwrap();
+        guard.dispose().unwrap();
+        guard.dispose().unwrap();
+        drop(guard);
+        assert_eq!(count.load(std::sync::atomic::Ordering::Relaxed), 1);
+    }
+
+    #[test]
+    fn using_closes_after_action() {
+        let count = Arc::new(AtomicUsize::new(0));
+        let factory_count = Arc::clone(&count);
+        let result = DisposalHelper::using_with_func_func_3fbb9598(
+            Box::new(move || CountedClose(factory_count.clone())),
+            Box::new(|_| Some(42)),
+        )
+        .unwrap();
+        assert_eq!(result, Some(42));
+        assert_eq!(count.load(std::sync::atomic::Ordering::Relaxed), 1);
+    }
+
+    #[test]
+    fn bounded_thread_join_reports_timeout_then_completion() {
+        let thread = Thread::spawn(Some("disposal-test".to_owned()), || {
+            std::thread::sleep(Duration::from_millis(30));
+        })
+        .unwrap();
+        assert!(
+            !DisposalHelper::safe_join_thread(thread.clone(), Duration::from_millis(1), None,)
+                .unwrap()
+        );
+        assert!(DisposalHelper::safe_join_thread(thread, Duration::from_secs(1), None,).unwrap());
+    }
+}