From 2a4ea3dcc065dacba480e6cdddc053a814ea9760 Mon Sep 17 00:00:00 2001 From: streamer45 Date: Sat, 27 Dec 2025 19:23:10 +0100 Subject: [PATCH] fix(plugins): resolve flaky native plugin reload test --- apps/skit/src/plugins.rs | 33 ++++++++++++++++++++++----------- apps/skit/src/server.rs | 13 +++++++++++++ 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/apps/skit/src/plugins.rs b/apps/skit/src/plugins.rs index af38ef8e..8af892bd 100644 --- a/apps/skit/src/plugins.rs +++ b/apps/skit/src/plugins.rs @@ -559,16 +559,6 @@ impl UnifiedPluginManager { } } - if remove_file { - if let Err(err) = std::fs::remove_file(&managed.file_path) { - warn!( - error = %err, - file = ?managed.file_path, - "Failed to remove plugin file during unload" - ); - } - } - let plugin_type = match managed.plugin_type { PluginType::Wasm => "wasm", PluginType::Native => "native", @@ -581,7 +571,28 @@ impl UnifiedPluginManager { ); self.update_loaded_gauge(); - Ok(PluginSummary::from_entry(kind.to_string(), &managed)) + // Capture file path before dropping managed, as we need to delete the file + // AFTER the library is unloaded to avoid dlopen caching issues on reload. + let file_path = managed.file_path.clone(); + let summary = PluginSummary::from_entry(kind.to_string(), &managed); + + // Explicitly drop managed to ensure the library (Arc) is fully + // unloaded (dlclose called) BEFORE we delete the file. This prevents race + // conditions where dlopen during reload might return a cached handle to + // the old library if the file is deleted while the library is still loaded. + drop(managed); + + if remove_file { + if let Err(err) = std::fs::remove_file(&file_path) { + warn!( + error = %err, + file = ?file_path, + "Failed to remove plugin file during unload" + ); + } + } + + Ok(summary) } /// Returns all loaded plugins as summaries. diff --git a/apps/skit/src/server.rs b/apps/skit/src/server.rs index ea36e4ea..8188ef28 100644 --- a/apps/skit/src/server.rs +++ b/apps/skit/src/server.rs @@ -499,6 +499,19 @@ async fn upload_plugin_handler( } } + // Ensure all data is flushed to disk before we try to load the plugin. + // This is important because load_from_temp_file uses sync file operations. + if let Err(e) = file.flush().await { + let _ = tokio::fs::remove_file(&tmp_path).await; + return Err(PluginHttpError::BadRequest(format!("Failed to flush temp file: {e}"))); + } + if let Err(e) = file.sync_all().await { + let _ = tokio::fs::remove_file(&tmp_path).await; + return Err(PluginHttpError::BadRequest(format!("Failed to sync temp file: {e}"))); + } + // Explicitly drop the file handle to ensure it's closed before we read it + drop(file); + plugin_file_name = Some(file_name); temp_file_path = Some(tmp_path); }