Skip to content

Commit 0861233

Browse files
authored
fix(plugins): resolve flaky native plugin reload test (#3)
1 parent c215b1c commit 0861233

File tree

2 files changed

+35
-11
lines changed

2 files changed

+35
-11
lines changed

apps/skit/src/plugins.rs

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -559,16 +559,6 @@ impl UnifiedPluginManager {
559559
}
560560
}
561561

562-
if remove_file {
563-
if let Err(err) = std::fs::remove_file(&managed.file_path) {
564-
warn!(
565-
error = %err,
566-
file = ?managed.file_path,
567-
"Failed to remove plugin file during unload"
568-
);
569-
}
570-
}
571-
572562
let plugin_type = match managed.plugin_type {
573563
PluginType::Wasm => "wasm",
574564
PluginType::Native => "native",
@@ -581,7 +571,28 @@ impl UnifiedPluginManager {
581571
);
582572
self.update_loaded_gauge();
583573

584-
Ok(PluginSummary::from_entry(kind.to_string(), &managed))
574+
// Capture file path before dropping managed, as we need to delete the file
575+
// AFTER the library is unloaded to avoid dlopen caching issues on reload.
576+
let file_path = managed.file_path.clone();
577+
let summary = PluginSummary::from_entry(kind.to_string(), &managed);
578+
579+
// Explicitly drop managed to ensure the library (Arc<Library>) is fully
580+
// unloaded (dlclose called) BEFORE we delete the file. This prevents race
581+
// conditions where dlopen during reload might return a cached handle to
582+
// the old library if the file is deleted while the library is still loaded.
583+
drop(managed);
584+
585+
if remove_file {
586+
if let Err(err) = std::fs::remove_file(&file_path) {
587+
warn!(
588+
error = %err,
589+
file = ?file_path,
590+
"Failed to remove plugin file during unload"
591+
);
592+
}
593+
}
594+
595+
Ok(summary)
585596
}
586597

587598
/// Returns all loaded plugins as summaries.

apps/skit/src/server.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,19 @@ async fn upload_plugin_handler(
499499
}
500500
}
501501

502+
// Ensure all data is flushed to disk before we try to load the plugin.
503+
// This is important because load_from_temp_file uses sync file operations.
504+
if let Err(e) = file.flush().await {
505+
let _ = tokio::fs::remove_file(&tmp_path).await;
506+
return Err(PluginHttpError::BadRequest(format!("Failed to flush temp file: {e}")));
507+
}
508+
if let Err(e) = file.sync_all().await {
509+
let _ = tokio::fs::remove_file(&tmp_path).await;
510+
return Err(PluginHttpError::BadRequest(format!("Failed to sync temp file: {e}")));
511+
}
512+
// Explicitly drop the file handle to ensure it's closed before we read it
513+
drop(file);
514+
502515
plugin_file_name = Some(file_name);
503516
temp_file_path = Some(tmp_path);
504517
}

0 commit comments

Comments
 (0)