diff --git a/Cargo.lock b/Cargo.lock index 9ce0b6db..e3790295 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6197,6 +6197,7 @@ dependencies = [ "async-trait", "bytes", "libloading 0.9.0", + "opentelemetry", "serde-saphyr", "serde_json", "streamkit-core", diff --git a/apps/skit/src/config.rs b/apps/skit/src/config.rs index 36147ef9..ce6ee7cd 100644 --- a/apps/skit/src/config.rs +++ b/apps/skit/src/config.rs @@ -1,7 +1,6 @@ // SPDX-FileCopyrightText: © 2025 StreamKit Contributors // // SPDX-License-Identifier: MPL-2.0 - use figment::{ providers::{Env, Format, Toml}, Figment, @@ -17,6 +16,15 @@ const fn default_engine_batch_size() -> usize { 32 } +/// Deserialize `Option` with a minimum clamp of 1 for timeout values. +fn deserialize_clamp_timeout<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let val: Option = Option::deserialize(deserializer)?; + Ok(val.map(|v| v.max(1))) +} + /// Preset tuning profiles for the engine. #[derive(Deserialize, Serialize, Debug, Clone, Copy, JsonSchema)] #[serde(rename_all = "kebab-case")] @@ -187,6 +195,10 @@ const fn default_max_body_size() -> usize { 100 * 1024 * 1024 } +const fn default_native_call_timeout_value() -> u64 { + 300 +} + fn default_cors_allowed_origins() -> Vec { vec![ // Portless localhost (e.g., reverse proxy on 80/443) @@ -354,6 +366,17 @@ impl Default for ServerConfig { #[derive(Deserialize, Serialize, Debug, Clone, JsonSchema)] pub struct PluginConfig { pub directory: String, + /// Native plugin FFI call timeout in seconds (default: 300, minimum: 1). + /// + /// Set to `null` to use only the default backstop timeout on the reply + /// side; the send-side backpressure guard remains bounded regardless. + /// + /// Values below 1 are clamped to 1 to prevent instant timeouts. + #[serde( + default = "PluginConfig::default_native_call_timeout_secs", + deserialize_with = "deserialize_clamp_timeout" + )] + pub native_call_timeout_secs: Option, #[serde(flatten, default)] pub http_management: PluginHttpConfig, #[serde(flatten, default)] @@ -450,6 +473,7 @@ impl Default for PluginConfig { fn default() -> Self { Self { directory: ".plugins".to_string(), + native_call_timeout_secs: Some(default_native_call_timeout_value()), http_management: PluginHttpConfig::default(), marketplace: PluginMarketplaceConfig::default(), trusted_pubkeys: Vec::new(), @@ -460,6 +484,15 @@ impl Default for PluginConfig { } } +impl PluginConfig { + // Serde default hooks must return the exact field type; the wrapped value + // distinguishes missing config from explicit null. + #[allow(clippy::unnecessary_wraps)] + const fn default_native_call_timeout_secs() -> Option { + Some(default_native_call_timeout_value()) + } +} + const fn default_require_registry_origin() -> bool { false } diff --git a/apps/skit/src/marketplace_installer.rs b/apps/skit/src/marketplace_installer.rs index 10e0f68d..a7813763 100644 --- a/apps/skit/src/marketplace_installer.rs +++ b/apps/skit/src/marketplace_installer.rs @@ -2108,6 +2108,7 @@ mod tests { plugin_dir.to_path_buf(), wasm_dir, native_dir, + Some(std::time::Duration::from_secs(300)), )?; Ok(Arc::new(tokio::sync::Mutex::new(manager))) } @@ -2201,6 +2202,7 @@ mod tests { let config = PluginConfig { directory: plugin_dir.to_string_lossy().to_string(), + native_call_timeout_secs: Some(300), http_management: crate::config::PluginHttpConfig { allow_http_management: false }, marketplace: crate::config::PluginMarketplaceConfig { marketplace_enabled: true, @@ -2302,6 +2304,7 @@ mod tests { let config = PluginConfig { directory: plugin_dir.to_string_lossy().to_string(), + native_call_timeout_secs: Some(300), http_management: crate::config::PluginHttpConfig { allow_http_management: false }, marketplace: crate::config::PluginMarketplaceConfig { marketplace_enabled: true, @@ -2369,6 +2372,7 @@ mod tests { let config = PluginConfig { directory: plugin_dir.to_string_lossy().to_string(), + native_call_timeout_secs: Some(300), http_management: crate::config::PluginHttpConfig { allow_http_management: false }, marketplace: crate::config::PluginMarketplaceConfig { marketplace_enabled: true, diff --git a/apps/skit/src/plugins.rs b/apps/skit/src/plugins.rs index 8495d0a9..3b3d9bdd 100644 --- a/apps/skit/src/plugins.rs +++ b/apps/skit/src/plugins.rs @@ -143,6 +143,7 @@ pub struct UnifiedPluginManager { plugin_base_dir: PathBuf, wasm_directory: PathBuf, native_directory: PathBuf, + native_call_timeout: Option, engine: Arc, #[allow(dead_code)] // Will be used when plugins are migrated to new resource system resource_manager: Arc, @@ -165,6 +166,7 @@ impl UnifiedPluginManager { plugin_base_dir: PathBuf, wasm_directory: PathBuf, native_directory: PathBuf, + native_call_timeout: Option, ) -> Result { if !wasm_directory.exists() { std::fs::create_dir_all(&wasm_directory).with_context(|| { @@ -188,6 +190,7 @@ impl UnifiedPluginManager { plugin_base_dir, wasm_directory, native_directory, + native_call_timeout, engine, resource_manager, plugins_loaded_gauge: meter @@ -776,12 +779,13 @@ impl UnifiedPluginManager { return Err(anyhow!("Native plugin file {} does not exist", path.to_string_lossy())); } - let plugin = LoadedNativePlugin::load(path) + let mut plugin = LoadedNativePlugin::load(path) .map_err(|e| { tracing::error!(error = %e, path = ?path, "Detailed native plugin load error"); e }) .with_context(|| format!("failed to load native plugin {}", path.to_string_lossy()))?; + plugin.set_call_timeout(self.native_call_timeout); let metadata = plugin.metadata(); let original_kind = metadata.kind.clone(); diff --git a/apps/skit/src/server/mod.rs b/apps/skit/src/server/mod.rs index b9a50c4d..9cfe52a0 100644 --- a/apps/skit/src/server/mod.rs +++ b/apps/skit/src/server/mod.rs @@ -4061,6 +4061,7 @@ pub fn create_app( plugin_base_dir, wasm_plugin_dir, native_plugin_dir, + config.plugins.native_call_timeout_secs.map(std::time::Duration::from_secs), ) .expect("Failed to initialize unified plugin manager"); let plugin_manager = Arc::new(tokio::sync::Mutex::new(plugin_manager)); diff --git a/crates/plugin-native/Cargo.toml b/crates/plugin-native/Cargo.toml index 3c57febb..314eb185 100644 --- a/crates/plugin-native/Cargo.toml +++ b/crates/plugin-native/Cargo.toml @@ -18,6 +18,7 @@ streamkit-plugin-sdk-native = { workspace = true } libloading = "0.9" anyhow = "1.0" +opentelemetry = { workspace = true } tracing = { workspace = true } tokio = { workspace = true, features = ["sync", "macros", "rt", "time"] } async-trait = { workspace = true } diff --git a/crates/plugin-native/src/lib.rs b/crates/plugin-native/src/lib.rs index a93dd5a8..2ee00c72 100644 --- a/crates/plugin-native/src/lib.rs +++ b/crates/plugin-native/src/lib.rs @@ -7,6 +7,7 @@ //! This crate provides the host-side runtime for loading and executing native plugins //! that use the C ABI interface. +pub mod metrics; pub mod wrapper; use anyhow::{anyhow, Context, Result}; @@ -14,7 +15,10 @@ use libloading::{Library, Symbol}; use std::path::Path; use std::sync::Arc; use streamkit_core::{NodeRegistry, PinCardinality}; -use streamkit_plugin_sdk_native::types::{CNativePluginAPI, NATIVE_PLUGIN_API_VERSION}; +use streamkit_plugin_sdk_native::types::PLUGIN_SET_LOG_ENABLED_SYMBOL; +use streamkit_plugin_sdk_native::types::{ + CNativePluginAPI, CSetLogEnabledCallback, NATIVE_PLUGIN_API_VERSION, +}; use streamkit_plugin_sdk_native::{conversions, types::PLUGIN_API_SYMBOL}; use tracing::{info, warn}; @@ -35,6 +39,8 @@ pub struct LoadedNativePlugin { library: Arc, api: &'static CNativePluginAPI, metadata: PluginMetadata, + call_timeout: Option, + set_log_enabled_callback: Option, } /// Metadata extracted from a plugin @@ -106,12 +112,16 @@ impl LoadedNativePlugin { // the lifetime of the loaded library, which we keep alive via Arc. let api = unsafe { &*api_ptr }; - // Check API version compatibility — accept v6 (pre-BinaryWithMeta), - // v7 (added BinaryWithMeta), and v8 (added EncodedAudio metadata). - // All are wire-compatible; v6 plugins never emit BinaryWithMeta - // packets, v7 plugins don't declare EncodedAudio pin types (they - // fall back to Binary), but runtime packet transport is unaffected - // since EncodedAudio is a metadata-only discriminant. + // Check API version compatibility — accept v6 through v9. + // v6: pre-BinaryWithMeta. + // v7: added BinaryWithMeta. + // v8: added EncodedAudio metadata. + // v9: zero-copy binary packets (buffer_handle), logger overhaul + // (set_log_enabled_callback, target = plugin kind). + // v6–v8 are wire-compatible; v9 adds a Binary→BinaryWithMeta wire + // upgrade (see v9 notes in types.rs). Version-gated features use + // runtime api_version checks (e.g. buffer_handle for v9, downgrade + // for v6). if api.version < MIN_SUPPORTED_API_VERSION || api.version > NATIVE_PLUGIN_API_VERSION { let plugin_version = api.version; return Err(anyhow!( @@ -123,6 +133,25 @@ impl LoadedNativePlugin { // Extract metadata let mut metadata = Self::extract_metadata(api)?; + let set_log_enabled_callback = if api.version >= 9 { + // SAFETY: Optional extension symbol. When present, the SDK macro + // exports it with the exact `CSetLogEnabledCallback` signature. + match unsafe { library.get::(PLUGIN_SET_LOG_ENABLED_SYMBOL) } { + Ok(symbol) => Some(*symbol), + Err(e) => { + warn!( + kind = %metadata.kind, + api_version = api.version, + error = %e, + "v9 plugin did not export log-enabled callback symbol" + ); + None + }, + } + } else { + None + }; + // Detect source plugin capability from the v3 API fields. // If the plugin provides `get_source_config`, we probe it with a temporary // instance to read tick parameters. If instance creation fails we fall back @@ -160,7 +189,13 @@ impl LoadedNativePlugin { info!(kind = %metadata.kind, "Successfully loaded native plugin"); - Ok(Self { library: Arc::new(library), api, metadata }) + Ok(Self { + library: Arc::new(library), + api, + metadata, + call_timeout: Some(wrapper::DEFAULT_CALL_TIMEOUT), + set_log_enabled_callback, + }) } /// Extract metadata from the plugin @@ -300,6 +335,21 @@ impl LoadedNativePlugin { &self.library } + /// Override the reply-side timeout for FFI calls (process_packet, flush, + /// tick). This controls how long the async side waits for the worker + /// thread's oneshot reply. + /// + /// Pass `None` to fall back to the default backstop timeout + /// ([`DEFAULT_CALL_TIMEOUT`](crate::wrapper::DEFAULT_CALL_TIMEOUT), + /// 300 s) instead of a caller-chosen duration. The reply side is + /// **never** truly unbounded — the backstop always applies. + /// + /// The channel-send timeout (backpressure guard) also uses + /// `DEFAULT_CALL_TIMEOUT` when this is `None`. + pub const fn set_call_timeout(&mut self, timeout: Option) { + self.call_timeout = timeout; + } + /// Create a new node instance from this plugin /// /// # Errors @@ -316,6 +366,8 @@ impl LoadedNativePlugin { self.api, self.metadata.clone(), params, + self.call_timeout, + self.set_log_enabled_callback, )?; Ok(Box::new(wrapper)) diff --git a/crates/plugin-native/src/metrics.rs b/crates/plugin-native/src/metrics.rs new file mode 100644 index 00000000..11c975c7 --- /dev/null +++ b/crates/plugin-native/src/metrics.rs @@ -0,0 +1,109 @@ +// SPDX-FileCopyrightText: © 2025 StreamKit Contributors +// +// SPDX-License-Identifier: MPL-2.0 + +//! OpenTelemetry metrics for native plugin FFI calls. +//! +//! Instruments are built against the OTel global meter on first access. +//! The OTel meter provider must be initialized before the first plugin load. + +use opentelemetry::metrics::{Counter, Histogram}; +use opentelemetry::KeyValue; + +/// Outcome of an FFI call, used by [`PluginMetrics::record`]. +#[derive(Clone, Copy, Debug)] +pub enum CallOutcome { + Success, + Error, + /// FFI call panicked. Only bumps `plugin.panics` (not `plugin.errors`) + /// so dashboards can sum `errors + panics` without double-counting. + Panic, +} + +/// Per-plugin metrics for FFI call instrumentation. +/// +/// A single global instance is shared across all plugin instances; +/// individual calls are distinguished by `plugin.kind` and `op` labels. +pub struct PluginMetrics { + call_duration: Histogram, + calls_total: Counter, + errors_total: Counter, + panics_total: Counter, + timeouts_total: Counter, +} + +impl PluginMetrics { + pub fn new() -> Self { + let meter = opentelemetry::global::meter("skit_plugin_native"); + Self { + call_duration: meter + .f64_histogram("plugin.call.duration") + .with_description("Duration of native plugin FFI calls") + .with_unit("s") + .with_boundaries( + streamkit_core::metrics::HISTOGRAM_BOUNDARIES_NODE_EXECUTION.to_vec(), + ) + .build(), + calls_total: meter + .u64_counter("plugin.calls") + .with_description("Total native plugin FFI calls") + .build(), + errors_total: meter + .u64_counter("plugin.errors") + .with_description("Native plugin FFI call errors") + .build(), + panics_total: meter + .u64_counter("plugin.panics") + .with_description("Native plugin FFI call panics") + .build(), + timeouts_total: meter + .u64_counter("plugin.timeouts") + .with_description("Native plugin FFI call timeouts (caller-side)") + .build(), + } + } + + /// Record a completed FFI call with pre-built labels. + pub fn record(&self, labels: &[KeyValue; 2], duration_secs: f64, outcome: CallOutcome) { + self.call_duration.record(duration_secs, labels); + self.calls_total.add(1, labels); + match outcome { + CallOutcome::Success => {}, + CallOutcome::Error => { + self.errors_total.add(1, labels); + }, + CallOutcome::Panic => { + self.panics_total.add(1, labels); + }, + } + } + + /// Record a timeout observed from the caller (async) side. + /// + /// Bumps `plugin.calls` (so the call attempt is counted) and + /// `plugin.timeouts` (so timeouts are distinguishable from FFI + /// errors). Does **not** bump `plugin.errors`. + /// + /// If the worker eventually completes the wedged FFI call, the + /// worker will record a second `plugin.calls` entry for the same + /// logical call. This is acceptable: `calls_total` counts + /// observed completions + timeout attempts, not unique call IDs. + pub fn record_timeout(&self, labels: &[KeyValue; 2]) { + self.calls_total.add(1, labels); + self.timeouts_total.add(1, labels); + } + + /// Build a set of metric labels for a given plugin kind and operation. + /// + /// Allocates `kind` into a `String`; intended to be called once per + /// instance (at construction time) and cached on [`InstanceState`]. + pub fn build_labels(kind: &str, op: &'static str) -> [KeyValue; 2] { + [KeyValue::new("plugin.kind", kind.to_string()), KeyValue::new("op", op)] + } +} + +impl Default for PluginMetrics { + fn default() -> Self { + Self::new() + } +} diff --git a/crates/plugin-native/src/wrapper.rs b/crates/plugin-native/src/wrapper.rs index bc2396d0..4c09be85 100644 --- a/crates/plugin-native/src/wrapper.rs +++ b/crates/plugin-native/src/wrapper.rs @@ -6,15 +6,66 @@ //! //! This module provides the `NativeNodeWrapper` which implements the `ProcessorNode` trait //! and bridges to the C ABI plugin interface. +//! +//! ## Callback Lifetime Contract +//! +//! The [`CNodeCallbacks`] struct and all pointers passed to plugin API +//! functions (`process_packet`, `flush`, `tick`) are valid **only for +//! the duration of that single host→plugin call**. The plugin must not +//! stash callback pointers or call them after returning. +//! +//! ## Per-Instance Worker Thread +//! +//! Each plugin instance runs its FFI calls on a dedicated OS thread +//! that receives [`WorkerRequest`] messages over a bounded channel and +//! replies via oneshot channels. This replaces the previous +//! `spawn_blocking` approach, eliminating per-packet thread-pool +//! dispatch overhead. The worker owns a reusable output `Vec` whose +//! *capacity* is preserved across calls (high-water-mark; a one-time +//! flush burst keeps the large `Vec` around for the instance lifetime). +//! Per-request `Arc::clone`s of `telemetry_tx`, `session_id`, +//! `node_id`, `video_pool`, and `audio_pool` into `CallbackContext` +//! still occur on the worker thread — the amortisation is of the +//! tokio-task-spawn and channel setup, not of those clones. +//! +//! Timeout behaviour is preserved: the async side applies +//! `tokio::time::timeout` on the oneshot reply channel rather than on +//! a `spawn_blocking` future. If a call times out, the worker thread +//! is not cancelled — it continues running until the FFI call returns. +//! `NativeNodeWrapper::drop` calls `request_drop` → +//! `destroy_instance` which is deferred if a call is in flight. +//! +//! ## Limitations +//! +//! - **Capacity-1 channel behind wedged FFI**: if a plugin FFI call +//! hangs, the bounded channel blocks further sends until the call +//! returns. Both sides are now bounded: `send_to_worker` applies +//! `call_timeout` on the send, and `await_reply` applies it on the +//! reply. For hints, `try_send` drops hints when worker is busy. +//! - **`on_upstream_hint` has no reply-side timeout**: hint delivery +//! uses `try_send` (non-blocking on the async side) but the worker +//! executes the FFI call synchronously with no timeout. A wedged +//! `on_upstream_hint` permanently blocks the worker; subsequent +//! `try_send`s will see the channel full and drop hints, and the +//! next `send_to_worker` for Tick will time out on the send side. +//! - **Detached workers**: `InstanceWorker::Drop` closes the channel +//! but does not join the thread (doing so on a tokio worker would +//! block the runtime). A worker stuck in a long FFI call is +//! detached and will run to completion; a debug-level log is emitted. +//! - **One OS thread per instance**: each plugin instance consumes an +//! OS thread for its entire lifetime. This is acceptable for the +//! expected instance counts but would not scale to thousands of +//! concurrent instances. use anyhow::Result; use async_trait::async_trait; use libloading::Library; use std::ffi::{c_void, CString}; -use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::panic::{catch_unwind, AssertUnwindSafe}; +use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicUsize, Ordering}; use std::sync::Arc; use streamkit_core::control::NodeControlMessage; -use streamkit_core::telemetry::TelemetryEvent; +use streamkit_core::telemetry::{TelemetryEmitter, TelemetryEvent}; use streamkit_core::types::Packet; use streamkit_core::{ AudioFramePool, InputPin, NodeContext, NodeState, NodeStateUpdate, OutputPin, ProcessorNode, @@ -24,24 +75,173 @@ use streamkit_plugin_sdk_native::{ conversions, types::{ CAllocAudioResult, CAllocVideoResult, CNativePluginAPI, CNodeCallbacks, CPacket, - CPluginHandle, CResult, + CPluginHandle, CResult, CSetLogEnabledCallback, }, }; -use tracing::{error, info, warn}; +use tracing::{error, info, warn, Instrument}; +use crate::metrics::{CallOutcome, PluginMetrics}; use crate::PluginMetadata; +use opentelemetry::KeyValue; + +/// Global metrics instance for native plugin FFI calls. +static PLUGIN_METRICS: std::sync::OnceLock = std::sync::OnceLock::new(); + +fn global_metrics() -> &'static PluginMetrics { + PLUGIN_METRICS.get_or_init(PluginMetrics::new) +} + +// ── Host-side FFI panic guards ───────────────────────────────────────────── +// +// These guard the `extern "C"` callbacks that the host exposes to plugins. +// A misbehaving plugin could trigger a panic in host code (e.g. via a +// poisoned mutex or an unexpected null pointer); without `catch_unwind` the +// panic would unwind across the C ABI boundary — instant UB. +// +// All guards are implemented in terms of `ffi_guard_with`, which handles +// catch_unwind + panic message extraction + logging. The `on_panic` +// closure receives the formatted message so the caller can embed it in +// the return value if desired. + +/// Generic host-side panic guard. +/// +/// Runs `f` inside [`catch_unwind`]. On panic, extracts a human-readable +/// message, logs it at `error!` level with `label`, and calls `on_panic` +/// to produce a fallback return value. +/// +/// **Note:** `on_panic` runs *outside* the `catch_unwind`. If it panics +/// (e.g. `error_to_c` hits a poisoned `RefCell`), the panic will +/// propagate across the C ABI — UB. All current `on_panic` impls are +/// trivial (construct a null result or call the infallible +/// `error_to_c`), so this is not a practical concern today. +fn ffi_guard_with(label: &str, on_panic: impl FnOnce(String) -> T, f: impl FnOnce() -> T) -> T { + match catch_unwind(AssertUnwindSafe(f)) { + Ok(value) => value, + Err(payload) => { + let msg = streamkit_plugin_sdk_native::ffi_guard::panic_message(&*payload); + error!("{label}: {msg}"); + on_panic(format!("{label}: {msg}")) + }, + } +} + +/// Guard a host callback that returns [`CResult`]. +fn ffi_guard_result(f: impl FnOnce() -> CResult) -> CResult { + ffi_guard_with("host callback panicked", |msg| CResult::error(conversions::error_to_c(msg)), f) +} + +/// Guard a host callback that returns [`CAllocVideoResult`]. +fn ffi_guard_alloc_video(f: impl FnOnce() -> CAllocVideoResult) -> CAllocVideoResult { + ffi_guard_with("host video allocation callback panicked", |_| CAllocVideoResult::null(), f) +} + +/// Guard a host callback that returns [`CAllocAudioResult`]. +fn ffi_guard_alloc_audio(f: impl FnOnce() -> CAllocAudioResult) -> CAllocAudioResult { + ffi_guard_with("host audio allocation callback panicked", |_| CAllocAudioResult::null(), f) +} + +/// Guard a host callback that returns nothing (e.g. logging, buffer free). +fn ffi_guard_unit(f: impl FnOnce()) { + ffi_guard_with( + "host callback panicked", + |_| {}, + || { + f(); + }, + ); +} + +/// Default timeout for plugin FFI calls (process_packet, flush, tick). +/// +/// 5 minutes — generous to support slow plugins (e.g. ML inference). +/// Overridden per-instance when `native_call_timeout_secs` is set in +/// the skit config. Also used as the backstop when the reply-side +/// timeout is configured as `None` (see [`InstanceWorker::await_reply`]). +pub(crate) const DEFAULT_CALL_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(300); + +/// Capacity of the per-instance worker request channel. +/// +/// A depth of 1 provides natural back-pressure: callers block on +/// `send_to_worker` until the worker drains the previous request, +/// which is the desired serialisation of FFI calls. +const WORKER_CHANNEL_CAPACITY: usize = 1; + +/// Wrapper preserving pointer provenance for the plugin's API vtable. +/// +/// The raw pointer is derived from `&'static CNativePluginAPI` which lives +/// inside the loaded `Library`. The library is kept alive by +/// `InstanceState::library: Arc`, so the pointee is valid for +/// the lifetime of the `InstanceState`. +/// +/// # Safety +/// +/// `ApiPtr` is `Send + Sync` because the pointee (`CNativePluginAPI`) is +/// a static vtable of function pointers — never mutated after construction +/// — and the `Library` keeping it alive is behind an `Arc`. +struct ApiPtr(*const CNativePluginAPI); + +// SAFETY: The pointee is a static vtable of function pointers, never mutated +// after construction. The `Library` owning the symbol is kept alive by an `Arc`. +unsafe impl Send for ApiPtr {} +// SAFETY: Same as Send — immutable vtable behind Arc. +unsafe impl Sync for ApiPtr {} + +/// RAII guard for an in-flight FFI call. +/// +/// Created by [`InstanceState::begin_call`]. Dropping the guard +/// decrements `in_flight_calls` (and triggers deferred destroy if +/// this was the last call and `drop_requested` is set). +/// +/// This ensures `finish_call` runs even if the body panics, early-returns, +/// or propagates an error via `?`. +/// +/// The borrow ties the guard's lifetime to the `InstanceState`. The +/// worker thread keeps the `Arc` alive for the guard's +/// entire lifetime. +struct CallGuard<'a> { + state: &'a InstanceState, + handle: CPluginHandle, +} + +impl CallGuard<'_> { + /// Returns the plugin handle for use in FFI calls. + const fn handle(&self) -> CPluginHandle { + self.handle + } +} + +impl Drop for CallGuard<'_> { + fn drop(&mut self) { + self.state.finish_call(); + } +} struct InstanceState { + /// Prevents the shared library from being unloaded while the + /// instance is alive. Never read directly — kept for its `Drop`. + #[allow(dead_code)] library: Arc, - api_addr: usize, - handle_addr: AtomicUsize, + api: ApiPtr, + handle: AtomicPtr, in_flight_calls: AtomicUsize, drop_requested: AtomicBool, - /// Plugin's declared API version (6, 7, or 8). Used to avoid sending - /// `BinaryWithMeta` packets to v6 plugins that don't understand them. - /// v7 plugins understand BinaryWithMeta but not EncodedAudio metadata - /// (which is fine — EncodedAudio is metadata-only, not a runtime packet). + /// One-shot flag: set after the first timeout warning so a wedged + /// plugin does not spam `warn!` on every subsequent FFI call. + timeout_warned: AtomicBool, + /// Plugin's declared API version (6–9). Used to gate features: + /// - v6: downgrade BinaryWithMeta to plain Binary + /// - v9+: enable zero-copy binary buffer_handle api_version: u32, + call_timeout: Option, + /// Raw plugin kind (e.g. `whisper`), not the namespaced form + /// (`plugin::native::whisper`) used in pipeline YAML / server logs. + /// Used for metric labels and worker-thread error!() fields. + plugin_kind: String, + /// Pre-built metric labels per operation, avoiding per-call heap allocation. + labels_process: [KeyValue; 2], + labels_flush: [KeyValue; 2], + labels_tick: [KeyValue; 2], + labels_update_params: [KeyValue; 2], } impl InstanceState { @@ -50,109 +250,709 @@ impl InstanceState { api: &'static CNativePluginAPI, handle: CPluginHandle, api_version: u32, + call_timeout: Option, + plugin_kind: String, ) -> Self { + let labels_process = PluginMetrics::build_labels(&plugin_kind, "process_packet"); + let labels_flush = PluginMetrics::build_labels(&plugin_kind, "flush"); + let labels_tick = PluginMetrics::build_labels(&plugin_kind, "tick"); + let labels_update_params = PluginMetrics::build_labels(&plugin_kind, "update_params"); Self { library, - api_addr: std::ptr::from_ref(api) as usize, - handle_addr: AtomicUsize::new(handle as usize), + api: ApiPtr(std::ptr::from_ref(api)), + handle: AtomicPtr::new(handle), in_flight_calls: AtomicUsize::new(0), drop_requested: AtomicBool::new(false), + timeout_warned: AtomicBool::new(false), api_version, + call_timeout, + plugin_kind, + labels_process, + labels_flush, + labels_tick, + labels_update_params, } } const fn api(&self) -> &'static CNativePluginAPI { - // SAFETY: api_addr was created from a valid &'static CNativePluginAPI reference. - // The loaded library is kept alive by self.library (Arc) held by this state, - // which is itself held by any in-flight spawn_blocking tasks. - unsafe { &*(self.api_addr as *const CNativePluginAPI) } + // SAFETY: The pointer was derived from a valid &'static CNativePluginAPI reference + // via std::ptr::from_ref, preserving provenance. The loaded library is kept alive + // by self.library (Arc) held by this state. + unsafe { &*self.api.0 } } - fn begin_call(&self) -> Option { - self.in_flight_calls.fetch_add(1, Ordering::AcqRel); + /// Acquire a guard for an in-flight FFI call. + /// + /// Returns `None` if the instance has been destroyed or a drop has been + /// requested. Uses Dekker-style mutual exclusion with [`request_drop`]: + /// + /// ```text + /// begin_call: write(in_flight, SeqCst) → read(drop_requested, SeqCst) + /// request_drop: write(drop_requested, SeqCst) → read(in_flight, SeqCst) + /// ``` + /// + /// `SeqCst` on both sides guarantees at least one side observes the + /// other's write, closing the TOCTOU window between incrementing + /// `in_flight_calls` and loading the handle. + fn begin_call(&self) -> Option> { + self.in_flight_calls.fetch_add(1, Ordering::SeqCst); + + // Dekker re-check: if drop was requested, roll back. A + // concurrent request_drop may have seen our in_flight == 1 and + // deferred destroy. If our rollback brings the count back to 0, + // we are responsible for triggering destroy (idempotent). + if self.drop_requested.load(Ordering::SeqCst) { + let prev = self.in_flight_calls.fetch_sub(1, Ordering::SeqCst); + if prev == 1 { + self.destroy_instance(); + } + return None; + } - let handle_addr = self.handle_addr.load(Ordering::Acquire); - if handle_addr == 0 { - self.in_flight_calls.fetch_sub(1, Ordering::AcqRel); + let h = self.handle.load(Ordering::Acquire); + if h.is_null() { + self.in_flight_calls.fetch_sub(1, Ordering::SeqCst); return None; } - Some(handle_addr as CPluginHandle) + Some(CallGuard { state: self, handle: h }) } + /// Decrement `in_flight_calls`; if this was the last call and + /// `drop_requested` is set, trigger `destroy_instance`. + /// + /// Both `finish_call` and `begin_call` (rollback path) may call + /// `destroy_instance` — this is benign because `destroy_instance` + /// is idempotent (atomic null-swap guard). fn finish_call(&self) { - let prev = self.in_flight_calls.fetch_sub(1, Ordering::AcqRel); - debug_assert!(prev > 0, "finish_call called without begin_call"); + let prev = + match self.in_flight_calls.fetch_update(Ordering::SeqCst, Ordering::SeqCst, |current| { + Some(current.saturating_sub(1)) + }) { + Ok(prev) | Err(prev) => prev, + }; + if prev == 0 { + error!(plugin_kind = %self.plugin_kind, "finish_call called without begin_call"); + return; + } - if prev == 1 && self.drop_requested.load(Ordering::Acquire) { + if prev == 1 && self.drop_requested.load(Ordering::SeqCst) { self.destroy_instance(); } } + /// Signal that the wrapper is done with this instance. + /// + /// If no calls are in flight, destroys immediately. Otherwise, + /// defers to the last [`finish_call`]. Uses `SeqCst` ordering + /// to form a Dekker pair with [`begin_call`]. fn request_drop(&self) { - self.drop_requested.store(true, Ordering::Release); - if self.in_flight_calls.load(Ordering::Acquire) == 0 { + self.drop_requested.store(true, Ordering::SeqCst); + if self.in_flight_calls.load(Ordering::SeqCst) == 0 { self.destroy_instance(); } } fn destroy_instance(&self) { - let handle_addr = self.handle_addr.swap(0, Ordering::AcqRel); - if handle_addr == 0 { + let h = self.handle.swap(std::ptr::null_mut(), Ordering::SeqCst); + if h.is_null() { return; } - // Keep the library alive for the duration of the destroy call. - let _lib = Arc::clone(&self.library); let api = self.api(); - (api.destroy_instance)(handle_addr as CPluginHandle); + // Wrap in ffi_guard_unit so a panicking destroy (e.g. via + // InstanceState::Drop) cannot unwind across the C ABI boundary + // or trigger a double-panic abort. + ffi_guard_unit(|| (api.destroy_instance)(h)); + } +} + +impl Drop for InstanceState { + fn drop(&mut self) { + // Defense-in-depth: if NativeNodeWrapper::drop failed to call + // request_drop (or if request_drop deferred), make one last + // attempt. destroy_instance is idempotent (null-swap guard). + self.destroy_instance(); + } +} + +/// Consistent error returned when the worker channel is closed (the worker +/// thread has exited, either normally or via a panic). +fn worker_died_error(op: &str, node: &str) -> StreamKitError { + StreamKitError::Runtime(format!("Worker thread for node {node} died during {op}")) +} + +// ── Per-instance worker thread types ─────────────────────────────────────── + +/// Message sent from the async side to the worker thread. +enum WorkerRequest { + Process { pin_index: usize, packet: Packet, reply: tokio::sync::oneshot::Sender }, + Flush { reply: tokio::sync::oneshot::Sender }, + Tick { reply: tokio::sync::oneshot::Sender }, + UpdateParams { params_cstr: CString, reply: tokio::sync::oneshot::Sender> }, + OnUpstreamHint { hints: Vec, reply: tokio::sync::oneshot::Sender<()> }, +} + +struct WorkerReply { + outputs: Vec<(String, Packet)>, + error: Option, + done: bool, +} + +struct InstanceWorker { + tx: tokio::sync::mpsc::Sender, + node_id: String, +} + +impl Drop for InstanceWorker { + fn drop(&mut self) { + // Dropping `tx` closes the channel so the worker's `blocking_recv` + // returns `None`. The worker thread is detached — do NOT join + // synchronously. This Drop runs on a tokio worker thread (inside + // async run_processor/run_source), so a blocking join would stall the + // async runtime if the worker is stuck in a long FFI call (e.g. after + // a timeout). The worker holds an Arc which ensures the + // plugin instance stays alive until the FFI call completes; + // request_drop/destroy_instance handle deferred cleanup. + tracing::debug!(node = %self.node_id, "Detaching plugin worker thread"); + } +} + +/// Main loop for the per-instance worker thread. +/// +/// Receives [`WorkerRequest`] messages, dispatches FFI calls through the +/// existing [`CallGuard`] / [`begin_call`](InstanceState::begin_call) pattern, +/// and replies via oneshot channels. A single reusable `Vec` for output +/// packets is kept across calls to avoid per-packet allocation. +/// +/// If the worker panics outside a per-call guard, Rust unwinding still drops any +/// active [`CallGuard`] and the worker's `Arc`. Once all +/// references are released, the idempotent [`InstanceState::drop`] path destroys +/// the plugin instance. +#[allow(clippy::too_many_lines, clippy::too_many_arguments, clippy::needless_pass_by_value)] +fn worker_thread_main( + mut rx: tokio::sync::mpsc::Receiver, + state: Arc, + pin_names: Vec, + telemetry_tx: Option>, + session_id: Option, + node_id: String, + video_pool: Option>, + audio_pool: Option>, +) { + // High-water-mark: capacity grows to the largest output batch seen and + // stays there for the instance lifetime (e.g. a one-time flush burst). + let mut reusable_outputs: Vec<(String, Packet)> = Vec::new(); + + let metrics = global_metrics(); + let plugin_kind = &state.plugin_kind; + + while let Some(request) = rx.blocking_recv() { + match request { + WorkerRequest::Process { pin_index, packet, reply } => { + let Some(guard) = state.begin_call() else { + let _ = reply.send(WorkerReply { + outputs: Vec::new(), + error: Some("Instance destroyed during process_packet".to_string()), + done: false, + }); + continue; + }; + + let api = state.api(); + + // Widen catch_unwind to cover packet conversion, clone + // setup, and the FFI call itself so that a panic in any + // of these steps produces a clean error instead of + // poisoning the worker. + let mut ctx_out = std::mem::take(&mut reusable_outputs); + let start = std::time::Instant::now(); + let panic_result = catch_unwind(AssertUnwindSafe(|| { + let mut packet_repr = conversions::packet_to_c(&packet); + if state.api_version < 7 { + packet_repr.downgrade_binary_with_meta(); + } else if state.api_version >= 9 { + // Enable zero-copy binary transfer for v9 plugins. + if let Packet::Binary { ref data, .. } = packet { + packet_repr.set_binary_buffer_handle(data); + } + } + + let mut callback_ctx = CallbackContext { + output_packets: std::mem::take(&mut ctx_out), + error: None, + telemetry_tx: telemetry_tx.clone(), + session_id: session_id.clone(), + node_id: node_id.clone(), + video_pool: video_pool.clone(), + audio_pool: audio_pool.clone(), + }; + + let callback_data = (&raw mut callback_ctx).cast::(); + let node_callbacks = build_node_callbacks(callback_data); + + let result = (api.process_packet)( + guard.handle(), + pin_names[pin_index].as_ptr(), + &raw const packet_repr.packet, + &raw const node_callbacks, + ); + + (result, callback_ctx) + })); + let duration = start.elapsed(); + + let (reply_outputs, error) = match panic_result { + Ok((result, mut callback_ctx)) => { + let err = if result.success { + callback_ctx.error + } else { + let error_msg = if result.error_message.is_null() { + "Unknown plugin error".to_string() + } else { + unsafe { + conversions::c_str_to_string(result.error_message) + .unwrap_or_else(|_| "Unknown plugin error".to_string()) + } + }; + Some(error_msg) + }; + let outcome = + if err.is_none() { CallOutcome::Success } else { CallOutcome::Error }; + metrics.record(&state.labels_process, duration.as_secs_f64(), outcome); + let outputs: Vec<_> = callback_ctx.output_packets.drain(..).collect(); + reusable_outputs = callback_ctx.output_packets; + (outputs, err) + }, + Err(payload) => { + let msg = streamkit_plugin_sdk_native::ffi_guard::panic_message(&*payload); + error!(plugin_kind = %plugin_kind, node_id = %node_id, "Plugin process_packet panicked: {msg}"); + metrics.record( + &state.labels_process, + duration.as_secs_f64(), + CallOutcome::Panic, + ); + // reusable_outputs capacity is lost on panic. + (Vec::new(), Some(format!("Plugin process_packet panicked: {msg}"))) + }, + }; + + drop(guard); + let _ = reply.send(WorkerReply { outputs: reply_outputs, error, done: false }); + }, + + WorkerRequest::Flush { reply } => { + let Some(guard) = state.begin_call() else { + let _ = reply.send(WorkerReply { + outputs: Vec::new(), + error: Some("Instance destroyed during flush".to_string()), + done: false, + }); + continue; + }; + + let api = state.api(); + + let mut ctx_out = std::mem::take(&mut reusable_outputs); + let start = std::time::Instant::now(); + let panic_result = catch_unwind(AssertUnwindSafe(|| { + let mut callback_ctx = CallbackContext { + output_packets: std::mem::take(&mut ctx_out), + error: None, + telemetry_tx: telemetry_tx.clone(), + session_id: session_id.clone(), + node_id: node_id.clone(), + video_pool: video_pool.clone(), + audio_pool: audio_pool.clone(), + }; + + let callback_data = (&raw mut callback_ctx).cast::(); + let node_callbacks = build_node_callbacks(callback_data); + + let result = (api.flush)(guard.handle(), &raw const node_callbacks); + (result, callback_ctx) + })); + let duration = start.elapsed(); + + let (reply_outputs, error) = match panic_result { + Ok((result, mut callback_ctx)) => { + let err = if result.success { + callback_ctx.error + } else { + let error_msg = if result.error_message.is_null() { + "Plugin flush failed".to_string() + } else { + unsafe { + conversions::c_str_to_string(result.error_message) + .unwrap_or_else(|_| "Plugin flush failed".to_string()) + } + }; + Some(error_msg) + }; + let outcome = + if err.is_none() { CallOutcome::Success } else { CallOutcome::Error }; + metrics.record(&state.labels_flush, duration.as_secs_f64(), outcome); + let outputs: Vec<_> = callback_ctx.output_packets.drain(..).collect(); + reusable_outputs = callback_ctx.output_packets; + (outputs, err) + }, + Err(payload) => { + let msg = streamkit_plugin_sdk_native::ffi_guard::panic_message(&*payload); + error!(plugin_kind = %plugin_kind, node_id = %node_id, "Plugin flush panicked: {msg}"); + metrics.record( + &state.labels_flush, + duration.as_secs_f64(), + CallOutcome::Panic, + ); + // reusable_outputs capacity is lost on panic. + (Vec::new(), Some(format!("Plugin flush panicked: {msg}"))) + }, + }; + + drop(guard); + let _ = reply.send(WorkerReply { outputs: reply_outputs, error, done: false }); + }, + + WorkerRequest::Tick { reply } => { + let Some(tick_fn) = state.api().tick else { + let _ = reply.send(WorkerReply { + outputs: Vec::new(), + error: Some("Source plugin missing tick function".to_string()), + done: false, + }); + continue; + }; + + let Some(guard) = state.begin_call() else { + let _ = reply.send(WorkerReply { + outputs: Vec::new(), + error: Some("Instance handle is null".to_string()), + done: false, + }); + continue; + }; + + let mut ctx_out = std::mem::take(&mut reusable_outputs); + let start = std::time::Instant::now(); + let panic_result = catch_unwind(AssertUnwindSafe(|| { + let mut callback_ctx = CallbackContext { + output_packets: std::mem::take(&mut ctx_out), + error: None, + telemetry_tx: telemetry_tx.clone(), + session_id: session_id.clone(), + node_id: node_id.clone(), + video_pool: video_pool.clone(), + audio_pool: audio_pool.clone(), + }; + + let callback_data = (&raw mut callback_ctx).cast::(); + let node_callbacks = build_node_callbacks(callback_data); + + let result = tick_fn(guard.handle(), &raw const node_callbacks); + (result, callback_ctx) + })); + let duration = start.elapsed(); + + let (reply_outputs, error, done) = match panic_result { + Ok((result, mut callback_ctx)) => { + let err = if result.result.success { + callback_ctx.error + } else if result.result.error_message.is_null() { + Some("Source tick failed".to_string()) + } else { + Some(unsafe { + conversions::c_str_to_string(result.result.error_message) + .unwrap_or_else(|_| "Source tick failed".to_string()) + }) + }; + let outcome = + if err.is_none() { CallOutcome::Success } else { CallOutcome::Error }; + metrics.record(&state.labels_tick, duration.as_secs_f64(), outcome); + let outputs: Vec<_> = callback_ctx.output_packets.drain(..).collect(); + reusable_outputs = callback_ctx.output_packets; + (outputs, err, result.done) + }, + Err(payload) => { + let msg = streamkit_plugin_sdk_native::ffi_guard::panic_message(&*payload); + error!(plugin_kind = %plugin_kind, node_id = %node_id, "Plugin tick panicked: {msg}"); + metrics.record( + &state.labels_tick, + duration.as_secs_f64(), + CallOutcome::Panic, + ); + // reusable_outputs capacity is lost on panic. + (Vec::new(), Some(format!("Plugin tick panicked: {msg}")), false) + }, + }; + + drop(guard); + let _ = reply.send(WorkerReply { outputs: reply_outputs, error, done }); + }, + + WorkerRequest::UpdateParams { params_cstr, reply } => { + let Some(guard) = state.begin_call() else { + let _ = reply.send(Some("Instance destroyed during update_params".to_string())); + continue; + }; + + let api = state.api(); + + let start = std::time::Instant::now(); + let panic_msg = catch_unwind(AssertUnwindSafe(|| { + (api.update_params)(guard.handle(), params_cstr.as_ptr()) + })); + let duration = start.elapsed(); + drop(guard); + + let error = match panic_msg { + Ok(result) => { + let outcome = + if result.success { CallOutcome::Success } else { CallOutcome::Error }; + metrics.record( + &state.labels_update_params, + duration.as_secs_f64(), + outcome, + ); + if result.success { + None + } else if result.error_message.is_null() { + Some("Failed to update parameters".to_string()) + } else { + unsafe { + Some( + conversions::c_str_to_string(result.error_message) + .unwrap_or_else(|_| { + "Failed to update parameters".to_string() + }), + ) + } + } + }, + Err(payload) => { + let msg = streamkit_plugin_sdk_native::ffi_guard::panic_message(&*payload); + error!(plugin_kind = %plugin_kind, node_id = %node_id, "Plugin update_params panicked: {msg}"); + metrics.record( + &state.labels_update_params, + duration.as_secs_f64(), + CallOutcome::Panic, + ); + Some(format!("Plugin update_params panicked: {msg}")) + }, + }; + + let _ = reply.send(error); + }, + + // Hints are best-effort advisory signals, not perf-critical FFI + // calls — intentionally not instrumented with metrics. + WorkerRequest::OnUpstreamHint { hints, reply } => { + if let Some(on_hint_fn) = state.api().on_upstream_hint { + for c_str in &hints { + let Some(guard) = state.begin_call() else { + tracing::trace!(node = %node_id, "Dropping hint: instance being destroyed"); + break; + }; + match catch_unwind(AssertUnwindSafe(|| { + on_hint_fn(guard.handle(), c_str.as_ptr()) + })) { + Ok(result) if !result.success => { + let msg = if result.error_message.is_null() { + "on_upstream_hint failed".to_string() + } else { + unsafe { + conversions::c_str_to_string(result.error_message) + .unwrap_or_else(|_| { + "on_upstream_hint failed".to_string() + }) + } + }; + warn!(node = %node_id, "on_upstream_hint error: {msg}"); + }, + Err(payload) => { + let msg = streamkit_plugin_sdk_native::ffi_guard::panic_message( + &*payload, + ); + error!(plugin_kind = %plugin_kind, node_id = %node_id, "Plugin on_upstream_hint panicked: {msg}"); + }, + Ok(_) => {}, + } + } + } + let _ = reply.send(()); + }, + } + } +} + +const PLUGIN_LOG_FIELD_NAMES: &[&str] = &["message"]; + +/// Callsite anchor for dynamically-constructed [`tracing::Metadata`]. +struct PluginLogCallsite; + +impl tracing::callsite::Callsite for PluginLogCallsite { + fn set_interest(&self, _: tracing::subscriber::Interest) {} + fn metadata(&self) -> &tracing::Metadata<'_> { + // Per-plugin targets are stored in `PLUGIN_LOG_METADATA_CACHE`; this + // fallback exists only to satisfy the `Callsite` trait. + static FALLBACK: std::sync::LazyLock> = + std::sync::LazyLock::new(|| { + tracing::Metadata::new( + "plugin_log_callsite", + "plugin", + tracing::Level::TRACE, + None, + None, + None, + plugin_log_field_set(), + tracing::metadata::Kind::EVENT, + ) + }); + &FALLBACK + } +} + +static PLUGIN_LOG_CALLSITE: PluginLogCallsite = PluginLogCallsite; + +struct PluginLogMetadata { + metadata: tracing::Metadata<'static>, + message_field: tracing::field::Field, +} + +static PLUGIN_LOG_METADATA_CACHE: std::sync::LazyLock< + std::sync::Mutex< + std::collections::HashMap<(String, tracing::Level), &'static PluginLogMetadata>, + >, +> = std::sync::LazyLock::new(|| std::sync::Mutex::new(std::collections::HashMap::new())); + +/// Build the shared field set for plugin log events. +/// +/// Called in both the early-out filter probe and the cached metadata +/// path; the two calls must produce value-equal `FieldSet`s (same +/// field names, same callsite identifier) so that `tracing`'s field +/// lookup is consistent. This is guaranteed as long as the inputs +/// (`PLUGIN_LOG_FIELD_NAMES` and `PLUGIN_LOG_CALLSITE`) are unchanged. +fn plugin_log_field_set() -> tracing::field::FieldSet { + tracing::field::FieldSet::new( + PLUGIN_LOG_FIELD_NAMES, + tracing::callsite::Identifier(&PLUGIN_LOG_CALLSITE), + ) +} + +fn plugin_log_static_metadata(target: &str, level: tracing::Level) -> &'static PluginLogMetadata { + let mut cache = + PLUGIN_LOG_METADATA_CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let key = (target.to_string(), level); + cache.entry(key).or_insert_with_key(|(target, level)| { + let target: &'static str = Box::leak(target.clone().into_boxed_str()); + let metadata = plugin_log_metadata(target, *level, plugin_log_field_set()); + let Some(message_field) = metadata.fields().field("message") else { + unreachable!("plugin log field set must contain message"); + }; + let entry = PluginLogMetadata { metadata, message_field }; + Box::leak(Box::new(entry)) + }) +} + +const fn plugin_log_metadata( + target: &str, + level: tracing::Level, + field_set: tracing::field::FieldSet, +) -> tracing::Metadata<'_> { + tracing::Metadata::new( + "plugin_log", + target, + level, + None, + None, + None, + field_set, + tracing::metadata::Kind::EVENT, + ) +} + +const fn plugin_log_level(level: streamkit_plugin_sdk_native::types::CLogLevel) -> tracing::Level { + use streamkit_plugin_sdk_native::types::CLogLevel; + + match level { + CLogLevel::Trace => tracing::Level::TRACE, + CLogLevel::Debug => tracing::Level::DEBUG, + CLogLevel::Info => tracing::Level::INFO, + CLogLevel::Warn => tracing::Level::WARN, + CLogLevel::Error => tracing::Level::ERROR, } } -/// C callback function for plugin logging -/// Routes plugin logs to the tracing infrastructure -#[allow(clippy::cognitive_complexity)] +/// C callback to check whether a log level is enabled for a given target. +/// +/// Consults the tracing subscriber with per-target metadata so that +/// directives like `RUST_LOG=whisper=debug` correctly filter plugin logs. +/// Used by v9 plugins via `set_log_enabled_callback`. +extern "C" fn plugin_log_enabled_callback( + level: streamkit_plugin_sdk_native::types::CLogLevel, + target: *const std::os::raw::c_char, + _user_data: *mut c_void, +) -> bool { + ffi_guard_with( + "plugin_log_enabled_callback panicked", + |_| true, + || { + let target_str = if target.is_null() { + "plugin" + } else { + unsafe { std::ffi::CStr::from_ptr(target) }.to_str().unwrap_or("plugin") + }; + + let metadata = + plugin_log_metadata(target_str, plugin_log_level(level), plugin_log_field_set()); + tracing::dispatcher::get_default(|d| d.enabled(&metadata)) + }, + ) +} + +/// C callback function for plugin logging. +/// Routes plugin logs to the tracing infrastructure. extern "C" fn plugin_log_callback( level: streamkit_plugin_sdk_native::types::CLogLevel, target: *const std::os::raw::c_char, message: *const std::os::raw::c_char, _user_data: *mut c_void, ) { - use streamkit_plugin_sdk_native::{conversions, types::CLogLevel}; - - // Convert C strings to Rust strings - let target_str = if target.is_null() { - "unknown".to_string() - } else { - unsafe { conversions::c_str_to_string(target) }.unwrap_or_else(|_| "unknown".to_string()) - }; - - let message_str = if message.is_null() { - String::new() - } else { - unsafe { conversions::c_str_to_string(message) } - .unwrap_or_else(|_| "[invalid UTF-8]".to_string()) - }; - - // Route to tracing based on log level - // Use the event! macro which allows dynamic targets - match level { - CLogLevel::Trace => { - tracing::event!(tracing::Level::TRACE, target = %target_str, "{}", message_str); - }, - CLogLevel::Debug => { - tracing::event!(tracing::Level::DEBUG, target = %target_str, "{}", message_str); - }, - CLogLevel::Info => { - tracing::event!(tracing::Level::INFO, target = %target_str, "{}", message_str); - }, - CLogLevel::Warn => { - tracing::event!(tracing::Level::WARN, target = %target_str, "{}", message_str); - }, - CLogLevel::Error => { - tracing::event!(tracing::Level::ERROR, target = %target_str, "{}", message_str); - }, - } + ffi_guard_unit(|| { + use streamkit_plugin_sdk_native::conversions; + + let target_str = if target.is_null() { + "unknown" + } else { + // SAFETY: target is a valid C string from the plugin SDK. + unsafe { std::ffi::CStr::from_ptr(target) }.to_str().unwrap_or("unknown") + }; + + // Check the subscriber filter *before* acquiring the metadata + // cache mutex. This avoids contention on high-volume + // DEBUG/TRACE logs that the subscriber would discard anyway. + let tracing_level = plugin_log_level(level); + let probe = plugin_log_metadata(target_str, tracing_level, plugin_log_field_set()); + let enabled = tracing::dispatcher::get_default(|d| d.enabled(&probe)); + if !enabled { + return; + } + + let message_str = if message.is_null() { + String::new() + } else { + unsafe { conversions::c_str_to_string(message) } + .unwrap_or_else(|_| "[invalid UTF-8]".to_string()) + }; + + let log_meta = plugin_log_static_metadata(target_str, tracing_level); + tracing::dispatcher::get_default(|d| { + d.register_callsite(&log_meta.metadata); + let values = + [(&log_meta.message_field, Some(&message_str as &dyn tracing::field::Value))]; + let value_set = log_meta.metadata.fields().value_set(&values); + d.event(&tracing::Event::new(&log_meta.metadata, &value_set)); + }); + }); } /// Wrapper that implements ProcessorNode for native plugins @@ -161,6 +961,14 @@ pub struct NativeNodeWrapper { metadata: PluginMetadata, } +struct WorkerCallContext<'a> { + op: &'a str, + node: &'a str, + state_tx: Option<&'a tokio::sync::mpsc::Sender>, + telemetry: Option<&'a TelemetryEmitter>, + metric_labels: &'a [KeyValue; 2], +} + impl NativeNodeWrapper { /// Create a new native node wrapper /// @@ -175,6 +983,8 @@ impl NativeNodeWrapper { api: &'static CNativePluginAPI, metadata: PluginMetadata, params: Option<&serde_json::Value>, + call_timeout: Option, + set_log_enabled_callback: Option, ) -> Result { // Convert params to JSON string if provided let params_json = params @@ -201,8 +1011,32 @@ impl NativeNodeWrapper { )); } + // For v9 plugins, inject the log-enabled callback so the plugin + // can short-circuit log formatting when the level is filtered. + // + // SAFETY: `handle` is valid (checked above), `plugin_log_enabled_callback` + // is an `extern "C"` fn, and `null_mut()` matches the `enabled_user_data` + // contract. Both `user_data` and `enabled_user_data` are host-managed + // pointers; if a future host stores per-instance state in + // `enabled_user_data`, it must ensure the pointed-to data is + // Send+Sync-safe. + if api.version >= 9 { + if let Some(set_cb) = set_log_enabled_callback { + ffi_guard_unit(|| { + set_cb(handle, plugin_log_enabled_callback, std::ptr::null_mut()); + }); + } + } + Ok(Self { - state: Arc::new(InstanceState::new(library, api, handle, api.version)), + state: Arc::new(InstanceState::new( + library, + api, + handle, + api.version, + call_timeout, + metadata.kind.clone(), + )), metadata, }) } @@ -219,52 +1053,206 @@ impl ProcessorNode for NativeNodeWrapper { } fn runtime_param_schema(&self) -> Option { - let get_schema = self.state.api().get_runtime_param_schema?; - let handle = self.state.begin_call()?; - - let result = get_schema(handle); - - if !result.success { - // FFI call failed — log and return None. - if !result.error_message.is_null() { - let msg = unsafe { conversions::c_str_to_string(result.error_message) } - .unwrap_or_default(); - warn!(error = %msg, "Plugin runtime_param_schema failed"); - } - self.state.finish_call(); - return None; - } + ffi_guard_with( + "runtime_param_schema panicked", + |_| None, + || { + let get_schema = self.state.api().get_runtime_param_schema?; + let guard = self.state.begin_call()?; + + let result = get_schema(guard.handle()); + + if !result.success { + if !result.error_message.is_null() { + let msg = unsafe { conversions::c_str_to_string(result.error_message) } + .unwrap_or_default(); + warn!(error = %msg, "Plugin runtime_param_schema failed"); + } + return None; + } - // success=true, null json_schema → plugin has no runtime schema. - if result.json_schema.is_null() { - self.state.finish_call(); - return None; - } + if result.json_schema.is_null() { + return None; + } - // success=true, non-null json_schema → JSON string containing the schema. - // SAFETY: result.json_schema points to a thread-local CString set by - // error_to_c (used here as a generic "String → *const c_char" helper). - // We must copy the string BEFORE any other FFI call on this thread - // (including finish_call) that could invoke error_to_c again and - // overwrite the thread-local buffer. - let json_str = unsafe { conversions::c_str_to_string(result.json_schema) }.ok(); - self.state.finish_call(); - json_str.and_then(|s| serde_json::from_str(&s).ok()) + let json_str = unsafe { conversions::c_str_to_string(result.json_schema) }.ok(); + drop(guard); + json_str.and_then(|s| serde_json::from_str(&s).ok()) + }, + ) } // The run method is complex by necessity - it's an async actor managing FFI calls, // control messages, and packet processing. Breaking it up would make the logic harder to follow. #[allow(clippy::too_many_lines)] async fn run(self: Box, context: NodeContext) -> Result<(), StreamKitError> { + let node_name = context.output_sender.node_name().to_string(); + let mode = if self.metadata.is_source { "source" } else { "processor" }; + let span = tracing::info_span!("native_plugin.run", + plugin.kind = %self.metadata.kind, + node.id = %node_name, + mode = %mode, + ); + if self.metadata.is_source { - return self.run_source(context).await; + return self.run_source(context).instrument(span).await; } - self.run_processor(context).await + self.run_processor(context).instrument(span).await } } // ── Private run implementations ──────────────────────────────────────────── impl NativeNodeWrapper { + /// Spawn a dedicated worker thread for this plugin instance. + fn spawn_worker( + state: Arc, + pin_names: Vec, + telemetry_tx: Option>, + session_id: Option, + node_id: String, + video_pool: Option>, + audio_pool: Option>, + ) -> Result { + let (tx, rx) = tokio::sync::mpsc::channel::(WORKER_CHANNEL_CAPACITY); + // Linux caps prctl(PR_SET_NAME) at 15 bytes; the OS silently + // truncates longer names. We just format and let the OS handle it. + let thread_name = format!("skp-{node_id}"); + let worker_node_id = node_id.clone(); + std::thread::Builder::new() + .name(thread_name) + .spawn(move || { + worker_thread_main( + rx, + state, + pin_names, + telemetry_tx, + session_id, + node_id, + video_pool, + audio_pool, + ); + }) + .map_err(|e| { + StreamKitError::Runtime(format!("Failed to spawn plugin worker thread: {e}")) + })?; + Ok(InstanceWorker { tx, node_id: worker_node_id }) + } + + /// Await a oneshot reply from the worker, applying the configured timeout. + /// + /// When `call_timeout` is `Some`, uses that duration; otherwise falls + /// back to [`DEFAULT_CALL_TIMEOUT`] as a backstop so the reply side + /// is never unbounded. + /// + /// When `state_tx` is provided and the call times out, emits + /// [`NodeState::Failed`] so the pipeline coordinator sees the failure + /// even though the worker thread continues running. + /// + /// Bumps `plugin.calls` + `plugin.timeouts` (not `plugin.errors`) so + /// timeouts are distinguishable from FFI errors and from + /// successful-but-slow calls recorded by the worker. + async fn await_reply( + &self, + op: &str, + node: &str, + reply_rx: tokio::sync::oneshot::Receiver, + state_tx: Option<&tokio::sync::mpsc::Sender>, + telemetry: Option<&TelemetryEmitter>, + metric_labels: &[KeyValue; 2], + ) -> Result { + match self.state.call_timeout { + Some(d) => tokio::time::timeout(d, reply_rx) + .await + .map_err(|_| { + if !self.state.timeout_warned.swap(true, Ordering::SeqCst) { + warn!( + node = %node, + "Plugin {op} timed out after {d:?}" + ); + } + global_metrics().record_timeout(metric_labels); + let reason = format!("Plugin {op} on node {node} timed out after {d:?}"); + // Best-effort notification — try_send may drop if the + // state channel is full, but that is acceptable because + // the Err returned from await_reply is the real guarantee + // that the caller observes the timeout. + if let Some(tx) = state_tx { + let _ = tx.try_send(NodeStateUpdate::new( + node.to_string(), + NodeState::Failed { reason: reason.clone() }, + )); + } + if let Some(telemetry) = telemetry { + telemetry.emit("plugin.call_timeout", serde_json::json!({ "op": op })); + } + StreamKitError::Runtime(reason) + })? + .map_err(|_| StreamKitError::Runtime("Worker reply channel dropped".into())), + None => tokio::time::timeout(DEFAULT_CALL_TIMEOUT, reply_rx) + .await + .map_err(|_| { + let reason = format!( + "Plugin {op} on node {node} timed out after {DEFAULT_CALL_TIMEOUT:?} (backstop)" + ); + global_metrics().record_timeout(metric_labels); + if let Some(tx) = state_tx { + let _ = tx.try_send(NodeStateUpdate::new( + node.to_string(), + NodeState::Failed { reason: reason.clone() }, + )); + } + StreamKitError::Runtime(reason) + })? + .map_err(|_| StreamKitError::Runtime("Worker reply channel dropped".into())), + } + } + + /// Send a request to the worker with timeout, preventing indefinite + /// blocking when a prior FFI call has wedged the worker. + /// + /// Uses [`Self::state.call_timeout`] when configured; otherwise falls back + /// to [`DEFAULT_CALL_TIMEOUT`] so the send side stays bounded even when + /// reply-side waiting is disabled with `set_call_timeout(None)`. + async fn send_to_worker( + &self, + call: WorkerCallContext<'_>, + tx: &tokio::sync::mpsc::Sender, + request: WorkerRequest, + ) -> Result<(), StreamKitError> { + let timeout_dur = self.state.call_timeout.unwrap_or(DEFAULT_CALL_TIMEOUT); + tokio::time::timeout(timeout_dur, tx.send(request)) + .await + .map_err(|_| { + if !self.state.timeout_warned.swap(true, Ordering::SeqCst) { + warn!( + node = %call.node, + "Plugin {} send to worker timed out after {timeout_dur:?}", + call.op, + ); + } + global_metrics().record_timeout(call.metric_labels); + let reason = format!( + "Plugin {} on node {}: send to worker timed out \ + (worker likely wedged in prior FFI call)", + call.op, call.node + ); + if let Some(tx) = call.state_tx { + let _ = tx.try_send(NodeStateUpdate::new( + call.node.to_string(), + NodeState::Failed { reason: reason.clone() }, + )); + } + if let Some(telemetry) = call.telemetry { + telemetry.emit( + "plugin.call_timeout", + serde_json::json!({ "op": call.op, "phase": "send_to_worker" }), + ); + } + StreamKitError::Runtime(reason) + })? + .map_err(|_| worker_died_error(call.op, call.node)) + } + /// Input-driven processing loop (existing behaviour for processor plugins). #[allow(clippy::too_many_lines)] async fn run_processor( @@ -299,8 +1287,6 @@ impl NativeNodeWrapper { let (merged_tx, mut merged_rx) = tokio::sync::mpsc::channel::<(usize, Packet)>(context.batch_size.max(1)); let cancellation_token = context.cancellation_token.clone(); - let video_pool = context.video_pool.clone(); - let audio_pool = context.audio_pool.clone(); for (pin_name, mut rx) in inputs.drain() { let pin_cstr = CString::new(pin_name.as_str()).map_err(|e| { @@ -308,7 +1294,7 @@ impl NativeNodeWrapper { })?; let pin_index = input_pin_names.len(); input_pin_names.push(pin_name); - input_pin_cstrs.push(Arc::new(pin_cstr)); + input_pin_cstrs.push(pin_cstr); let tx = merged_tx.clone(); let token = cancellation_token.clone(); @@ -337,11 +1323,39 @@ impl NativeNodeWrapper { drop(merged_tx); + // Spawn the dedicated worker thread for this instance. + let worker = match Self::spawn_worker( + Arc::clone(&self.state), + input_pin_cstrs, + context.telemetry_tx.clone(), + context.session_id.clone(), + node_name.clone(), + context.video_pool.clone(), + context.audio_pool.clone(), + ) { + Ok(w) => w, + Err(e) => { + let _ = context + .state_tx + .send(NodeStateUpdate::new( + node_name.clone(), + NodeState::Failed { reason: e.to_string() }, + )) + .await; + return Err(e); + }, + }; + tracing::debug!( node = %node_name, inputs = ?input_pin_names, "Got input channels, entering main loop" ); + let telemetry = TelemetryEmitter::new( + node_name.clone(), + context.session_id.clone(), + context.telemetry_tx.clone(), + ); // Emit running state if let Err(e) = @@ -352,267 +1366,119 @@ impl NativeNodeWrapper { let mut control_channel_open = true; - // Main processing loop - loop { - tokio::select! { - biased; - - () = async { - match &context.cancellation_token { - Some(token) => token.cancelled().await, - None => std::future::pending().await, + // Run the main processing loop, capturing the result so that + // input_tasks are always aborted on exit — including early returns + // from worker_died_error or timeout. + let loop_result: Result<(), StreamKitError> = async { + loop { + tokio::select! { + biased; + + () = async { + match &context.cancellation_token { + Some(token) => token.cancelled().await, + None => std::future::pending().await, + } + } => { + tracing::info!("Native plugin cancelled"); + break; } - } => { - tracing::info!("Native plugin cancelled"); - break; - } - - maybe_control = context.control_rx.recv(), if control_channel_open => { - match maybe_control { - Some(NodeControlMessage::UpdateParams(params_value)) => { - // Serialize params to JSON string - let params_json = serde_json::to_string(¶ms_value) - .map_err(|e| StreamKitError::Configuration(format!("Failed to serialize params: {e}")))?; - let params_cstr = CString::new(params_json) - .map_err(|e| StreamKitError::Configuration(format!("Invalid params string: {e}")))?; - - // Move the blocking FFI call to spawn_blocking - let state = Arc::clone(&self.state); - let error_msg = tokio::task::spawn_blocking(move || { - let handle = state.begin_call()?; - - let _lib = Arc::clone(&state.library); - let api = state.api(); - let result = (api.update_params)(handle, params_cstr.as_ptr()); - - // Convert error message immediately to String (CResult is not Send) - let error = if result.success { - None - } else if result.error_message.is_null() { - Some("Failed to update parameters".to_string()) - } else { - // SAFETY: The error_message pointer is provided by the plugin - // and is valid for the duration of this call. - unsafe { - Some(conversions::c_str_to_string(result.error_message) - .unwrap_or_else(|_| "Failed to update parameters".to_string())) - } - }; - - state.finish_call(); - error - }) - .await - .map_err(|e| { - StreamKitError::Runtime(format!( - "Update params task panicked: {e}" - )) - })?; - if let Some(err) = error_msg { - warn!(node = %node_name, error = %err, "Parameter update failed"); + maybe_control = context.control_rx.recv(), if control_channel_open => { + match maybe_control { + Some(NodeControlMessage::UpdateParams(params_value)) => { + self.apply_params_update( + &node_name, + ¶ms_value, + &worker.tx, + &context.state_tx, + Some(&telemetry), + ) + .await?; + } + Some(NodeControlMessage::Start) => { + // Native plugins don't implement ready/start lifecycle - ignore + } + Some(NodeControlMessage::Shutdown) => { + tracing::info!("Native plugin received shutdown signal"); + break; + } + None => { + control_channel_open = false; } - } - Some(NodeControlMessage::Start) => { - // Native plugins don't implement ready/start lifecycle - ignore - } - Some(NodeControlMessage::Shutdown) => { - tracing::info!("Native plugin received shutdown signal"); - break; - } - None => { - control_channel_open = false; } } - } - maybe_packet = merged_rx.recv() => { - let Some((pin_index, packet)) = maybe_packet else { - // Input closed - flush any buffered data before shutting down - tracing::debug!(node = %node_name, "Native plugin input closed, flushing buffers"); - - // Call flush to process any remaining buffered data - let state = Arc::clone(&self.state); - let telemetry_tx = context.telemetry_tx.clone(); - let session_id = context.session_id.clone(); - let node_id = node_name.clone(); - - let (outputs, error) = tokio::task::spawn_blocking(move || { - let Some(handle) = state.begin_call() else { - return (Vec::new(), None); - }; + maybe_packet = merged_rx.recv() => { + let Some((pin_index, packet)) = maybe_packet else { + // Input closed - flush any buffered data before shutting down + tracing::debug!(node = %node_name, "Native plugin input closed, flushing buffers"); - let _lib = Arc::clone(&state.library); - let api = state.api(); - - let mut callback_ctx = CallbackContext { - output_packets: Vec::new(), - error: None, - telemetry_tx, - session_id, - node_id, - video_pool, - audio_pool, - }; + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); + self.send_to_worker(WorkerCallContext { op: "flush", node: &node_name, state_tx: Some(&context.state_tx), telemetry: Some(&telemetry), metric_labels: &self.state.labels_flush }, &worker.tx, WorkerRequest::Flush { reply: reply_tx }).await?; + let reply = self.await_reply("flush", &node_name, reply_rx, Some(&context.state_tx), Some(&telemetry), &self.state.labels_flush).await?; - let callback_data = (&raw mut callback_ctx).cast::(); - let node_callbacks = build_node_callbacks(callback_data); + // Send flush outputs + for (pin, pkt) in reply.outputs { + if context.output_sender.send(&pin, pkt).await.is_err() { + tracing::debug!("Output channel closed during flush"); + } + } - // Call plugin's flush function - tracing::info!("Calling api.flush()"); - let result = (api.flush)( - handle, - &raw const node_callbacks, - ); - tracing::info!(success = result.success, "Flush returned"); + if let Some(error_msg) = reply.error { + warn!(node = %node_name, error = %error_msg, "Plugin flush failed"); + } - let error = if result.success { - callback_ctx.error - } else { - let error_msg = if result.error_message.is_null() { - "Plugin flush failed".to_string() - } else { - unsafe { - conversions::c_str_to_string(result.error_message) - .unwrap_or_else(|_| "Plugin flush failed".to_string()) - } - }; - Some(error_msg) - }; + break; + }; - let outputs = callback_ctx.output_packets; - state.finish_call(); - (outputs, error) - }) - .await - .map_err(|e| StreamKitError::Runtime(format!("Plugin flush task panicked: {e}")))?; + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); + self.send_to_worker(WorkerCallContext { op: "process_packet", node: &node_name, state_tx: Some(&context.state_tx), telemetry: Some(&telemetry), metric_labels: &self.state.labels_process }, &worker.tx, WorkerRequest::Process { pin_index, packet, reply: reply_tx }).await?; + let reply = self + .await_reply("process_packet", &node_name, reply_rx, Some(&context.state_tx), Some(&telemetry), &self.state.labels_process) + .await?; + let (outputs, error) = (reply.outputs, reply.error); - // Send flush outputs + // Send outputs for (pin, pkt) in outputs { if context.output_sender.send(&pin, pkt).await.is_err() { - tracing::debug!("Output channel closed during flush"); + tracing::debug!("Output channel closed, stopping node"); + break; } } + // Handle errors if let Some(error_msg) = error { - warn!(node = %node_name, error = %error_msg, "Plugin flush failed"); - } + error!(node = %node_name, error = %error_msg, "Plugin process failed"); - break; - }; - - // Move the blocking FFI call to spawn_blocking to avoid blocking the async runtime - let state = Arc::clone(&self.state); - let telemetry_tx = context.telemetry_tx.clone(); - let session_id = context.session_id.clone(); - let node_id = node_name.clone(); - let pin_cstr = Arc::clone(&input_pin_cstrs[pin_index]); - let video_pool = video_pool.clone(); - let audio_pool = audio_pool.clone(); - let (outputs, error) = tokio::task::spawn_blocking(move || { - let Some(handle) = state.begin_call() else { - return (Vec::new(), None); - }; - - let _lib = Arc::clone(&state.library); - let api = state.api(); - // Convert packet to C representation - let mut packet_repr = conversions::packet_to_c(&packet); - - // v6 plugins do not understand BinaryWithMeta (discriminant 10). - // Downgrade to plain Binary so the raw bytes still arrive; the - // metadata/content_type fields are lost but the plugin won't crash. - // Note: no downgrade needed for EncodedAudio (discriminant 11) - // because it is a metadata-only type used in CPacketTypeInfo for - // pin declarations — it never appears in runtime CPacket transport. - if state.api_version < 7 { - packet_repr.downgrade_binary_with_meta(); - } - - // Create callback context - let mut callback_ctx = CallbackContext { - output_packets: Vec::new(), - error: None, - telemetry_tx, - session_id, - node_id, - video_pool, - audio_pool, - }; - - let callback_data = (&raw mut callback_ctx).cast::(); - let node_callbacks = build_node_callbacks(callback_data); - - // Call plugin's process function (BLOCKING - but we're in spawn_blocking) - let result = (api.process_packet)( - handle, - pin_cstr.as_ptr(), - &raw const packet_repr.packet, - &raw const node_callbacks, - ); - - // Check for errors - let error = if result.success { - callback_ctx.error - } else { - let error_msg = if result.error_message.is_null() { - "Unknown plugin error".to_string() - } else { - // SAFETY: The error_message pointer is provided by the plugin - // and is valid for the duration of this call. - unsafe { - conversions::c_str_to_string(result.error_message) - .unwrap_or_else(|_| "Unknown plugin error".to_string()) - } - }; - Some(error_msg) - }; - - let outputs = callback_ctx.output_packets; - state.finish_call(); - (outputs, error) - }) - .await - .map_err(|e| { - StreamKitError::Runtime(format!("Plugin processing task panicked: {e}")) - })?; - - // Now send outputs (after dropping c_packet and result) - for (pin, pkt) in outputs { - if context.output_sender.send(&pin, pkt).await.is_err() { - tracing::debug!("Output channel closed, stopping node"); - break; - } - } - - // Handle errors - if let Some(error_msg) = error { - error!(node = %node_name, error = %error_msg, "Plugin process failed"); - - if let Err(e) = context - .state_tx - .send(NodeStateUpdate::new( - node_name.clone(), - NodeState::Failed { reason: error_msg.clone() }, - )) - .await - { - warn!(error = %e, node = %node_name, "Failed to send failed state"); - } + if let Err(e) = context + .state_tx + .send(NodeStateUpdate::new( + node_name.clone(), + NodeState::Failed { reason: error_msg.clone() }, + )) + .await + { + warn!(error = %e, node = %node_name, "Failed to send failed state"); + } - for handle in &input_tasks { - handle.abort(); + return Err(StreamKitError::Runtime(error_msg)); } - return Err(StreamKitError::Runtime(error_msg)); } } } + Ok(()) } + .await; + // Always abort input-forwarder tasks — including on early-return + // from worker_died_error or timeout. for handle in &input_tasks { handle.abort(); } + loop_result?; + // Input closed, emit stopped state info!(node = %node_name, "Input closed, shutting down"); if let Err(e) = context @@ -634,17 +1500,12 @@ impl NativeNodeWrapper { /// Lifecycle: Initializing → Ready → (wait for Start) → Running → tick loop → Stopped. #[allow(clippy::too_many_lines)] async fn run_source(self: Box, mut context: NodeContext) -> Result<(), StreamKitError> { - // Defined up-front to satisfy `items_after_statements` lint. - struct TickOutcome { - outputs: Vec<(String, Packet)>, - success: bool, - done: bool, - error_msg: Option, - } - let node_name = context.output_sender.node_name().to_string(); - let video_pool = context.video_pool.clone(); - let audio_pool = context.audio_pool.clone(); + let telemetry = TelemetryEmitter::new( + node_name.clone(), + context.session_id.clone(), + context.telemetry_tx.clone(), + ); tracing::info!(node = %node_name, "Native source plugin wrapper starting"); @@ -657,6 +1518,46 @@ impl NativeNodeWrapper { warn!(error = %e, node = %node_name, "Failed to send initializing state"); } + // Verify tick function exists before spawning the worker — a + // missing tick is a misconfiguration that should surface at + // Initializing, not after the Ready→Start handshake. + if self.state.api().tick.is_none() { + let reason = "Source plugin missing tick function".to_string(); + let _ = context + .state_tx + .send(NodeStateUpdate::new( + node_name.clone(), + NodeState::Failed { reason: reason.clone() }, + )) + .await; + return Err(StreamKitError::Runtime(reason)); + } + + // Spawn the dedicated worker thread for this instance. + // Spawned before the Ready→Start handshake so that pre-start + // UpdateParams can be routed through the worker. + let worker = match Self::spawn_worker( + Arc::clone(&self.state), + Vec::new(), // source plugins have no input pins + context.telemetry_tx.clone(), + context.session_id.clone(), + node_name.clone(), + context.video_pool.clone(), + context.audio_pool.clone(), + ) { + Ok(w) => w, + Err(e) => { + let _ = context + .state_tx + .send(NodeStateUpdate::new( + node_name.clone(), + NodeState::Failed { reason: e.to_string() }, + )) + .await; + return Err(e); + }, + }; + // ── Ready → Start handshake ───────────────────────────────────── // Emit Ready so the pipeline coordinator knows this node is waiting // for the Start signal before producing data. @@ -713,7 +1614,14 @@ impl NativeNodeWrapper { } Some(NodeControlMessage::UpdateParams(params_value)) => { // Apply parameter updates even before Start. - self.apply_params_update(&node_name, ¶ms_value).await?; + self.apply_params_update( + &node_name, + ¶ms_value, + &worker.tx, + &context.state_tx, + Some(&telemetry), + ) + .await?; } None => { // Control channel closed before Start — shut down gracefully. @@ -748,25 +1656,22 @@ impl NativeNodeWrapper { let ti = std::time::Duration::from_micros(self.metadata.tick_interval_us.max(1)); (ti, self.metadata.max_ticks) }; - let (tick_interval, max_ticks) = self - .state - .api() - .get_source_config - .and_then(|get_source_config_fn| { - self.state.begin_call().map(|h| { - let cfg = get_source_config_fn(h); - self.state.finish_call(); - let ti = std::time::Duration::from_micros(cfg.tick_interval_us.max(1)); - (ti, cfg.max_ticks) + let (tick_interval, max_ticks) = ffi_guard_with( + "get_source_config panicked", + |_| None, + || { + self.state.api().get_source_config.and_then(|get_source_config_fn| { + self.state.begin_call().map(|guard| { + let cfg = get_source_config_fn(guard.handle()); + let ti = std::time::Duration::from_micros(cfg.tick_interval_us.max(1)); + (ti, cfg.max_ticks) + }) }) - }) - .unwrap_or_else(fallback); + }, + ) + .unwrap_or_else(fallback); let mut tick_count: u64 = 0; - let tick_fn = self.state.api().tick.ok_or_else(|| { - StreamKitError::Runtime("Source plugin missing tick function".to_string()) - })?; - let mut interval = tokio::time::interval(tick_interval); interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); // Consume the first (immediate) tick so we don't double-fire on entry. @@ -781,6 +1686,8 @@ impl NativeNodeWrapper { tokio::sync::mpsc::Receiver, )> = Vec::new(); + let worker_tx = &worker.tx; + loop { // Check tick limit if max_ticks > 0 && tick_count >= max_ticks { @@ -806,7 +1713,14 @@ impl NativeNodeWrapper { return Ok(()); }, NodeControlMessage::UpdateParams(params_value) => { - self.apply_params_update(&node_name, ¶ms_value).await?; + self.apply_params_update( + &node_name, + ¶ms_value, + worker_tx, + &context.state_tx, + Some(&telemetry), + ) + .await?; }, NodeControlMessage::Start => { // Already started — ignore duplicate. @@ -816,11 +1730,6 @@ impl NativeNodeWrapper { // Non-blocking drain of pin management messages to pick up // OutputHintChannel deliveries from the engine. - // NOTE: this consumes ALL variants but only extracts - // OutputHintChannel. Safe today because source plugins - // don't receive AddedOutputPin/RemoveOutputPin/InputTypeResolved. - // If dynamic output pins are added to sources in the future, - // this drain must be updated to handle those variants. if let Some(ref mut pin_mgmt_rx) = context.pin_management_rx { while let Ok(msg) = pin_mgmt_rx.try_recv() { if let streamkit_core::pins::PinManagementMessage::OutputHintChannel { @@ -834,109 +1743,80 @@ impl NativeNodeWrapper { } } - // Drain all hint receivers and deliver to plugin via C ABI. - // Retain only receivers whose channels are still open. - // First collect pending hints (non-blocking), then deliver - // via spawn_blocking to avoid blocking the tokio worker — - // consistent with how tick_fn and other C ABI calls are made. - if !hint_receivers.is_empty() { - if let Some(on_hint_fn) = self.state.api().on_upstream_hint { - let mut pending_hints: Vec = Vec::new(); - hint_receivers.retain_mut(|(_pin, rx)| loop { - match rx.try_recv() { - Ok(hint) => { - tracing::info!(node = %node_name, ?hint, "Delivering upstream hint to plugin"); - if let Ok(json) = serde_json::to_string(&hint) { - if let Ok(c_str) = std::ffi::CString::new(json) { - pending_hints.push(c_str); - } - } - }, - Err(tokio::sync::mpsc::error::TryRecvError::Empty) => return true, - Err(tokio::sync::mpsc::error::TryRecvError::Disconnected) => { - return false - }, - } - }); - if !pending_hints.is_empty() { - let state = Arc::clone(&self.state); - let _ = tokio::task::spawn_blocking(move || { - for c_str in &pending_hints { - if let Some(handle) = state.begin_call() { - let _ = on_hint_fn(handle, c_str.as_ptr()); - state.finish_call(); + // Drain all hint receivers and deliver to plugin via the worker. + if !hint_receivers.is_empty() && self.state.api().on_upstream_hint.is_some() { + let mut pending_hints: Vec = Vec::new(); + hint_receivers.retain_mut(|(_pin, rx)| loop { + match rx.try_recv() { + Ok(hint) => { + tracing::info!(node = %node_name, ?hint, "Delivering upstream hint to plugin"); + if let Ok(json) = serde_json::to_string(&hint) { + if let Ok(c_str) = std::ffi::CString::new(json) { + pending_hints.push(c_str); } } - }) - .await; + }, + Err(tokio::sync::mpsc::error::TryRecvError::Empty) => return true, + Err(tokio::sync::mpsc::error::TryRecvError::Disconnected) => { + return false + }, + } + }); + if !pending_hints.is_empty() { + let (hint_reply_tx, _hint_reply_rx) = tokio::sync::oneshot::channel(); + // Use try_send: if the worker is busy (channel full), drop + // the hints rather than blocking. This prevents a wedged + // on_upstream_hint from stalling the tick loop — the + // capacity-1 channel would otherwise block the send until + // the worker drains the previous request. + match worker_tx.try_send(WorkerRequest::OnUpstreamHint { + hints: pending_hints, + reply: hint_reply_tx, + }) { + Ok(()) => { + // Hint enqueued; we don't await the reply — the + // worker will process it before the next Tick. + }, + Err(tokio::sync::mpsc::error::TrySendError::Full(_)) => { + warn!(node = %node_name, "Dropping upstream hints: worker busy"); + }, + Err(tokio::sync::mpsc::error::TrySendError::Closed(_)) => { + warn!(node = %node_name, "Dropping upstream hints: worker died"); + }, } } } // ── Tick ──────────────────────────────────────────────────── - let state = Arc::clone(&self.state); - let telemetry_tx = context.telemetry_tx.clone(); - let session_id = context.session_id.clone(); - let node_id = node_name.clone(); - let video_pool = video_pool.clone(); - let audio_pool = audio_pool.clone(); - let outcome = tokio::task::spawn_blocking(move || { - let Some(handle) = state.begin_call() else { - return TickOutcome { - outputs: Vec::new(), - success: false, - done: false, - error_msg: Some("Instance handle is null".to_string()), - }; - }; - - let _lib = Arc::clone(&state.library); - - let mut callback_ctx = CallbackContext { - output_packets: Vec::new(), - error: None, - telemetry_tx, - session_id, - node_id, - video_pool, - audio_pool, - }; - - let callback_data = (&raw mut callback_ctx).cast::(); - let node_callbacks = build_node_callbacks(callback_data); - - let result = tick_fn(handle, &raw const node_callbacks); - - // Extract error string while pointers are still valid. - let error_msg = if result.result.success { - callback_ctx.error - } else if result.result.error_message.is_null() { - Some("Source tick failed".to_string()) - } else { - Some(unsafe { - conversions::c_str_to_string(result.result.error_message) - .unwrap_or_else(|_| "Source tick failed".to_string()) - }) - }; - - let outputs = callback_ctx.output_packets; - state.finish_call(); - - TickOutcome { - outputs, - success: result.result.success, - done: result.done, - error_msg, - } - }) - .await - .map_err(|e| StreamKitError::Runtime(format!("Source tick task panicked: {e}")))?; + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); + self.send_to_worker( + WorkerCallContext { + op: "tick", + node: &node_name, + state_tx: Some(&context.state_tx), + telemetry: Some(&telemetry), + metric_labels: &self.state.labels_tick, + }, + worker_tx, + WorkerRequest::Tick { reply: reply_tx }, + ) + .await?; + let reply = self + .await_reply( + "tick", + &node_name, + reply_rx, + Some(&context.state_tx), + Some(&telemetry), + &self.state.labels_tick, + ) + .await?; // Send outputs produced by tick. If the output channel is closed, // stop ticking — source nodes have no input-close backstop so we must // detect consumer disconnect here. let mut output_closed = false; - for (pin, pkt) in outcome.outputs { + for (pin, pkt) in reply.outputs { if context.output_sender.send(&pin, pkt).await.is_err() { tracing::debug!(node = %node_name, "Output channel closed during tick"); output_closed = true; @@ -950,9 +1830,7 @@ impl NativeNodeWrapper { tick_count += 1; // Check tick result - if !outcome.success { - let error_msg = - outcome.error_msg.unwrap_or_else(|| "Source tick failed".to_string()); + if let Some(error_msg) = reply.error { error!(node = %node_name, error = %error_msg, "Source tick error"); if let Err(e) = context .state_tx @@ -967,7 +1845,7 @@ impl NativeNodeWrapper { return Err(StreamKitError::Runtime(error_msg)); } - if outcome.done { + if reply.done { tracing::info!(node = %node_name, ticks = tick_count, "Source signalled done"); break; } @@ -1003,48 +1881,61 @@ impl NativeNodeWrapper { Ok(()) } - /// Helper to apply a parameter update via the C ABI. + /// Helper to apply a parameter update via the worker thread. + /// + /// Serialization / null-byte errors from bad user params are logged + /// as warnings rather than killing the node — the pipeline continues + /// with the previous parameters. Only a dead worker is fatal. async fn apply_params_update( &self, node_name: &str, params_value: &serde_json::Value, + worker_tx: &tokio::sync::mpsc::Sender, + state_tx: &tokio::sync::mpsc::Sender, + telemetry: Option<&TelemetryEmitter>, ) -> Result<(), StreamKitError> { - let params_json = serde_json::to_string(params_value).map_err(|e| { - StreamKitError::Configuration(format!("Failed to serialize params: {e}")) - })?; - let params_cstr = CString::new(params_json) - .map_err(|e| StreamKitError::Configuration(format!("Invalid params string: {e}")))?; - - let state = Arc::clone(&self.state); - let node_name_owned = node_name.to_string(); - let error_msg = tokio::task::spawn_blocking(move || { - let handle = state.begin_call()?; - - let _lib = Arc::clone(&state.library); - let api = state.api(); - let result = (api.update_params)(handle, params_cstr.as_ptr()); - - let error = if result.success { - None - } else if result.error_message.is_null() { - Some("Failed to update parameters".to_string()) - } else { - unsafe { - Some( - conversions::c_str_to_string(result.error_message) - .unwrap_or_else(|_| "Failed to update parameters".to_string()), - ) - } - }; + let params_json = match serde_json::to_string(params_value) { + Ok(json) => json, + Err(e) => { + warn!(node = %node_name, error = %e, "Failed to serialize params, ignoring update"); + return Ok(()); + }, + }; + let params_cstr = match CString::new(params_json) { + Ok(cstr) => cstr, + Err(e) => { + warn!(node = %node_name, error = %e, "Invalid params string, ignoring update"); + return Ok(()); + }, + }; - state.finish_call(); - error - }) - .await - .map_err(|e| StreamKitError::Runtime(format!("Update params task panicked: {e}")))?; + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); + self.send_to_worker( + WorkerCallContext { + op: "update_params", + node: node_name, + state_tx: Some(state_tx), + telemetry, + metric_labels: &self.state.labels_update_params, + }, + worker_tx, + WorkerRequest::UpdateParams { params_cstr, reply: reply_tx }, + ) + .await?; + + let error_msg = self + .await_reply( + "update_params", + node_name, + reply_rx, + Some(state_tx), + telemetry, + &self.state.labels_update_params, + ) + .await?; if let Some(err) = error_msg { - warn!(node = %node_name_owned, error = %err, "Parameter update failed"); + warn!(node = %node_name, error = %err, "Parameter update failed"); } Ok(()) @@ -1082,68 +1973,91 @@ unsafe fn free_packet_buffer_handle(c_packet: *const CPacket) { use streamkit_core::frame_pool::{PooledSamples, PooledVideoData}; use streamkit_plugin_sdk_native::types::CPacketType; - let pkt = &*c_packet; - if pkt.data.is_null() { + // Use raw-pointer reads throughout to stay consistent with the SDK's + // Stacked Borrows model (no intermediate references to FFI structs). + let data = (*c_packet).data; + if data.is_null() { return; } - match pkt.packet_type { + match (*c_packet).packet_type { CPacketType::RawVideo => { - let frame = &*pkt.data.cast::(); - if !frame.buffer_handle.is_null() { - drop(Box::from_raw(frame.buffer_handle.cast::())); + let frame = data.cast::(); + let handle = (*frame).buffer_handle; + if !handle.is_null() { + drop(Box::from_raw(handle.cast::())); } }, CPacketType::RawAudio => { - let frame = &*pkt.data.cast::(); - if !frame.buffer_handle.is_null() { - drop(Box::from_raw(frame.buffer_handle.cast::())); + let frame = data.cast::(); + let handle = (*frame).buffer_handle; + if !handle.is_null() { + drop(Box::from_raw(handle.cast::())); + } + }, + CPacketType::BinaryWithMeta => { + // ABI compat: v7/v8 plugins allocate a smaller CBinaryPacket + // without buffer_handle/free_fn. Only read those fields when + // the struct is large enough (v9+). + if (*c_packet).len + >= std::mem::size_of::() + { + let bp = data.cast::(); + let handle = (*bp).buffer_handle; + if !handle.is_null() { + if let Some(free_fn) = (*bp).free_fn { + free_fn(handle); + } + } } }, _ => {}, } } -/// C callback function for sending output packets -/// This collects packets and they are sent asynchronously after the callback returns +/// C callback function for sending output packets. +/// This collects packets and they are sent asynchronously after the callback returns. extern "C" fn output_callback_shim( pin_name: *const std::os::raw::c_char, c_packet: *const CPacket, user_data: *mut c_void, ) -> CResult { - if pin_name.is_null() || c_packet.is_null() || user_data.is_null() { - return CResult::error(std::ptr::null()); - } + ffi_guard_result(|| { + if pin_name.is_null() || c_packet.is_null() || user_data.is_null() { + return CResult::error(std::ptr::null()); + } - // SAFETY: user_data is a valid pointer to CallbackContext that we passed to process_packet. - // The pointer remains valid for the duration of this callback. - let ctx = unsafe { &mut *user_data.cast::() }; + // SAFETY: user_data is a valid pointer to CallbackContext that we passed to process_packet. + // The pointer remains valid for the duration of this callback. + let ctx = unsafe { &mut *user_data.cast::() }; - // SAFETY: pin_name is a valid C string pointer provided by the plugin. - let pin_str = match unsafe { conversions::c_str_to_string(pin_name) } { - Ok(s) => s, - Err(e) => { - ctx.error = Some(format!("Invalid pin name: {e}")); - // Free any pooled buffer the plugin already consumed. - unsafe { free_packet_buffer_handle(c_packet) }; - return CResult::error(std::ptr::null()); - }, - }; - - // SAFETY: c_packet is a valid pointer to CPacket provided by the plugin. - let packet = match unsafe { conversions::packet_from_c(c_packet) } { - Ok(p) => p, - Err(e) => { - // packet_from_c already frees the buffer_handle on its own error - // paths (Critical #1), so no extra cleanup needed here. - ctx.error = Some(format!("Failed to convert packet: {e}")); - return CResult::error(std::ptr::null()); - }, - }; + // SAFETY: pin_name is a valid C string pointer provided by the plugin. + let pin_str = match unsafe { conversions::c_str_to_string(pin_name) } { + Ok(s) => s, + Err(e) => { + ctx.error = Some(format!("Invalid pin name: {e}")); + // Free any pooled buffer the plugin already consumed. + unsafe { free_packet_buffer_handle(c_packet) }; + return CResult::error(std::ptr::null()); + }, + }; - // Store packet for async sending after callback returns - ctx.output_packets.push((pin_str, packet)); + // SAFETY: c_packet is a valid pointer to CPacket provided by the plugin. + let packet = match unsafe { conversions::packet_from_c(c_packet) } { + Ok(p) => p, + Err(e) => { + // packet_from_c already frees the buffer_handle on its own error + // paths (Critical #1), so no extra cleanup needed here. + ctx.error = Some(format!("Failed to convert packet: {e}")); + return CResult::error(std::ptr::null()); + }, + }; - CResult::success() + // If Vec::push panics (OOM), Packet::drop runs during unwind and + // returns any pooled buffers — no separate RAII guard needed. + ctx.output_packets.push((pin_str, packet)); + + CResult::success() + }) } /// C callback function for emitting telemetry events. @@ -1157,135 +2071,156 @@ extern "C" fn telemetry_callback_shim( metadata: *const streamkit_plugin_sdk_native::types::CPacketMetadata, user_data: *mut c_void, ) -> CResult { - if event_type.is_null() || user_data.is_null() { - return CResult::success(); - } - - let ctx = unsafe { &mut *user_data.cast::() }; - let Some(ref tx) = ctx.telemetry_tx else { - return CResult::success(); - }; + ffi_guard_result(|| { + if event_type.is_null() || user_data.is_null() { + return CResult::success(); + } - let event_type_str = match unsafe { conversions::c_str_to_string(event_type) } { - Ok(s) => s, - Err(e) => { - warn!(error = %e, node = %ctx.node_id, "Invalid telemetry event_type"); + let ctx = unsafe { &mut *user_data.cast::() }; + let Some(ref tx) = ctx.telemetry_tx else { return CResult::success(); - }, - }; - - let data_value = if data_json.is_null() || data_len == 0 { - serde_json::Value::Object(serde_json::Map::new()) - } else { - let bytes = unsafe { std::slice::from_raw_parts(data_json, data_len) }; - match serde_json::from_slice::(bytes) { - Ok(v) => v, + }; + + let event_type_str = match unsafe { conversions::c_str_to_string(event_type) } { + Ok(s) => s, Err(e) => { - warn!(error = %e, node = %ctx.node_id, event_type = %event_type_str, "Invalid telemetry JSON payload"); + warn!(error = %e, node = %ctx.node_id, "Invalid telemetry event_type"); return CResult::success(); }, - } - }; - - let timestamp_us = if metadata.is_null() { - None - } else { - let meta = unsafe { &*metadata }; - if meta.has_timestamp_us { - Some(meta.timestamp_us) + }; + + let data_value = if data_json.is_null() || data_len == 0 { + serde_json::Value::Object(serde_json::Map::new()) } else { + let bytes = unsafe { std::slice::from_raw_parts(data_json, data_len) }; + match serde_json::from_slice::(bytes) { + Ok(v) => v, + Err(e) => { + warn!(error = %e, node = %ctx.node_id, event_type = %event_type_str, "Invalid telemetry JSON payload"); + return CResult::success(); + }, + } + }; + + let fallback_timestamp_us = || { + use std::time::{SystemTime, UNIX_EPOCH}; + SystemTime::now() + .duration_since(UNIX_EPOCH) + .ok() + .and_then(|d| u64::try_from(d.as_micros()).ok()) + .unwrap_or(0) + }; + + let timestamp_us = if metadata.is_null() { None + } else { + let meta = unsafe { &*metadata }; + if meta.has_timestamp_us { + Some(meta.timestamp_us) + } else { + None + } } - } - .unwrap_or_else(|| { - use std::time::{SystemTime, UNIX_EPOCH}; - SystemTime::now() - .duration_since(UNIX_EPOCH) - .ok() - .and_then(|d| u64::try_from(d.as_micros()).ok()) - .unwrap_or(0) - }); + .unwrap_or_else(fallback_timestamp_us); - let mut event_data = match data_value { - serde_json::Value::Object(map) => serde_json::Value::Object(map), - other => serde_json::json!({ "value": other }), - }; + let mut event_data = match data_value { + serde_json::Value::Object(map) => serde_json::Value::Object(map), + other => serde_json::json!({ "value": other }), + }; - if let Some(obj) = event_data.as_object_mut() { - obj.insert("event_type".to_string(), serde_json::Value::String(event_type_str)); - } + if let Some(obj) = event_data.as_object_mut() { + obj.insert("event_type".to_string(), serde_json::Value::String(event_type_str.clone())); + } - let event = - TelemetryEvent::new(ctx.session_id.clone(), ctx.node_id.clone(), event_data, timestamp_us); + let event = TelemetryEvent::new( + ctx.session_id.clone(), + ctx.node_id.clone(), + event_data, + timestamp_us, + ); - if tx.try_send(event).is_err() { - // Drop silently: best-effort. - } + if let Err(err) = tx.try_send(event) { + warn!( + node = %ctx.node_id, + event_type = %event_type_str, + reason = %err, + "Dropping plugin telemetry event" + ); + } - CResult::success() + CResult::success() + }) } // ── Frame pool allocation shims (v6) ───────────────────────────────────── /// Allocate a video buffer from the host's frame pool. extern "C" fn alloc_video_shim(min_bytes: usize, user_data: *mut c_void) -> CAllocVideoResult { - use streamkit_core::frame_pool::PooledVideoData; + ffi_guard_alloc_video(|| { + use streamkit_core::frame_pool::PooledVideoData; - if user_data.is_null() { - return CAllocVideoResult::null(); - } + if user_data.is_null() { + return CAllocVideoResult::null(); + } - let ctx = unsafe { &*user_data.cast::() }; - let Some(pool) = ctx.video_pool.as_ref() else { - return CAllocVideoResult::null(); - }; + let ctx = unsafe { &*user_data.cast::() }; + let Some(pool) = ctx.video_pool.as_ref() else { + return CAllocVideoResult::null(); + }; - let mut pooled: PooledVideoData = pool.get(min_bytes); - let data_ptr = pooled.as_mut_ptr(); - let len = pooled.len(); - let handle = Box::into_raw(Box::new(pooled)).cast::(); + let mut pooled: PooledVideoData = pool.get(min_bytes); + let data_ptr = pooled.as_mut_ptr(); + let len = pooled.len(); + let handle = Box::into_raw(Box::new(pooled)).cast::(); - CAllocVideoResult { data: data_ptr, len, handle, free_fn: Some(free_video_buffer) } + CAllocVideoResult { data: data_ptr, len, handle, free_fn: Some(free_video_buffer) } + }) } /// Free a video buffer without sending it (error/discard path). extern "C" fn free_video_buffer(handle: *mut c_void) { - use streamkit_core::frame_pool::PooledVideoData; + ffi_guard_unit(|| { + use streamkit_core::frame_pool::PooledVideoData; - if !handle.is_null() { - // SAFETY: handle was created by alloc_video_shim via Box::into_raw. - let _ = unsafe { Box::from_raw(handle.cast::()) }; - } + if !handle.is_null() { + // SAFETY: handle was created by alloc_video_shim via Box::into_raw. + let _ = unsafe { Box::from_raw(handle.cast::()) }; + } + }); } /// Allocate an audio buffer from the host's frame pool. extern "C" fn alloc_audio_shim(min_samples: usize, user_data: *mut c_void) -> CAllocAudioResult { - use streamkit_core::frame_pool::PooledSamples; + ffi_guard_alloc_audio(|| { + use streamkit_core::frame_pool::PooledSamples; - if user_data.is_null() { - return CAllocAudioResult::null(); - } + if user_data.is_null() { + return CAllocAudioResult::null(); + } - let ctx = unsafe { &*user_data.cast::() }; - let Some(pool) = ctx.audio_pool.as_ref() else { - return CAllocAudioResult::null(); - }; + let ctx = unsafe { &*user_data.cast::() }; + let Some(pool) = ctx.audio_pool.as_ref() else { + return CAllocAudioResult::null(); + }; - let mut pooled: PooledSamples = pool.get(min_samples); - let data_ptr = pooled.as_mut_ptr(); - let sample_count = pooled.len(); - let handle = Box::into_raw(Box::new(pooled)).cast::(); + let mut pooled: PooledSamples = pool.get(min_samples); + let data_ptr = pooled.as_mut_ptr(); + let sample_count = pooled.len(); + let handle = Box::into_raw(Box::new(pooled)).cast::(); - CAllocAudioResult { data: data_ptr, sample_count, handle, free_fn: Some(free_audio_buffer) } + CAllocAudioResult { data: data_ptr, sample_count, handle, free_fn: Some(free_audio_buffer) } + }) } /// Free an audio buffer without sending it (error/discard path). extern "C" fn free_audio_buffer(handle: *mut c_void) { - use streamkit_core::frame_pool::PooledSamples; + ffi_guard_unit(|| { + use streamkit_core::frame_pool::PooledSamples; - if !handle.is_null() { - let _ = unsafe { Box::from_raw(handle.cast::()) }; - } + if !handle.is_null() { + let _ = unsafe { Box::from_raw(handle.cast::()) }; + } + }); } /// Build a `CNodeCallbacks` struct from a `CallbackContext` pointer. @@ -1304,3 +2239,1018 @@ fn build_node_callbacks(callback_data: *mut c_void) -> CNodeCallbacks { alloc_user_data: callback_data, } } + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod ffi_guard_tests { + use super::*; + use std::sync::atomic::AtomicUsize; + + #[test] + fn host_guard_result_catches_panic_and_preserves_message() { + let r = ffi_guard_result(|| panic!("host boom")); + assert!(!r.success); + assert!(!r.error_message.is_null()); + let msg = unsafe { conversions::c_str_to_string(r.error_message) }.expect("valid UTF-8"); + assert!(msg.contains("host boom"), "expected panic message, got: {msg}"); + } + + #[test] + fn host_guard_alloc_video_catches_panic() { + let r = ffi_guard_alloc_video(|| panic!("video alloc boom")); + assert!(r.data.is_null()); + } + + #[test] + fn host_guard_alloc_audio_catches_panic() { + let r = ffi_guard_alloc_audio(|| panic!("audio alloc boom")); + assert!(r.data.is_null()); + } + + #[test] + fn host_guard_unit_catches_panic() { + ffi_guard_unit(|| panic!("unit boom")); + } + + // ── Real shim tests ──────────────────────────────────────────────── + // + // These exercise the actual `extern "C"` callback shims (not just the + // guard helpers) so that accidentally removing a guard from a shim + // body would be caught by CI. + + #[test] + fn output_callback_shim_null_args_returns_error() { + let r = output_callback_shim(std::ptr::null(), std::ptr::null(), std::ptr::null_mut()); + assert!(!r.success); + } + + #[test] + fn output_callback_shim_valid_text_packet() { + let mut ctx = CallbackContext { + output_packets: Vec::new(), + error: None, + telemetry_tx: None, + session_id: None, + node_id: "test-node".to_string(), + video_pool: None, + audio_pool: None, + }; + let user_data = (&raw mut ctx).cast::(); + + let pin = CString::new("output").expect("valid pin"); + // packet_from_c reads Text packets via CStr::from_ptr, so the + // data must be a null-terminated C string. + let text = CString::new("hello").expect("valid text"); + let c_packet = CPacket { + packet_type: streamkit_plugin_sdk_native::types::CPacketType::Text, + data: text.as_ptr().cast(), + len: text.as_bytes_with_nul().len(), + }; + + let r = output_callback_shim(pin.as_ptr(), &raw const c_packet, user_data); + assert!(r.success, "expected success for valid text packet"); + assert_eq!(ctx.output_packets.len(), 1); + assert_eq!(ctx.output_packets[0].0, "output"); + } + + #[test] + fn telemetry_callback_shim_null_args_returns_success() { + // telemetry is best-effort — null args should not panic + let r = telemetry_callback_shim( + std::ptr::null(), + std::ptr::null(), + 0, + std::ptr::null(), + std::ptr::null_mut(), + ); + assert!(r.success); + } + + #[test] + fn alloc_video_shim_null_user_data_returns_null() { + let r = alloc_video_shim(1024, std::ptr::null_mut()); + assert!(r.data.is_null()); + } + + #[test] + fn alloc_audio_shim_null_user_data_returns_null() { + let r = alloc_audio_shim(960, std::ptr::null_mut()); + assert!(r.data.is_null()); + } + + // ── CallGuard / ApiPtr tests ─────────────────────────────────────── + + /// Dummy `extern "C"` stubs used to populate a test `CNativePluginAPI`. + mod test_stubs { + use super::*; + use streamkit_plugin_sdk_native::types::{CNodeCallbacks, CNodeMetadata}; + + pub extern "C" fn get_metadata() -> *const CNodeMetadata { + std::ptr::null() + } + pub extern "C" fn create_instance( + _: *const std::os::raw::c_char, + _: streamkit_plugin_sdk_native::types::CLogCallback, + _: *mut c_void, + ) -> CPluginHandle { + std::ptr::null_mut() + } + pub extern "C" fn process_packet( + _: CPluginHandle, + _: *const std::os::raw::c_char, + _: *const CPacket, + _: *const CNodeCallbacks, + ) -> CResult { + CResult::success() + } + pub extern "C" fn update_params( + _: CPluginHandle, + _: *const std::os::raw::c_char, + ) -> CResult { + CResult::success() + } + pub extern "C" fn flush(_: CPluginHandle, _: *const CNodeCallbacks) -> CResult { + CResult::success() + } + pub extern "C" fn destroy_instance(_: CPluginHandle) {} + pub static DESTROY_CALL_COUNT: std::sync::atomic::AtomicUsize = + std::sync::atomic::AtomicUsize::new(0); + pub extern "C" fn destroy_instance_counted(_: CPluginHandle) { + DESTROY_CALL_COUNT.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + } + } + + /// Build a valid `CNativePluginAPI` populated with no-op stubs. + fn dummy_api() -> CNativePluginAPI { + CNativePluginAPI { + version: 8, + get_metadata: test_stubs::get_metadata, + create_instance: test_stubs::create_instance, + process_packet: test_stubs::process_packet, + update_params: test_stubs::update_params, + flush: test_stubs::flush, + destroy_instance: test_stubs::destroy_instance, + get_source_config: None, + tick: None, + get_runtime_param_schema: None, + on_upstream_hint: None, + } + } + + fn dummy_api_counted_destroy() -> CNativePluginAPI { + let mut api = dummy_api(); + api.destroy_instance = test_stubs::destroy_instance_counted; + api + } + + /// Helper: build a minimal `InstanceState` for guard tests. + fn test_instance_state() -> Arc { + // SAFETY: loading libc is harmless; we never call any symbols from it. + let lib = unsafe { Library::new("libc.so.6").expect("libc must be loadable") }; + let api: &'static CNativePluginAPI = Box::leak(Box::new(dummy_api())); + Arc::new(InstanceState::new( + Arc::new(lib), + api, + std::ptr::without_provenance_mut::(1), // non-null dummy handle + 8, + None, + "test".to_string(), + )) + } + + #[test] + fn call_guard_drops_on_normal_path() { + let state = test_instance_state(); + { + let guard = state.begin_call().expect("begin_call should succeed"); + assert_eq!(state.in_flight_calls.load(Ordering::Acquire), 1); + let _h = guard.handle(); + } + assert_eq!(state.in_flight_calls.load(Ordering::Acquire), 0); + } + + #[test] + fn call_guard_drops_on_panic() { + let state = test_instance_state(); + let state2 = Arc::clone(&state); + let result = std::panic::catch_unwind(AssertUnwindSafe(move || { + let _guard = state2.begin_call().expect("begin_call should succeed"); + panic!("boom"); + })); + assert!(result.is_err(), "should have panicked"); + assert_eq!( + state.in_flight_calls.load(Ordering::Acquire), + 0, + "guard must decrement in_flight_calls even on panic" + ); + } + + #[test] + fn call_guard_panic_then_destroy_invariant() { + let state = test_instance_state(); + let state2 = Arc::clone(&state); + let _ = std::panic::catch_unwind(AssertUnwindSafe(move || { + let _guard = state2.begin_call().expect("begin_call should succeed"); + panic!("mid-FFI panic"); + })); + // After panic unwind, in_flight_calls must be 0. + assert_eq!( + state.in_flight_calls.load(Ordering::SeqCst), + 0, + "in_flight_calls must be 0 after panic unwind drops CallGuard" + ); + // request_drop + destroy must still work cleanly. + state.request_drop(); + assert!(state.handle.load(Ordering::SeqCst).is_null(), "handle must be null after destroy"); + } + + #[test] + fn finish_call_without_begin_does_not_panic() { + let state = test_instance_state(); + let result = std::panic::catch_unwind(AssertUnwindSafe(|| { + state.finish_call(); + })); + + assert!(result.is_ok(), "finish_call must not panic from Drop paths"); + assert_eq!(state.in_flight_calls.load(Ordering::SeqCst), 0); + } + + #[test] + fn begin_call_returns_none_after_request_drop() { + let state = test_instance_state(); + state.request_drop(); + // After request_drop the handle is swapped to null, so begin_call + // must return None. + assert!(state.begin_call().is_none(), "begin_call must return None after request_drop"); + assert_eq!( + state.in_flight_calls.load(Ordering::Acquire), + 0, + "failed begin_call must not leave in_flight_calls elevated" + ); + } + + #[test] + fn instance_state_drop_destroys_without_request_drop() { + test_stubs::DESTROY_CALL_COUNT.store(0, Ordering::SeqCst); + let lib = unsafe { Library::new("libc.so.6").expect("libc must be loadable") }; + let api: &'static CNativePluginAPI = Box::leak(Box::new(dummy_api_counted_destroy())); + + let state = InstanceState::new( + Arc::new(lib), + api, + std::ptr::without_provenance_mut::(1), + 8, + None, + "test".to_string(), + ); + + drop(state); + + assert_eq!( + test_stubs::DESTROY_CALL_COUNT.load(Ordering::SeqCst), + 1, + "InstanceState::drop must destroy an unreleased instance" + ); + } + + /// Stress test: hammer `begin_call` from many threads while another + /// thread calls `request_drop`. Asserts that after all threads join: + /// - `in_flight_calls` is 0 (no stranded counts) + /// - `handle` is null (destroy was called exactly once) + /// - `drop_requested` is set + /// + /// This exercises the Dekker pair and the rollback-triggers-destroy + /// path that was missing before the fix. + #[test] + fn concurrent_begin_call_and_request_drop() { + use std::sync::Barrier; + + for _round in 0..50 { + let state = test_instance_state(); + let num_callers = 8; + let barrier = Arc::new(Barrier::new(num_callers + 1)); + + let mut handles = Vec::new(); + + // Spawn begin_call threads + for _ in 0..num_callers { + let s = Arc::clone(&state); + let b = Arc::clone(&barrier); + handles.push(std::thread::spawn(move || { + b.wait(); + // Try to begin a call; if we get a guard, hold it + // briefly then drop it. + let _guard = s.begin_call(); + })); + } + + // Spawn request_drop thread + { + let s = Arc::clone(&state); + let b = Arc::clone(&barrier); + handles.push(std::thread::spawn(move || { + b.wait(); + s.request_drop(); + })); + } + + for h in handles { + h.join().expect("thread panicked"); + } + + assert_eq!( + state.in_flight_calls.load(Ordering::SeqCst), + 0, + "in_flight_calls must be 0 after all threads complete" + ); + assert!( + state.handle.load(Ordering::SeqCst).is_null(), + "handle must be null (destroy_instance must have been called)" + ); + assert!(state.drop_requested.load(Ordering::SeqCst), "drop_requested must be set"); + } + } + + // Pointer-equality alone does not prove provenance is preserved. + // Under Miri this test authoritatively validates that ApiPtr never + // strips provenance. Outside Miri, it is a no-op. + // TODO: add `cargo +nightly miri test -p streamkit-plugin-native` to CI. + #[test] + #[cfg(miri)] + fn api_ptr_preserves_provenance() { + let api: &'static CNativePluginAPI = Box::leak(Box::new(dummy_api())); + let original_addr = std::ptr::from_ref(api); + let wrapper = ApiPtr(original_addr); + let recovered: *const CNativePluginAPI = wrapper.0; + assert_eq!( + original_addr, recovered, + "ApiPtr must round-trip the pointer without losing provenance" + ); + } + + // ── Worker thread tests ──────────────────────────────────────────── + + /// Additional stubs for worker tests (error-returning, slow variants). + mod worker_stubs { + use super::*; + use streamkit_plugin_sdk_native::types::CNodeCallbacks; + + /// Static error message for the process_error stub — avoids a + /// per-call `Box::leak`. + static PROCESS_ERROR_MSG: &std::ffi::CStr = c"plugin exploded"; + + pub extern "C" fn process_error( + _: CPluginHandle, + _: *const std::os::raw::c_char, + _: *const CPacket, + _: *const CNodeCallbacks, + ) -> CResult { + CResult { success: false, error_message: PROCESS_ERROR_MSG.as_ptr() } + } + + pub extern "C" fn process_slow( + _: CPluginHandle, + _: *const std::os::raw::c_char, + _: *const CPacket, + _: *const CNodeCallbacks, + ) -> CResult { + std::thread::sleep(std::time::Duration::from_millis(500)); + CResult::success() + } + + // NOTE: A panicking `extern "C"` stub cannot be used to test catch_unwind + // because Rust inserts an abort shim at extern "C" boundaries — the panic + // never reaches catch_unwind, it terminates the process. The catch_unwind + // in worker_thread_main guards against panics in *Rust* code around the FFI + // call (e.g. inside AssertUnwindSafe closures or callback shims). + + pub extern "C" fn tick_ok( + _: CPluginHandle, + _: *const CNodeCallbacks, + ) -> streamkit_plugin_sdk_native::types::CTickResult { + streamkit_plugin_sdk_native::types::CTickResult::ok() + } + + /// Counter incremented each time `on_hint_ok` is called. + pub static HINT_CALL_COUNT: std::sync::atomic::AtomicUsize = + std::sync::atomic::AtomicUsize::new(0); + + pub extern "C" fn on_hint_ok(_: CPluginHandle, _: *const std::os::raw::c_char) -> CResult { + HINT_CALL_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + CResult::success() + } + } + + fn dummy_api_error() -> CNativePluginAPI { + let mut api = dummy_api(); + api.process_packet = worker_stubs::process_error; + api + } + + fn dummy_api_slow() -> CNativePluginAPI { + let mut api = dummy_api(); + api.process_packet = worker_stubs::process_slow; + api + } + + fn dummy_api_with_tick() -> CNativePluginAPI { + let mut api = dummy_api(); + api.tick = Some(worker_stubs::tick_ok); + api + } + + fn dummy_api_with_hint() -> CNativePluginAPI { + let mut api = dummy_api(); + api.on_upstream_hint = Some(worker_stubs::on_hint_ok); + api + } + + fn test_instance_state_with_api(api: CNativePluginAPI) -> Arc { + let lib = unsafe { Library::new("libc.so.6").expect("libc must be loadable") }; + let api: &'static CNativePluginAPI = Box::leak(Box::new(api)); + Arc::new(InstanceState::new( + Arc::new(lib), + api, + std::ptr::without_provenance_mut::(1), + 8, + None, + "test".to_string(), + )) + } + + fn test_instance_state_with_timeout( + api: CNativePluginAPI, + timeout: std::time::Duration, + ) -> Arc { + let lib = unsafe { Library::new("libc.so.6").expect("libc must be loadable") }; + let api: &'static CNativePluginAPI = Box::leak(Box::new(api)); + Arc::new(InstanceState::new( + Arc::new(lib), + api, + std::ptr::without_provenance_mut::(1), + 8, + Some(timeout), + "test".to_string(), + )) + } + + fn test_wrapper_with_timeout(timeout: Option) -> NativeNodeWrapper { + let lib = unsafe { Library::new("libc.so.6").expect("libc must be loadable") }; + let api: &'static CNativePluginAPI = Box::leak(Box::new(dummy_api())); + NativeNodeWrapper { + state: Arc::new(InstanceState::new( + Arc::new(lib), + api, + std::ptr::without_provenance_mut::(1), + 8, + timeout, + "test".to_string(), + )), + metadata: PluginMetadata { + kind: "test".to_string(), + description: None, + inputs: Vec::new(), + outputs: Vec::new(), + param_schema: serde_json::json!({}), + categories: Vec::new(), + is_source: false, + tick_interval_us: 0, + max_ticks: 0, + }, + } + } + + #[tokio::test] + async fn send_to_worker_timeout_emits_failed_state() { + let wrapper = test_wrapper_with_timeout(Some(std::time::Duration::from_millis(10))); + let (tx, mut rx) = tokio::sync::mpsc::channel::(1); + let (state_tx, mut state_rx) = tokio::sync::mpsc::channel::(1); + + let (first_reply, _first_rx) = tokio::sync::oneshot::channel(); + tx.send(WorkerRequest::Flush { reply: first_reply }).await.unwrap(); + + let (second_reply, _second_rx) = tokio::sync::oneshot::channel(); + let result = wrapper + .send_to_worker( + WorkerCallContext { + op: "flush", + node: "node-a", + state_tx: Some(&state_tx), + telemetry: None, + metric_labels: &wrapper.state.labels_flush, + }, + &tx, + WorkerRequest::Flush { reply: second_reply }, + ) + .await; + + assert!(result.is_err(), "send should time out while channel is full"); + let update = state_rx.recv().await.expect("failed state should be emitted"); + assert_eq!(update.node_id, "node-a"); + assert!(matches!(update.state, NodeState::Failed { .. })); + + drop(rx.recv().await); + } + + #[tokio::test] + async fn await_reply_timeout_emits_failed_state() { + let wrapper = test_wrapper_with_timeout(Some(std::time::Duration::from_millis(10))); + let (_reply_tx, reply_rx) = tokio::sync::oneshot::channel::<()>(); + let (state_tx, mut state_rx) = tokio::sync::mpsc::channel::(1); + + let result = wrapper + .await_reply( + "update_params", + "node-a", + reply_rx, + Some(&state_tx), + None, + &wrapper.state.labels_update_params, + ) + .await; + + assert!(result.is_err(), "reply should time out"); + let update = state_rx.recv().await.expect("failed state should be emitted"); + assert_eq!(update.node_id, "node-a"); + assert!(matches!(update.state, NodeState::Failed { .. })); + } + + #[tokio::test] + async fn send_to_worker_none_timeout_still_bounds_send_side() { + let wrapper = test_wrapper_with_timeout(None); + let (tx, _rx) = tokio::sync::mpsc::channel::(1); + let (first_reply, _first_rx) = tokio::sync::oneshot::channel(); + tx.send(WorkerRequest::Flush { reply: first_reply }).await.unwrap(); + + let completed = Arc::new(AtomicUsize::new(0)); + let completed_in_task = Arc::clone(&completed); + let (second_reply, _second_rx) = tokio::sync::oneshot::channel(); + let wrapper_task = NativeNodeWrapper { + state: Arc::clone(&wrapper.state), + metadata: wrapper.metadata.clone(), + }; + let tx_task = tx.clone(); + let task = tokio::spawn(async move { + let result = wrapper_task + .send_to_worker( + WorkerCallContext { + op: "flush", + node: "node-a", + state_tx: None, + telemetry: None, + metric_labels: &wrapper_task.state.labels_flush, + }, + &tx_task, + WorkerRequest::Flush { reply: second_reply }, + ) + .await; + completed_in_task.fetch_add(1, Ordering::Relaxed); + result + }); + + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + assert_eq!( + completed.load(Ordering::Relaxed), + 0, + "None reply timeout must not make send_to_worker unboundedly complete early" + ); + task.abort(); + } + + /// Spawn a worker, send a Process request, verify successful round-trip. + #[test] + fn worker_happy_path_process() { + let state = test_instance_state(); + let (tx, rx) = tokio::sync::mpsc::channel::(1); + let pin_names = vec![CString::new("input").unwrap()]; + + let s = Arc::clone(&state); + let handle = std::thread::Builder::new() + .name("test-worker".into()) + .spawn(move || { + worker_thread_main(rx, s, pin_names, None, None, "test".into(), None, None); + }) + .unwrap(); + + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); + tx.blocking_send(WorkerRequest::Process { + pin_index: 0, + packet: Packet::Text("hello".into()), + reply: reply_tx, + }) + .unwrap(); + + let reply = reply_rx.blocking_recv().unwrap(); + assert!(reply.error.is_none(), "expected no error, got: {:?}", reply.error); + assert!(!reply.done); + + drop(tx); + handle.join().unwrap(); + } + + /// Send a Process to an error-returning stub — the worker propagates + /// the plugin's error message in the reply, and survives for the next call. + #[test] + fn worker_error_propagation() { + let state = test_instance_state_with_api(dummy_api_error()); + let (tx, rx) = tokio::sync::mpsc::channel::(1); + let pin_names = vec![CString::new("input").unwrap()]; + + let s = Arc::clone(&state); + let handle = std::thread::Builder::new() + .name("test-worker-error".into()) + .spawn(move || { + worker_thread_main(rx, s, pin_names, None, None, "test".into(), None, None); + }) + .unwrap(); + + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); + tx.blocking_send(WorkerRequest::Process { + pin_index: 0, + packet: Packet::Text("trigger-error".into()), + reply: reply_tx, + }) + .unwrap(); + + let reply = reply_rx.blocking_recv().unwrap(); + let err = reply.error.expect("expected error from plugin"); + assert!(err.contains("plugin exploded"), "expected plugin error message, got: {err}"); + + // Worker should still be alive after returning an error. + let (flush_tx, flush_rx) = tokio::sync::oneshot::channel(); + tx.blocking_send(WorkerRequest::Flush { reply: flush_tx }).unwrap(); + let flush_reply = flush_rx.blocking_recv().unwrap(); + assert!(flush_reply.error.is_none(), "worker should survive after error reply"); + + drop(tx); + handle.join().unwrap(); + } + + /// Drop the channel while the worker is idle — the worker exits + /// cleanly via `blocking_recv` returning `None`. + #[test] + fn worker_channel_close_exits_cleanly() { + let state = test_instance_state(); + let (tx, rx) = tokio::sync::mpsc::channel::(1); + + let s = Arc::clone(&state); + let handle = std::thread::Builder::new() + .name("test-worker-close".into()) + .spawn(move || { + worker_thread_main(rx, s, Vec::new(), None, None, "test".into(), None, None); + }) + .unwrap(); + + drop(tx); + // Worker should join cleanly. + handle.join().expect("worker should exit cleanly when channel closes"); + } + + /// request_drop while the worker is processing — the worker finishes + /// its current call and subsequent begin_call returns None. + #[test] + fn worker_request_drop_during_processing() { + let state = test_instance_state_with_api(dummy_api_slow()); + let (tx, rx) = tokio::sync::mpsc::channel::(1); + let pin_names = vec![CString::new("input").unwrap()]; + + let s = Arc::clone(&state); + let handle = std::thread::Builder::new() + .name("test-worker-detach".into()) + .spawn(move || { + worker_thread_main(rx, s, pin_names, None, None, "test".into(), None, None); + }) + .unwrap(); + + // Send a slow process request. + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); + tx.blocking_send(WorkerRequest::Process { + pin_index: 0, + packet: Packet::Text("slow".into()), + reply: reply_tx, + }) + .unwrap(); + + // Request drop while the worker is processing. + std::thread::sleep(std::time::Duration::from_millis(50)); + state.request_drop(); + + // The slow process should still complete its reply. + let reply = reply_rx.blocking_recv().unwrap(); + assert!(reply.error.is_none(), "slow process should succeed"); + + // Next process should get "Instance destroyed" error. + let (post_drop_tx, post_drop_rx) = tokio::sync::oneshot::channel(); + tx.blocking_send(WorkerRequest::Process { + pin_index: 0, + packet: Packet::Text("after-drop".into()), + reply: post_drop_tx, + }) + .unwrap(); + let post_drop_reply = post_drop_rx.blocking_recv().unwrap(); + assert!( + post_drop_reply.error.as_ref().is_some_and(|e| e.contains("destroyed")), + "expected destroyed error, got: {:?}", + post_drop_reply.error + ); + + drop(tx); + handle.join().unwrap(); + } + + /// Configure a short timeout and a slow stub — verify the reply + /// channel times out but the worker survives for the next request. + #[tokio::test] + async fn worker_timeout_then_next_send() { + let state = test_instance_state_with_timeout( + dummy_api_slow(), + std::time::Duration::from_millis(50), + ); + let (tx, rx) = tokio::sync::mpsc::channel::(1); + let pin_names = vec![CString::new("input").unwrap()]; + + let s = Arc::clone(&state); + let handle = std::thread::Builder::new() + .name("test-worker-timeout".into()) + .spawn(move || { + worker_thread_main(rx, s, pin_names, None, None, "test".into(), None, None); + }) + .unwrap(); + + // Send a request that will take 500ms — timeout is 50ms. + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel::(); + tx.send(WorkerRequest::Process { + pin_index: 0, + packet: Packet::Text("slow".into()), + reply: reply_tx, + }) + .await + .unwrap(); + + let result = tokio::time::timeout(std::time::Duration::from_millis(50), reply_rx).await; + assert!(result.is_err(), "expected timeout"); + + // Wait for worker to finish the slow call. + tokio::time::sleep(std::time::Duration::from_millis(600)).await; + + // Worker should accept the next request. + let (flush_tx, flush_rx) = tokio::sync::oneshot::channel(); + tx.send(WorkerRequest::Flush { reply: flush_tx }).await.unwrap(); + let flush_reply = flush_rx.await.unwrap(); + assert!(flush_reply.error.is_none(), "worker should survive after timeout"); + + drop(tx); + handle.join().unwrap(); + } + + /// Flush round-trip through the worker. + #[test] + fn worker_flush_round_trip() { + let state = test_instance_state(); + let (tx, rx) = tokio::sync::mpsc::channel::(1); + + let s = Arc::clone(&state); + let handle = std::thread::Builder::new() + .name("test-worker-flush".into()) + .spawn(move || { + worker_thread_main(rx, s, Vec::new(), None, None, "test".into(), None, None); + }) + .unwrap(); + + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); + tx.blocking_send(WorkerRequest::Flush { reply: reply_tx }).unwrap(); + let reply = reply_rx.blocking_recv().unwrap(); + assert!(reply.error.is_none(), "flush should succeed"); + + drop(tx); + handle.join().unwrap(); + } + + /// Tick round-trip through the worker (source plugin path). + #[test] + fn worker_tick_round_trip() { + let state = test_instance_state_with_api(dummy_api_with_tick()); + let (tx, rx) = tokio::sync::mpsc::channel::(1); + + let s = Arc::clone(&state); + let handle = std::thread::Builder::new() + .name("test-worker-tick".into()) + .spawn(move || { + worker_thread_main(rx, s, Vec::new(), None, None, "test".into(), None, None); + }) + .unwrap(); + + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); + tx.blocking_send(WorkerRequest::Tick { reply: reply_tx }).unwrap(); + let reply = reply_rx.blocking_recv().unwrap(); + assert!(reply.error.is_none(), "tick should succeed"); + assert!(!reply.done, "tick should not signal done"); + + drop(tx); + handle.join().unwrap(); + } + + /// UpdateParams round-trip through the worker. + #[test] + fn worker_update_params_round_trip() { + let state = test_instance_state(); + let (tx, rx) = tokio::sync::mpsc::channel::(1); + + let s = Arc::clone(&state); + let handle = std::thread::Builder::new() + .name("test-worker-params".into()) + .spawn(move || { + worker_thread_main(rx, s, Vec::new(), None, None, "test".into(), None, None); + }) + .unwrap(); + + let params = CString::new(r#"{"key": "value"}"#).unwrap(); + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); + tx.blocking_send(WorkerRequest::UpdateParams { params_cstr: params, reply: reply_tx }) + .unwrap(); + let reply = reply_rx.blocking_recv().unwrap(); + assert!(reply.is_none(), "update_params should succeed with no error"); + + drop(tx); + handle.join().unwrap(); + } + + /// OnUpstreamHint is delivered without blocking the caller. + #[test] + fn worker_on_upstream_hint() { + // Reset counter before test. + worker_stubs::HINT_CALL_COUNT.store(0, Ordering::Relaxed); + + let state = test_instance_state_with_api(dummy_api_with_hint()); + let (tx, rx) = tokio::sync::mpsc::channel::(1); + + let s = Arc::clone(&state); + let handle = std::thread::Builder::new() + .name("test-worker-hint".into()) + .spawn(move || { + worker_thread_main(rx, s, Vec::new(), None, None, "test".into(), None, None); + }) + .unwrap(); + + let hint = CString::new("audio/opus").unwrap(); + let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); + tx.blocking_send(WorkerRequest::OnUpstreamHint { hints: vec![hint], reply: reply_tx }) + .unwrap(); + reply_rx.blocking_recv().expect("on_upstream_hint should complete"); + + assert_eq!( + worker_stubs::HINT_CALL_COUNT.load(Ordering::Relaxed), + 1, + "on_hint_fn should have been called exactly once" + ); + + drop(tx); + handle.join().unwrap(); + } + + // NOTE: worker_thread_main wraps each FFI call in catch_unwind, but + // we cannot test that path with an extern "C" stub because Rust aborts + // on panic-across-FFI. See the comment in worker_stubs above. + + // ── plugin_log_enabled_callback tests ────────────────────────────── + + /// Minimal subscriber that only enables events at or above a given level. + struct LevelGateSubscriber(tracing::Level); + + impl tracing::Subscriber for LevelGateSubscriber { + fn enabled(&self, metadata: &tracing::Metadata<'_>) -> bool { + *metadata.level() <= self.0 + } + fn new_span(&self, _: &tracing::span::Attributes<'_>) -> tracing::span::Id { + tracing::span::Id::from_u64(1) + } + fn record(&self, _: &tracing::span::Id, _: &tracing::span::Record<'_>) {} + fn record_follows_from(&self, _: &tracing::span::Id, _: &tracing::span::Id) {} + fn event(&self, _: &tracing::Event<'_>) {} + fn enter(&self, _: &tracing::span::Id) {} + fn exit(&self, _: &tracing::span::Id) {} + } + + #[test] + fn plugin_log_enabled_respects_subscriber_level() { + use streamkit_plugin_sdk_native::types::CLogLevel; + + let subscriber = LevelGateSubscriber(tracing::Level::INFO); + let dispatch = tracing::Dispatch::new(subscriber); + let _guard = tracing::dispatcher::set_default(&dispatch); + + // INFO and above should be enabled. + assert!(plugin_log_enabled_callback( + CLogLevel::Error, + std::ptr::null(), + std::ptr::null_mut(), + )); + assert!(plugin_log_enabled_callback( + CLogLevel::Info, + std::ptr::null(), + std::ptr::null_mut(), + )); + // DEBUG / TRACE should be disabled. + assert!(!plugin_log_enabled_callback( + CLogLevel::Debug, + std::ptr::null(), + std::ptr::null_mut(), + )); + assert!(!plugin_log_enabled_callback( + CLogLevel::Trace, + std::ptr::null(), + std::ptr::null_mut(), + )); + } + + /// Subscriber that only enables events whose target matches a given string. + struct TargetFilterSubscriber(&'static str); + + impl tracing::Subscriber for TargetFilterSubscriber { + fn enabled(&self, metadata: &tracing::Metadata<'_>) -> bool { + metadata.target() == self.0 + } + fn new_span(&self, _: &tracing::span::Attributes<'_>) -> tracing::span::Id { + tracing::span::Id::from_u64(1) + } + fn record(&self, _: &tracing::span::Id, _: &tracing::span::Record<'_>) {} + fn record_follows_from(&self, _: &tracing::span::Id, _: &tracing::span::Id) {} + fn event(&self, _: &tracing::Event<'_>) {} + fn enter(&self, _: &tracing::span::Id) {} + fn exit(&self, _: &tracing::span::Id) {} + } + + struct CapturingTargetSubscriber { + expected_target: &'static str, + seen_target: std::sync::Arc>>, + event_count: std::sync::Arc, + } + + impl tracing::Subscriber for CapturingTargetSubscriber { + fn enabled(&self, metadata: &tracing::Metadata<'_>) -> bool { + metadata.target() == self.expected_target + } + fn new_span(&self, _: &tracing::span::Attributes<'_>) -> tracing::span::Id { + tracing::span::Id::from_u64(1) + } + fn record(&self, _: &tracing::span::Id, _: &tracing::span::Record<'_>) {} + fn record_follows_from(&self, _: &tracing::span::Id, _: &tracing::span::Id) {} + fn event(&self, event: &tracing::Event<'_>) { + *self.seen_target.lock().unwrap() = Some(event.metadata().target().to_string()); + self.event_count.fetch_add(1, Ordering::SeqCst); + } + fn enter(&self, _: &tracing::span::Id) {} + fn exit(&self, _: &tracing::span::Id) {} + } + + #[test] + fn plugin_log_enabled_passes_target_to_subscriber() { + use streamkit_plugin_sdk_native::types::CLogLevel; + + let subscriber = TargetFilterSubscriber("whisper"); + let dispatch = tracing::Dispatch::new(subscriber); + let _guard = tracing::dispatcher::set_default(&dispatch); + + let whisper = std::ffi::CString::new("whisper").unwrap(); + let kokoro = std::ffi::CString::new("kokoro").unwrap(); + + assert!(plugin_log_enabled_callback( + CLogLevel::Info, + whisper.as_ptr(), + std::ptr::null_mut(), + )); + assert!(!plugin_log_enabled_callback( + CLogLevel::Info, + kokoro.as_ptr(), + std::ptr::null_mut(), + )); + } + + #[test] + fn plugin_log_callback_emits_event_with_plugin_target_metadata() { + use streamkit_plugin_sdk_native::types::CLogLevel; + + let seen_target = std::sync::Arc::new(std::sync::Mutex::new(None)); + let event_count = std::sync::Arc::new(AtomicUsize::new(0)); + let subscriber = CapturingTargetSubscriber { + expected_target: "whisper", + seen_target: std::sync::Arc::clone(&seen_target), + event_count: std::sync::Arc::clone(&event_count), + }; + let dispatch = tracing::Dispatch::new(subscriber); + let _guard = tracing::dispatcher::set_default(&dispatch); + + let whisper = std::ffi::CString::new("whisper").unwrap(); + let message = std::ffi::CString::new("hello from plugin").unwrap(); + + plugin_log_callback( + CLogLevel::Info, + whisper.as_ptr(), + message.as_ptr(), + std::ptr::null_mut(), + ); + + assert_eq!(event_count.load(Ordering::SeqCst), 1); + assert_eq!(*seen_target.lock().unwrap(), Some("whisper".to_string())); + } +} diff --git a/plugins/native/parakeet/src/parakeet_node.rs b/plugins/native/parakeet/src/parakeet_node.rs index 7f0d77be..d18a2a0e 100644 --- a/plugins/native/parakeet/src/parakeet_node.rs +++ b/plugins/native/parakeet/src/parakeet_node.rs @@ -2,11 +2,12 @@ // // SPDX-License-Identifier: MPL-2.0 -use std::collections::{HashMap, VecDeque}; +use std::collections::VecDeque; use std::ffi::{CStr, CString}; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; use streamkit_plugin_sdk_native::prelude::*; +use streamkit_plugin_sdk_native::resource_cache::ResourceCache; use streamkit_plugin_sdk_native::streamkit_core::types::{ AudioFormat, SampleFormat, TranscriptionData, TranscriptionSegment, }; @@ -43,18 +44,8 @@ impl Drop for RecognizerWrapper { } } -/// Cached recognizer -struct CachedRecognizer { - recognizer: Arc, -} - -/// Global cache of recognizers -/// Key: (model_dir, num_threads, execution_provider) -// Allow: Type complexity is acceptable here - composite key for caching recognizers -#[allow(clippy::type_complexity)] -static RECOGNIZER_CACHE: std::sync::LazyLock< - Mutex>, -> = std::sync::LazyLock::new(|| Mutex::new(HashMap::new())); +static RECOGNIZER_CACHE: ResourceCache<(String, i32, String), RecognizerWrapper> = + ResourceCache::new(); pub struct ParakeetNode { config: ParakeetConfig, @@ -78,9 +69,14 @@ pub struct ParakeetNode { logger: Logger, } -// SAFETY: We ensure thread-safety through Arc -unsafe impl Send for ParakeetNode {} -unsafe impl Sync for ParakeetNode {} +// All fields are Send+Sync: Logger and RecognizerWrapper have their +// own manual impls in their respective modules; the remaining fields +// are standard library types that auto-derive Send+Sync. +const _: () = { + const fn assert_send_sync() {} + // Compilation fails here if any field stops being Send+Sync. + assert_send_sync::(); +}; impl NativeProcessorNode for ParakeetNode { fn metadata() -> NodeMetadata { @@ -212,39 +208,13 @@ impl NativeProcessorNode for ParakeetNode { cache_key.2 ); - // Check cache - let cached_recognizer = { - let cache = RECOGNIZER_CACHE - .lock() - .map_err(|e| format!("Failed to lock recognizer cache: {e}"))?; - - plugin_info!(logger, "Cache has {} entries", cache.len()); - cache.get(&cache_key).map(|cached| cached.recognizer.clone()) - }; - - let recognizer = if let Some(rec) = cached_recognizer { - plugin_info!(logger, "CACHE HIT: Reusing cached recognizer"); - rec - } else { - plugin_info!(logger, "CACHE MISS: Creating new recognizer"); - - let recognizer_ptr = unsafe { create_recognizer(&logger, &model_dir, &config)? }; - let recognizer_arc = Arc::new(RecognizerWrapper::new(recognizer_ptr)); - - // Insert into cache - plugin_info!(logger, "Inserting recognizer into cache"); - let cache_size = { - let mut cache = RECOGNIZER_CACHE - .lock() - .map_err(|e| format!("Failed to lock recognizer cache: {e}"))?; - - cache.insert(cache_key, CachedRecognizer { recognizer: recognizer_arc.clone() }); - cache.len() - }; - plugin_info!(logger, "Cache now has {} entries", cache_size); - - recognizer_arc - }; + let recognizer = RECOGNIZER_CACHE + .get_or_init(cache_key, |_| { + plugin_info!(logger, "CACHE MISS: Creating new recognizer"); + let recognizer_ptr = unsafe { create_recognizer(&logger, &model_dir, &config)? }; + Ok(RecognizerWrapper::new(recognizer_ptr)) + }) + .map_err(|e| format!("recognizer cache: {e}"))?; // Initialize VAD if enabled let vad = if config.use_vad { diff --git a/sdks/plugin-sdk/native/src/conversions.rs b/sdks/plugin-sdk/native/src/conversions.rs index a8a4221d..06300089 100644 --- a/sdks/plugin-sdk/native/src/conversions.rs +++ b/sdks/plugin-sdk/native/src/conversions.rs @@ -366,7 +366,7 @@ pub fn packet_type_to_c(pt: &PacketType) -> (CPacketTypeInfo, CPacketTypeOwned) pub struct CPacketRepr { pub packet: CPacket, - _owned: CPacketOwned, + owned: CPacketOwned, } impl CPacketRepr { @@ -378,10 +378,9 @@ impl CPacketRepr { /// `metadata` that the older plugin cannot interpret. /// /// No-op if the packet is not `BinaryWithMeta`. - #[allow(clippy::used_underscore_binding)] pub fn downgrade_binary_with_meta(&mut self) { if self.packet.packet_type == CPacketType::BinaryWithMeta { - if let CPacketOwned::BinaryWithMeta(ref bwm) = self._owned { + if let CPacketOwned::BinaryWithMeta(ref bwm) = self.owned { // Point directly at the raw data buffer, same as the plain // Binary path in `packet_to_c`. self.packet = CPacket { @@ -389,10 +388,93 @@ impl CPacketRepr { data: bwm.binary.data.cast::(), len: bwm.binary.data_len, }; - // Keep _owned alive — the data pointer still references it. + // Keep owned alive — the data pointer still references it. } } } + + /// Enable zero-copy binary transfer for v9 plugins. + /// + /// For `BinaryWithMeta` packets, clones the `Bytes` (cheap Arc bump), + /// boxes it, and sets `buffer_handle` + `free_fn` on the underlying + /// `CBinaryPacket`. For plain `Binary` packets, first upgrades to + /// `BinaryWithMeta` format (with null content_type/metadata) so that + /// the `CBinaryPacket` struct is available for the handle. + /// + /// The plugin SDK's `packet_from_c` reclaims the box via + /// `Box::from_raw`, avoiding a memcpy. + /// + /// Ownership of the boxed `Bytes` is guarded by + /// [`BinaryWithMetaOwned`]'s `Drop` impl: if the plugin never reclaims + /// the handle (e.g. it drops/filters the packet), the `Drop` calls + /// `free_fn` to release the box. + /// + /// **Wire note (S2):** For plain `Binary` packets this method upgrades + /// the wire type to `BinaryWithMeta` (with null content_type/metadata) + /// so that the `CBinaryPacket` struct is available for the handle. + /// C plugins on v9 must handle `BinaryWithMeta` even when those + /// fields are null. + /// + /// **Lifetime:** For the `Binary` upgrade path, `self.packet.data` + /// (inherited from the original `CPacketRepr`) is stored into the + /// new `CBinaryPacket`. The caller must keep `self` (and thus the + /// original `CPacketOwned` backing storage) alive for the duration + /// of the FFI callback. This is satisfied by `CPacketRepr`'s + /// normal ownership model — the repr lives on the stack across the + /// FFI call. + /// + /// No-op for non-binary packet types. + pub fn set_binary_buffer_handle(&mut self, source_bytes: &bytes::Bytes) { + match self.packet.packet_type { + CPacketType::BinaryWithMeta => { + if let CPacketOwned::BinaryWithMeta(ref mut bwm) = self.owned { + // Free any previously-set handle to avoid a leak + // if this method is called more than once. + if !bwm.binary.buffer_handle.is_null() { + if let Some(f) = bwm.binary.free_fn { + f(bwm.binary.buffer_handle); + } + } + let cloned = Box::new(source_bytes.clone()); + bwm.binary.buffer_handle = Box::into_raw(cloned).cast::(); + bwm.binary.free_fn = Some(free_binary_buffer_handle); + } + }, + CPacketType::Binary => { + // Upgrade to BinaryWithMeta so we have a CBinaryPacket to + // attach the zero-copy handle to. + // + // `bp.data` inherits the original `self.packet.data` pointer, + // while `buffer_handle` holds a `Bytes::clone()` (Arc bump — + // same underlying buffer). This relies on `Bytes::clone()` + // preserving `as_ptr()` identity, which holds for the + // current `bytes` crate implementation (shared-reference + // Bytes always clones the Arc, not the data). + let mut bp = Box::new(CBinaryPacket { + data: self.packet.data.cast::(), + data_len: self.packet.len, + content_type: std::ptr::null(), + metadata: std::ptr::null(), + buffer_handle: std::ptr::null_mut(), + free_fn: None, + }); + let cloned = Box::new(source_bytes.clone()); + bp.buffer_handle = Box::into_raw(cloned).cast::(); + bp.free_fn = Some(free_binary_buffer_handle); + self.packet = CPacket { + packet_type: CPacketType::BinaryWithMeta, + data: std::ptr::from_mut::(&mut *bp).cast::(), + len: std::mem::size_of::(), + }; + self.owned = CPacketOwned::BinaryWithMeta(BinaryWithMetaOwned { + content_type: None, + metadata: None, + binary: bp, + }); + }, + _ => {}, + } + } } #[allow(dead_code)] // Owned values are kept alive to support FFI pointers during callbacks. @@ -433,6 +515,17 @@ struct BinaryWithMetaOwned { binary: Box, } +impl Drop for BinaryWithMetaOwned { + fn drop(&mut self) { + if !self.binary.buffer_handle.is_null() { + if let Some(free_fn) = self.binary.free_fn { + free_fn(self.binary.buffer_handle); + } + self.binary.buffer_handle = std::ptr::null_mut(); + } + } +} + pub fn metadata_to_c(meta: &PacketMetadata) -> CPacketMetadata { CPacketMetadata { timestamp_us: meta.timestamp_us.unwrap_or_default(), @@ -457,6 +550,43 @@ fn cstring_sanitize(s: &str) -> CString { CString::new(s).unwrap_or_else(|_| CString::new(s.replace('\0', " ")).unwrap_or_default()) } +/// Null out the `buffer_handle` of a [`CBinaryPacket`] through its parent +/// [`CPacket`]'s data pointer. +/// +/// Called after reclaiming or freeing the handle in [`packet_from_c`] so +/// that the host-side [`BinaryWithMetaOwned::Drop`] won't double-free. +/// +/// # Safety +/// +/// `data_ptr` must point to a valid, **heap-allocated** `CBinaryPacket` +/// owned by the caller for the duration of this call. Although +/// `CPacket::data` is typed as `*const` for ABI compatibility, this helper +/// writes through the pointer via `addr_of_mut!`; passing a packet whose +/// `data` field points into read-only memory (e.g. `.rodata`) is undefined +/// behavior. +/// +/// In practice, the host always constructs `CBinaryPacket` via +/// `Box::new(…)` in [`CPacketRepr`], so the writable-storage +/// precondition is satisfied. +unsafe fn null_binary_buffer_handle(data_ptr: *const c_void) { + let bp = data_ptr.cast::().cast_mut(); + std::ptr::addr_of_mut!((*bp).buffer_handle).write(std::ptr::null_mut()); +} + +/// Free a boxed `bytes::Bytes` stored as a `buffer_handle` in [`CBinaryPacket`]. +/// +/// Used as the `free_fn` field value — called if the plugin discards the +/// packet without reclaiming the buffer (e.g. on error paths). +extern "C" fn free_binary_buffer_handle(handle: *mut c_void) { + crate::ffi_guard::guard_unit("free_binary_buffer_handle", || { + if !handle.is_null() { + // SAFETY: handle was created by `Box::into_raw(Box::new(bytes))` in + // `set_binary_buffer_handle` and has not been reclaimed yet. + drop(unsafe { Box::from_raw(handle.cast::()) }); + } + }); +} + /// Convert Rust Packet to C representation. /// /// The returned representation owns any allocations needed for the duration of the C callback. @@ -479,7 +609,7 @@ pub fn packet_to_c(packet: &Packet) -> CPacketRepr { }; CPacketRepr { packet, - _owned: CPacketOwned::Audio(AudioOwned { frame: c_frame, metadata }), + owned: CPacketOwned::Audio(AudioOwned { frame: c_frame, metadata }), } }, Packet::Text(text) => { @@ -500,7 +630,7 @@ pub fn packet_to_c(packet: &Packet) -> CPacketRepr { data: c_str.as_ptr().cast::(), len: c_str.as_bytes_with_nul().len(), }; - CPacketRepr { packet, _owned: CPacketOwned::Text(c_str) } + CPacketRepr { packet, owned: CPacketOwned::Text(c_str) } }, Packet::Transcription(trans_data) => { let json = serde_json::to_vec(trans_data).unwrap_or_else(|e| { @@ -512,7 +642,7 @@ pub fn packet_to_c(packet: &Packet) -> CPacketRepr { data: json.as_ptr().cast::(), len: json.len(), }; - CPacketRepr { packet, _owned: CPacketOwned::Bytes(json) } + CPacketRepr { packet, owned: CPacketOwned::Bytes(json) } }, Packet::Custom(custom) => { let type_id = cstring_sanitize(custom.type_id.as_str()); @@ -540,7 +670,7 @@ pub fn packet_to_c(packet: &Packet) -> CPacketRepr { CPacketRepr { packet, - _owned: CPacketOwned::Custom(CustomOwned { + owned: CPacketOwned::Custom(CustomOwned { type_id, data_json, metadata, @@ -557,6 +687,8 @@ pub fn packet_to_c(packet: &Packet) -> CPacketRepr { data_len: data.len(), content_type: ct_cstring.as_ref().map_or(std::ptr::null(), |cs| cs.as_ptr()), metadata: meta_box.as_deref().map_or(std::ptr::null(), std::ptr::from_ref), + buffer_handle: std::ptr::null_mut(), + free_fn: None, }); let packet = CPacket { packet_type: CPacketType::BinaryWithMeta, @@ -565,7 +697,7 @@ pub fn packet_to_c(packet: &Packet) -> CPacketRepr { }; CPacketRepr { packet, - _owned: CPacketOwned::BinaryWithMeta(BinaryWithMetaOwned { + owned: CPacketOwned::BinaryWithMeta(BinaryWithMetaOwned { content_type: ct_cstring, metadata: meta_box, binary: bp, @@ -578,7 +710,7 @@ pub fn packet_to_c(packet: &Packet) -> CPacketRepr { data: data.as_ref().as_ptr().cast::(), len: data.len(), }, - _owned: CPacketOwned::None, + owned: CPacketOwned::None, } } }, @@ -600,7 +732,7 @@ pub fn packet_to_c(packet: &Packet) -> CPacketRepr { }; CPacketRepr { packet, - _owned: CPacketOwned::Video(VideoOwned { frame: c_frame, metadata }), + owned: CPacketOwned::Video(VideoOwned { frame: c_frame, metadata }), } }, } @@ -720,22 +852,68 @@ pub unsafe fn packet_from_c(c_packet: *const CPacket) -> Result }) }, CPacketType::BinaryWithMeta => { - let bp = &*c_pkt.data.cast::(); - if bp.data.is_null() && bp.data_len > 0 { + // Read all fields via raw-pointer dereference — no shared + // reference is created, so the subsequent write through + // null_binary_buffer_handle has no parent tag to conflict + // with under Stacked Borrows. + let bp = c_pkt.data.cast::(); + let data_ptr = (*bp).data; + let data_len = (*bp).data_len; + let content_type_ptr = (*bp).content_type; + let metadata_ptr = (*bp).metadata; + + // ABI compat: v7/v8 plugins allocate a smaller CBinaryPacket + // (4 fields, no buffer_handle/free_fn). CPacket::len is set + // to size_of::() by the sender at their SDK + // version. If it's smaller than the v9 struct, the new fields + // are past the allocation — reading them would be UB. + let is_v9_binary = c_pkt.len >= std::mem::size_of::(); + let (buffer_handle, free_fn) = if is_v9_binary { + ((*bp).buffer_handle, (*bp).free_fn) + } else { + (std::ptr::null_mut(), None) + }; + let has_handle = !buffer_handle.is_null(); + + if data_ptr.is_null() && data_len > 0 { + // Free buffer_handle via free_fn to honour the contract. + if has_handle { + if let Some(f) = free_fn { + f(buffer_handle); + } + null_binary_buffer_handle(c_pkt.data); + } return Err("BinaryWithMeta packet has null data pointer".to_string()); } - let data = if bp.data_len == 0 { + + let data = if data_len == 0 { + // Free buffer_handle via free_fn — the host may have set + // one even for an empty payload. + if has_handle { + if let Some(f) = free_fn { + f(buffer_handle); + } + null_binary_buffer_handle(c_pkt.data); + } bytes::Bytes::new() + } else if has_handle { + // v9 zero-copy path: reclaim the Bytes from the handle. + let bytes = *Box::from_raw(buffer_handle.cast::()); + // Null out so the host-side BinaryWithMetaOwned::Drop + // won't double-free. + null_binary_buffer_handle(c_pkt.data); + bytes } else { - bytes::Bytes::copy_from_slice(std::slice::from_raw_parts(bp.data, bp.data_len)) + // Legacy copy path (v8 host or null handle). + bytes::Bytes::copy_from_slice(std::slice::from_raw_parts(data_ptr, data_len)) }; - let content_type = if bp.content_type.is_null() { + let content_type = if content_type_ptr.is_null() { None } else { - Some(std::borrow::Cow::Owned(c_str_to_string(bp.content_type)?)) + Some(std::borrow::Cow::Owned(c_str_to_string(content_type_ptr)?)) }; let metadata = - if bp.metadata.is_null() { None } else { Some(metadata_from_c(&*bp.metadata)) }; + if metadata_ptr.is_null() { None } else { Some(metadata_from_c(&*metadata_ptr)) }; Ok(Packet::Binary { data, content_type, metadata }) }, CPacketType::RawVideo => { @@ -780,20 +958,12 @@ pub unsafe fn packet_from_c(c_packet: *const CPacket) -> Result .map_err(|e| format!("Invalid video frame: {e}")) } }, - CPacketType::EncodedVideo => { - // Encoded video is carried as opaque bytes across the C ABI. - let data = std::slice::from_raw_parts(c_pkt.data.cast::(), c_pkt.len); - Ok(Packet::Binary { - data: bytes::Bytes::copy_from_slice(data), - content_type: None, - metadata: None, - }) - }, - CPacketType::EncodedAudio => { - // EncodedAudio is a *type-level* discriminant used in pin - // declarations. At runtime, encoded audio packets travel as - // BinaryWithMeta (preserving content_type and metadata). - // If we somehow receive one here, treat it as opaque bytes. + CPacketType::EncodedVideo | CPacketType::EncodedAudio => { + // Both discriminants are type-level tags for pin declarations. + // At runtime, encoded packets normally travel as BinaryWithMeta + // (preserving content_type and metadata). If we receive one + // here (e.g. a plugin declared the raw discriminant), treat it + // as opaque bytes. let data = std::slice::from_raw_parts(c_pkt.data.cast::(), c_pkt.len); Ok(Packet::Binary { data: bytes::Bytes::copy_from_slice(data), @@ -837,10 +1007,18 @@ pub fn string_to_c(s: &str) -> *const c_char { /// Convert an error message to a C string for returning across the C ABI. /// +/// Despite the name, this is a generic "String → thread-local CString" +/// helper and is also used for non-error payloads (e.g. the JSON string +/// returned by `get_runtime_param_schema`). A rename to e.g. +/// `thread_local_c_str` would clarify intent but touches many call-sites. +/// /// # Ownership and lifetime /// /// The returned pointer is **borrowed** and **must not be freed** by the caller. -/// It remains valid until the next `error_to_c()` call on the same OS thread. +/// It remains valid until the next `error_to_c()` call **on the same OS +/// thread**. The host (or any nested FFI call back into the plugin) may +/// call `error_to_c` again and invalidate the previous pointer — callers +/// must copy the string into owned storage immediately after receiving it. /// /// This design: /// - Prevents host-side leaks when the host copies the message into an owned string. @@ -1113,4 +1291,234 @@ mod tests { } } } + + // ── Zero-copy binary handle tests ────────────────────────────────── + + /// Valid `buffer_handle` with invalid `content_type` UTF-8: the handle + /// must be reclaimed (no leak) even though content_type parsing fails. + #[test] + fn packet_from_c_valid_handle_invalid_content_type() { + let original = bytes::Bytes::from_static(b"test-payload"); + let handle = Box::into_raw(Box::new(original.clone())).cast::(); + + // Invalid UTF-8 followed by a null terminator. + let invalid_utf8: [u8; 3] = [0xFF, 0xFE, 0x00]; + let bp = CBinaryPacket { + data: original.as_ptr(), + data_len: original.len(), + content_type: invalid_utf8.as_ptr().cast::(), + metadata: std::ptr::null(), + buffer_handle: handle, + free_fn: Some(free_binary_buffer_handle), + }; + + let c_pkt = CPacket { + packet_type: CPacketType::BinaryWithMeta, + data: std::ptr::from_ref(&bp).cast(), + len: std::mem::size_of::(), + }; + + let result = unsafe { packet_from_c(&raw const c_pkt) }; + // The handle was reclaimed (zero-copy path) before the content_type + // error. If it weren't, this test would leak under miri/valgrind. + match result { + Err(msg) => assert!(msg.contains("Invalid UTF-8"), "unexpected error: {msg}"), + Ok(_) => panic!("expected error from invalid UTF-8 content_type"), + } + } + + /// Empty `BinaryWithMeta` (data_len == 0) with a non-null `buffer_handle`: + /// the handle must be freed even though data is empty. + #[test] + fn packet_from_c_empty_binary_with_handle_freed() { + let empty = bytes::Bytes::new(); + let handle = Box::into_raw(Box::new(empty)).cast::(); + + let bp = CBinaryPacket { + data: std::ptr::null(), + data_len: 0, + content_type: std::ptr::null(), + metadata: std::ptr::null(), + buffer_handle: handle, + free_fn: Some(free_binary_buffer_handle), + }; + + let c_pkt = CPacket { + packet_type: CPacketType::BinaryWithMeta, + data: std::ptr::from_ref(&bp).cast(), + len: std::mem::size_of::(), + }; + + let result = unsafe { packet_from_c(&raw const c_pkt) }; + let pkt = result + .unwrap_or_else(|e| panic!("empty BinaryWithMeta with handle should succeed: {e}")); + match pkt { + Packet::Binary { data, content_type, metadata } => { + assert!(data.is_empty()); + assert!(content_type.is_none()); + assert!(metadata.is_none()); + }, + other => panic!("expected Binary, got {other:?}"), + } + } + + /// `set_binary_buffer_handle` upgrades a plain `Binary` packet to + /// `BinaryWithMeta` and attaches the zero-copy handle. + #[test] + fn set_binary_buffer_handle_upgrades_plain_binary() { + let payload = bytes::Bytes::from_static(b"plain-data"); + let packet = Packet::Binary { data: payload.clone(), content_type: None, metadata: None }; + + let mut repr = packet_to_c(&packet); + assert_eq!(repr.packet.packet_type, CPacketType::Binary, "should start as Binary"); + + repr.set_binary_buffer_handle(&payload); + assert_eq!( + repr.packet.packet_type, + CPacketType::BinaryWithMeta, + "should be upgraded to BinaryWithMeta" + ); + + // Verify the packet round-trips through packet_from_c with zero-copy. + let result = unsafe { packet_from_c(&raw const repr.packet) }; + let pkt = result.unwrap_or_else(|e| panic!("upgraded packet should round-trip: {e}")); + match pkt { + Packet::Binary { data, content_type, metadata } => { + assert_eq!(data.as_ref(), b"plain-data"); + assert!(content_type.is_none()); + assert!(metadata.is_none()); + }, + other => panic!("expected Binary, got {other:?}"), + } + } + + /// Dropping a `CPacketRepr` with an unreclaimed buffer_handle must + /// free the handle via `BinaryWithMetaOwned::Drop` — no leak. + #[test] + fn dropping_packet_repr_frees_unreclaimed_handle() { + use std::sync::atomic::{AtomicBool, Ordering}; + + static FREED: AtomicBool = AtomicBool::new(false); + + extern "C" fn track_free(handle: *mut c_void) { + if !handle.is_null() { + FREED.store(true, Ordering::SeqCst); + // Also actually free the box to avoid real leak. + drop(unsafe { Box::from_raw(handle.cast::()) }); + } + } + + FREED.store(false, Ordering::SeqCst); + + let payload = bytes::Bytes::from_static(b"drop-test"); + let packet = Packet::Binary { + data: payload.clone(), + content_type: Some(std::borrow::Cow::Borrowed("application/octet-stream")), + metadata: None, + }; + + { + let mut repr = packet_to_c(&packet); + repr.set_binary_buffer_handle(&payload); + + // Overwrite the free_fn to our tracker. + if let CPacketOwned::BinaryWithMeta(ref mut bwm) = repr.owned { + bwm.binary.free_fn = Some(track_free); + } + + // Drop repr WITHOUT calling packet_from_c — simulates plugin + // dropping/filtering the packet. + } + + assert!(FREED.load(Ordering::SeqCst), "buffer_handle must be freed on drop"); + } + + /// v8 ABI compat: a `BinaryWithMeta` packet with `CPacket::len` smaller + /// than the v9 `CBinaryPacket` size must NOT read `buffer_handle`/`free_fn` + /// — those fields are past the allocation boundary. + #[test] + fn packet_from_c_v8_binary_with_meta_ignores_new_fields() { + // Simulate a v8-sized CBinaryPacket (4 fields, no buffer_handle/free_fn). + // We allocate a full v9 struct but set garbage in the new fields to + // prove they are NOT read when len < size_of::(). + let payload = bytes::Bytes::from_static(b"v8-payload"); + let bp = CBinaryPacket { + data: payload.as_ptr(), + data_len: payload.len(), + content_type: std::ptr::null(), + metadata: std::ptr::null(), + // Poison: if packet_from_c reads these, it would crash or + // enter the zero-copy path with an invalid pointer. + buffer_handle: 0xDEAD_BEEF_usize as *mut c_void, + free_fn: Some(free_binary_buffer_handle), + }; + + // v8 CBinaryPacket has 4 pointer-sized fields = 32 bytes on 64-bit. + let v8_size = 4 * std::mem::size_of::(); + assert!( + v8_size < std::mem::size_of::(), + "test assumes v9 CBinaryPacket is larger than v8" + ); + + let c_pkt = CPacket { + packet_type: CPacketType::BinaryWithMeta, + data: std::ptr::from_ref(&bp).cast(), + len: v8_size, // v8 size — smaller than v9 + }; + + // Must take the legacy copy path, not touch buffer_handle. + let result = unsafe { packet_from_c(&raw const c_pkt) }; + let pkt = result.unwrap_or_else(|e| panic!("v8 BinaryWithMeta should succeed: {e}")); + match pkt { + Packet::Binary { data, content_type, metadata } => { + assert_eq!(data.as_ref(), b"v8-payload"); + assert!(content_type.is_none()); + assert!(metadata.is_none()); + }, + other => panic!("expected Binary, got {other:?}"), + } + } + + /// After `packet_from_c` reclaims the handle, `BinaryWithMetaOwned::Drop` + /// must NOT double-free (handle is nulled out). + #[test] + fn packet_from_c_nulls_handle_prevents_double_free() { + let payload = bytes::Bytes::from_static(b"reclaim-test"); + let packet = Packet::Binary { data: payload.clone(), content_type: None, metadata: None }; + + let mut repr = packet_to_c(&packet); + repr.set_binary_buffer_handle(&payload); + + // Reclaim via packet_from_c — this should null out buffer_handle. + let result = unsafe { packet_from_c(&raw const repr.packet) }; + assert!(result.is_ok(), "reclaim should succeed"); + + // Verify the handle was nulled out. + if let CPacketOwned::BinaryWithMeta(ref bwm) = repr.owned { + assert!(bwm.binary.buffer_handle.is_null(), "buffer_handle must be null after reclaim"); + } else { + panic!("expected BinaryWithMeta owned variant"); + } + + // repr drops here — Drop should see null handle and be a no-op. + } + + #[test] + fn binary_buffer_handle_clone_preserves_bytes_pointer() { + let payload = bytes::Bytes::from_static(b"pointer-identity"); + let packet = Packet::Binary { data: payload.clone(), content_type: None, metadata: None }; + + let mut repr = packet_to_c(&packet); + repr.set_binary_buffer_handle(&payload); + + if let CPacketOwned::BinaryWithMeta(ref bwm) = repr.owned { + assert_eq!(bwm.binary.data, payload.as_ptr()); + assert!(!bwm.binary.buffer_handle.is_null()); + let cloned = unsafe { &*bwm.binary.buffer_handle.cast::() }; + assert_eq!(cloned.as_ptr(), payload.as_ptr()); + assert_eq!(cloned.len(), payload.len()); + } else { + panic!("expected BinaryWithMeta owned variant"); + } + } } diff --git a/sdks/plugin-sdk/native/src/ffi_guard.rs b/sdks/plugin-sdk/native/src/ffi_guard.rs new file mode 100644 index 00000000..bbf5f74b --- /dev/null +++ b/sdks/plugin-sdk/native/src/ffi_guard.rs @@ -0,0 +1,265 @@ +// SPDX-FileCopyrightText: © 2025 StreamKit Contributors +// +// SPDX-License-Identifier: MPL-2.0 + +//! FFI panic boundary helpers. +//! +//! Every `extern "C"` trampoline generated by the plugin entry macros must +//! run under one of these guards so that a panic inside plugin code never +//! unwinds across the C ABI boundary (which is instant UB on all platforms). +//! +//! # `AssertUnwindSafe` justification +//! +//! The closures capture raw pointers and references derived from them. +//! After a panic the plugin instance may be in an inconsistent state, but +//! the host will receive an error return and can decide to destroy the +//! instance. The alternative — aborting the entire server — is strictly +//! worse. + +use std::ffi::CString; +use std::panic::{catch_unwind, AssertUnwindSafe}; + +use crate::types; + +/// Extract a human-readable message from a panic payload. +/// +/// Handles `&str`, `String`, and `Box` payloads +/// (the latter covers e.g. [`std::panic::panic_any`] with a boxed error). +pub fn panic_message(payload: &(dyn std::any::Any + Send)) -> String { + if let Some(s) = payload.downcast_ref::<&str>() { + return (*s).to_string(); + } + if let Some(s) = payload.downcast_ref::() { + return s.clone(); + } + if let Some(e) = payload.downcast_ref::>() { + return e.to_string(); + } + "unknown panic".to_string() +} + +/// Guard a trampoline that returns [`types::CResult`]. +pub fn guard_result types::CResult>(f: F) -> types::CResult { + match catch_unwind(AssertUnwindSafe(f)) { + Ok(result) => result, + Err(payload) => { + let msg = panic_message(&*payload); + let err = crate::conversions::error_to_c(format!("plugin panicked: {msg}")); + types::CResult::error(err) + }, + } +} + +/// Guard a trampoline that returns [`types::CPluginHandle`] (null on panic). +/// +/// The host receives a null handle and must treat it as instance creation +/// failure. The panic message is logged via [`tracing::error!`]. +pub fn guard_handle types::CPluginHandle>(f: F) -> types::CPluginHandle { + match catch_unwind(AssertUnwindSafe(f)) { + Ok(handle) => handle, + Err(payload) => { + let msg = panic_message(&*payload); + tracing::error!("plugin panicked in create_instance: {msg}"); + std::ptr::null_mut() + }, + } +} + +/// Guard a trampoline that returns `*const T` (null on panic). +/// +/// `label` identifies the trampoline in the log message (e.g. `"get_metadata"`). +/// +/// The host receives a null pointer and must treat it as a load-time +/// failure. The panic message is logged via [`tracing::error!`]. +pub fn guard_ptr *const T>(label: &str, f: F) -> *const T { + match catch_unwind(AssertUnwindSafe(f)) { + Ok(ptr) => ptr, + Err(payload) => { + let msg = panic_message(&*payload); + tracing::error!("plugin panicked in {label}: {msg}"); + std::ptr::null() + }, + } +} + +/// Guard a trampoline that returns [`types::CTickResult`]. +pub fn guard_tick types::CTickResult>(f: F) -> types::CTickResult { + match catch_unwind(AssertUnwindSafe(f)) { + Ok(result) => result, + Err(payload) => { + let msg = panic_message(&*payload); + let err = crate::conversions::error_to_c(format!("plugin panicked: {msg}")); + types::CTickResult::error(err) + }, + } +} + +/// Guard a trampoline that returns [`types::CSchemaResult`]. +pub fn guard_schema types::CSchemaResult>(f: F) -> types::CSchemaResult { + match catch_unwind(AssertUnwindSafe(f)) { + Ok(result) => result, + Err(payload) => { + let msg = panic_message(&*payload); + let err = crate::conversions::error_to_c(format!("plugin panicked: {msg}")); + types::CSchemaResult::error(err) + }, + } +} + +/// Guard a trampoline that returns [`types::CSourceConfig`]. +/// +/// On panic the returned config has `is_source: false`, which tells the +/// host to treat this node as a processor rather than a source. Because +/// the node was *declared* as a source (it implements [`NativeSourceNode`]), +/// the pipeline will route no input packets to it, and it will never +/// receive `tick()` calls — effectively becoming an inert no-op that +/// produces no output. A `tracing::error!` is emitted so operators can +/// detect this and restart/remove the plugin. +pub fn guard_source_config types::CSourceConfig>(f: F) -> types::CSourceConfig { + match catch_unwind(AssertUnwindSafe(f)) { + Ok(cfg) => cfg, + Err(payload) => { + let msg = panic_message(&*payload); + tracing::error!( + "plugin panicked in get_source_config \ + (node reclassified as non-source): {msg}" + ); + types::CSourceConfig { is_source: false, tick_interval_us: 0, max_ticks: 0 } + }, + } +} + +/// Guard a trampoline that returns nothing. +/// +/// # Double-panic risk +/// +/// `__plugin_destroy_instance` wraps `cleanup()` in its own nested +/// `catch_unwind`, so a `cleanup` panic does not propagate into `Drop`. +/// Plugin authors should still avoid panicking in `Drop`. +pub fn guard_unit(label: &str, f: F) { + if let Err(payload) = catch_unwind(AssertUnwindSafe(f)) { + let msg = panic_message(&*payload); + tracing::error!("plugin panicked in {label}: {msg}"); + } +} + +/// Create a [`CString`], sanitizing embedded null bytes instead of panicking. +/// +/// If `s` contains null bytes they are stripped and a `tracing::error!` is +/// emitted with `context` so plugin authors can locate the mistake. This +/// avoids a panic inside `__plugin_get_metadata` that [`guard_ptr`] would +/// silently turn into a null return. +/// +pub fn cstring_lossy(s: &str, context: &str) -> CString { + match CString::new(s) { + Ok(c) => c, + Err(e) => { + tracing::error!("metadata field '{context}' contains null bytes: {e}"); + let sanitized = s.replace('\0', ""); + CString::new(sanitized) + .unwrap_or_else(|_| unreachable!("sanitized string must not contain null bytes")) + }, + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use super::*; + + #[test] + fn guard_result_catches_panic() { + let result = guard_result(|| panic!("boom")); + assert!(!result.success); + assert!(!result.error_message.is_null()); + let msg = unsafe { crate::conversions::c_str_to_string(result.error_message) } + .expect("valid UTF-8"); + assert!(msg.contains("boom"), "expected panic message, got: {msg}"); + } + + #[test] + fn guard_result_passes_through_ok() { + let result = guard_result(types::CResult::success); + assert!(result.success); + } + + #[test] + fn guard_handle_returns_null_on_panic() { + let handle = guard_handle(|| panic!("handle boom")); + assert!(handle.is_null()); + } + + #[test] + fn guard_ptr_returns_null_on_panic() { + let ptr: *const u8 = guard_ptr("test_trampoline", || panic!("ptr boom")); + assert!(ptr.is_null()); + } + + #[test] + fn guard_tick_catches_panic() { + let result = guard_tick(|| panic!("tick boom")); + assert!(!result.result.success); + } + + #[test] + fn guard_schema_catches_panic() { + let result = guard_schema(|| panic!("schema boom")); + assert!(!result.success); + } + + #[test] + fn guard_source_config_returns_default_on_panic() { + let cfg = guard_source_config(|| panic!("config boom")); + assert!(!cfg.is_source); + assert_eq!(cfg.tick_interval_us, 0); + assert_eq!(cfg.max_ticks, 0); + } + + #[test] + fn guard_unit_catches_panic() { + guard_unit("test_trampoline", || panic!("unit boom")); + } + + #[test] + fn panic_message_extracts_str() { + let msg = std::panic::catch_unwind(|| panic!("hello")).unwrap_err(); + assert_eq!(panic_message(&*msg), "hello"); + } + + #[test] + fn panic_message_extracts_string() { + let msg = std::panic::catch_unwind(|| panic!("{}", "formatted")).unwrap_err(); + assert_eq!(panic_message(&*msg), "formatted"); + } + + #[test] + fn panic_message_unknown_type() { + let msg = std::panic::catch_unwind(|| { + std::panic::panic_any(42_i32); + }) + .unwrap_err(); + assert_eq!(panic_message(&*msg), "unknown panic"); + } + + #[test] + fn panic_message_extracts_boxed_error() { + let err: Box = "boxed error".into(); + let msg = std::panic::catch_unwind(AssertUnwindSafe(|| { + std::panic::panic_any(err); + })) + .unwrap_err(); + assert_eq!(panic_message(&*msg), "boxed error"); + } + + #[test] + fn cstring_lossy_clean_string() { + let c = cstring_lossy("hello", "test"); + assert_eq!(c.to_str().unwrap(), "hello"); + } + + #[test] + fn cstring_lossy_strips_null_bytes() { + let c = cstring_lossy("hel\0lo", "test field"); + assert_eq!(c.to_str().unwrap(), "hello"); + } +} diff --git a/sdks/plugin-sdk/native/src/lib.rs b/sdks/plugin-sdk/native/src/lib.rs index 6dd1c853..2c48d134 100644 --- a/sdks/plugin-sdk/native/src/lib.rs +++ b/sdks/plugin-sdk/native/src/lib.rs @@ -44,12 +44,15 @@ //! ``` pub mod conversions; +pub mod ffi_guard; pub mod logger; +pub mod metadata_storage; +pub mod resource_cache; pub mod types; use std::ffi::CString; use streamkit_core::types::{Packet, PacketType}; -use streamkit_core::{InputPin, OutputPin, PinCardinality, Resource}; +use streamkit_core::{InputPin, OutputPin, PinCardinality}; use logger::Logger; @@ -78,14 +81,15 @@ pub use types::*; /// Re-export commonly used types pub mod prelude { pub use crate::logger::Logger; + pub use crate::resource_cache::{CacheError, ResourceCache}; pub use crate::types::{CLogCallback, CLogLevel}; pub use crate::{ native_plugin_entry, native_source_plugin_entry, plugin_debug, plugin_error, plugin_info, plugin_log, plugin_trace, plugin_warn, NativeProcessorNode, NativeSourceNode, NodeMetadata, - OutputSender, PooledAudioBuffer, PooledVideoBuffer, ResourceSupport, SourceConfig, + OutputSender, PooledAudioBuffer, PooledVideoBuffer, SourceConfig, }; pub use streamkit_core::types::{AudioFrame, Packet, PacketType}; - pub use streamkit_core::{InputPin, OutputPin, PinCardinality, Resource, UpstreamHint}; + pub use streamkit_core::{InputPin, OutputPin, PinCardinality, UpstreamHint}; } /// Metadata about a node type @@ -299,6 +303,14 @@ impl OutputSender { unsafe { &*self.callbacks } } + /// Check if a callback field at the given byte offset is within the + /// host-provided `CNodeCallbacks` struct. Returns `false` when the + /// host is older and doesn't include this field. + #[allow(clippy::missing_const_for_fn)] // Not const because self.cb() dereferences a raw pointer. + fn callback_available(&self, field_end_offset: usize) -> bool { + self.cb().struct_size >= field_end_offset + } + /// Send a packet to an output pin /// /// # Errors @@ -327,18 +339,23 @@ impl OutputSender { /// /// Returns `None` if the host has no video pool or allocation fails. pub fn alloc_video(&self, min_bytes: usize) -> Option { + let end = std::mem::offset_of!(types::CNodeCallbacks, alloc_video) + + std::mem::size_of::>(); + if !self.callback_available(end) { + return None; + } let cb = self.cb(); let alloc_fn = cb.alloc_video?; let res = alloc_fn(min_bytes, cb.alloc_user_data); - if res.data.is_null() || res.free_fn.is_none() { + let free_fn = res.free_fn?; + if res.data.is_null() { return None; } Some(PooledVideoBuffer { data: res.data, len: res.len, handle: res.handle, - // SAFETY: free_fn is guaranteed to be Some by the check above. - free_fn: unsafe { res.free_fn.unwrap_unchecked() }, + free_fn, consumed: false, }) } @@ -347,18 +364,23 @@ impl OutputSender { /// /// Returns `None` if the host has no audio pool or allocation fails. pub fn alloc_audio(&self, min_samples: usize) -> Option { + let end = std::mem::offset_of!(types::CNodeCallbacks, alloc_audio) + + std::mem::size_of::>(); + if !self.callback_available(end) { + return None; + } let cb = self.cb(); let alloc_fn = cb.alloc_audio?; let res = alloc_fn(min_samples, cb.alloc_user_data); - if res.data.is_null() || res.free_fn.is_none() { + let free_fn = res.free_fn?; + if res.data.is_null() { return None; } Some(PooledAudioBuffer { data: res.data, sample_count: res.sample_count, handle: res.handle, - // SAFETY: free_fn is guaranteed to be Some by the check above. - free_fn: unsafe { res.free_fn.unwrap_unchecked() }, + free_fn, consumed: false, }) } @@ -546,7 +568,14 @@ pub trait NativeProcessorNode: Sized + Send + 'static { Ok(()) } - /// Clean up resources (optional) + /// Clean up resources (optional). + /// + /// # Panics + /// + /// This method **must not panic**. It runs inside a `catch_unwind` + /// guard, but the plugin value is dropped immediately afterwards. + /// If both `cleanup()` and the type's `Drop` impl panic, the process + /// aborts (Rust double-panic rule). fn cleanup(&mut self) {} /// Return a runtime-discovered param schema after initialization (optional). @@ -561,6 +590,37 @@ pub trait NativeProcessorNode: Sized + Send + 'static { fn runtime_param_schema(&self) -> Option { None } + + /// Return a mutable reference to the plugin's [`Logger`] (v9). + /// + /// Override this to enable the host's log-enabled callback, allowing + /// `plugin_trace!` / `plugin_debug!` / etc. to short-circuit before + /// formatting when the level is disabled by the tracing subscriber. + /// + /// The host calls `set_log_enabled_callback` immediately after instance + /// creation; the SDK trampoline uses this method to inject the callback. + /// + /// **Clone caveat:** If the plugin clones the [`Logger`] before the + /// host injects the callback (i.e. during `create`), those clones + /// will **not** see the enabled callback. Recommended patterns for + /// multi-threaded plugins: + /// + /// - **Single owner + shared ref:** Store the `Logger` in the plugin + /// struct and pass `&Logger` to spawned tasks (requires scoped + /// threads or an `Arc>`). + /// - **Re-clone after create:** Clone the logger only after `create` + /// returns, at which point the callback is already injected. + /// - **`Arc>`:** Wrap in a mutex so all threads see + /// the injected callback. The lock is uncontended in practice + /// (only `logger_mut` needs `&mut`). + /// + /// Avoid `Arc` (without interior mutability) — `logger_mut` + /// cannot reach through an `Arc` to inject the callback. + /// + /// Default: `None` (no short-circuit — all levels always "enabled"). + fn logger_mut(&mut self) -> Option<&mut Logger> { + None + } } /// Configuration for a source node's tick loop. @@ -664,6 +724,13 @@ pub trait NativeSourceNode: Sized + Send + 'static { } /// Clean up resources (optional). + /// + /// # Panics + /// + /// This method **must not panic**. It runs inside a `catch_unwind` + /// guard, but the plugin value is dropped immediately afterwards. + /// If both `cleanup()` and the type's `Drop` impl panic, the process + /// aborts (Rust double-panic rule). fn cleanup(&mut self) {} /// Return a runtime-discovered param schema after initialization (optional). @@ -687,83 +754,22 @@ pub trait NativeSourceNode: Sized + Send + 'static { fn on_upstream_hint(&mut self, _hint: streamkit_core::UpstreamHint) { // default: ignore } -} -/// Optional trait for plugins that need shared resource management (e.g., ML models). -/// -/// Plugins that implement this trait can have their resources (models) automatically -/// cached and shared across multiple node instances. This avoids loading the same -/// model multiple times in memory. -/// -/// # Example -/// -/// ```ignore -/// use streamkit_plugin_sdk_native::prelude::*; -/// use std::sync::Arc; -/// -/// pub struct MyModelResource { -/// model_data: Vec, -/// } -/// -/// impl Resource for MyModelResource { -/// fn size_bytes(&self) -> usize { -/// self.model_data.len() * std::mem::size_of::() -/// } -/// fn resource_type(&self) -> &str { "ml_model" } -/// } -/// -/// pub struct MyPlugin { -/// resource: Arc, -/// } -/// -/// // Note: MyPlugin must also implement NativeProcessorNode for this to compile -/// impl ResourceSupport for MyPlugin { -/// type Resource = MyModelResource; -/// -/// fn compute_resource_key(params: Option<&serde_json::Value>) -> String { -/// // Hash only the params that affect resource creation -/// format!("{:?}", params) -/// } -/// -/// fn init_resource(params: Option) -> Result { -/// // Load model (can be expensive, but only happens once per unique params) -/// Ok(MyModelResource { model_data: vec![0.0; 1000] }) -/// } -/// } -/// ``` -pub trait ResourceSupport: NativeProcessorNode { - /// The type of resource this plugin uses - type Resource: Resource + 'static; - - /// Compute a cache key from parameters. - /// - /// This should hash only the parameters that affect resource initialization - /// (e.g., model path, GPU device ID). Different parameters that produce the - /// same key will share the same cached resource. - fn compute_resource_key(params: Option<&serde_json::Value>) -> String; - - /// Initialize/load the resource. + /// Return a mutable reference to the plugin's [`Logger`] (v9). /// - /// This is called once per unique cache key. The result is cached and shared - /// across all node instances with matching parameters. + /// Override this to enable the host's log-enabled callback, allowing + /// `plugin_trace!` / `plugin_debug!` / etc. to short-circuit before + /// formatting when the level is disabled by the tracing subscriber. /// - /// # Errors + /// **Clone caveat:** If the plugin clones the [`Logger`] before the + /// host injects the callback (i.e. during `create`), those clones + /// will **not** see the enabled callback. See + /// [`NativeProcessorNode::logger_mut`] for recommended multi-thread + /// patterns (`Arc>`, re-clone after create, etc.). /// - /// Returns an error if resource initialization fails (e.g., model file not found, - /// GPU initialization error). - /// - /// # Note - /// - /// This method may be called from a blocking thread pool to avoid blocking - /// async execution during model loading. - fn init_resource(params: Option) -> Result; - - /// Optional cleanup when the resource is being unloaded. - /// - /// This is called when the last reference to the resource is dropped - /// (typically during plugin unload or LRU eviction). - fn deinit_resource(_resource: Self::Resource) { - // Default: just drop it + /// Default: `None` (no short-circuit — all levels always "enabled"). + fn logger_mut(&mut self) -> Option<&mut Logger> { + None } } @@ -778,37 +784,65 @@ macro_rules! __plugin_shared_ffi { extern "C" fn __plugin_get_runtime_param_schema( handle: $crate::types::CPluginHandle, ) -> $crate::types::CSchemaResult { - if handle.is_null() { - return $crate::types::CSchemaResult::none(); - } - - let instance = unsafe { &*(handle as *const $plugin_type) }; - match instance.runtime_param_schema() { - None => $crate::types::CSchemaResult::none(), - Some(schema) => match serde_json::to_string(&schema) { - Ok(json) => { - // NOTE: error_to_c is a misnomer here — it's a generic - // "String → thread-local CString" helper reused for the - // success payload. A rename to e.g. `thread_local_c_str` - // would clarify intent but touches many call-sites. - let c_str = $crate::conversions::error_to_c(json); - $crate::types::CSchemaResult::schema(c_str) - }, - Err(e) => { - let err_msg = $crate::conversions::error_to_c(format!( - "Failed to serialize runtime param schema: {e}" - )); - $crate::types::CSchemaResult::error(err_msg) + $crate::ffi_guard::guard_schema(|| { + if handle.is_null() { + return $crate::types::CSchemaResult::none(); + } + + let instance = unsafe { &*(handle as *const $plugin_type) }; + match instance.runtime_param_schema() { + None => $crate::types::CSchemaResult::none(), + Some(schema) => match serde_json::to_string(&schema) { + Ok(json) => { + let c_str = $crate::conversions::error_to_c(json); + $crate::types::CSchemaResult::schema(c_str) + }, + Err(e) => { + let err_msg = $crate::conversions::error_to_c(format!( + "Failed to serialize runtime param schema: {e}" + )); + $crate::types::CSchemaResult::error(err_msg) + }, }, - }, - } + } + }) } extern "C" fn __plugin_destroy_instance(handle: $crate::types::CPluginHandle) { - if !handle.is_null() { - let mut instance = unsafe { Box::from_raw(handle as *mut $plugin_type) }; - instance.cleanup(); - } + $crate::ffi_guard::guard_unit("destroy_instance", || { + if !handle.is_null() { + let mut instance = unsafe { Box::from_raw(handle as *mut $plugin_type) }; + // Run cleanup() in a nested catch_unwind so that a + // panic here does not cause a double-panic abort if + // Drop also panics. + if let Err(payload) = + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + instance.cleanup() + })) + { + let msg = $crate::ffi_guard::panic_message(&*payload); + tracing::error!("plugin cleanup() panicked: {msg}"); + } + // instance (Box) is dropped here — if Drop panics, + // the outer guard_unit catches it. + } + }) + } + + extern "C" fn __plugin_set_log_enabled_callback( + handle: $crate::types::CPluginHandle, + callback: $crate::types::CLogEnabledCallback, + user_data: *mut std::os::raw::c_void, + ) { + $crate::ffi_guard::guard_unit("set_log_enabled_callback", || { + if handle.is_null() { + return; + } + let instance = unsafe { &mut *(handle as *mut $plugin_type) }; + if let Some(logger) = instance.logger_mut() { + logger.set_enabled_callback(callback, user_data); + } + }) } }; } @@ -832,26 +866,8 @@ macro_rules! __plugin_shared_ffi { #[macro_export] macro_rules! native_plugin_entry { ($plugin_type:ty) => { - // Static metadata storage - static mut METADATA: std::sync::OnceLock<( - $crate::types::CNodeMetadata, - Vec<$crate::types::CInputPin>, - Vec<$crate::types::COutputPin>, - Vec, - Vec>, - Vec>>, - Vec>>, - Vec>>, - Vec, - Vec>, - Vec>, - Vec>, - Vec, - Vec<*const std::os::raw::c_char>, - std::ffi::CString, - Option, - std::ffi::CString, - )> = std::sync::OnceLock::new(); + static METADATA: std::sync::OnceLock<$crate::metadata_storage::PluginMetadataStorage> = + std::sync::OnceLock::new(); #[no_mangle] pub extern "C" fn streamkit_native_plugin_api() -> *const $crate::types::CNativePluginAPI { @@ -871,318 +887,23 @@ macro_rules! native_plugin_entry { &API } + #[no_mangle] + pub extern "C" fn streamkit_native_plugin_set_log_enabled_callback( + handle: $crate::types::CPluginHandle, + callback: $crate::types::CLogEnabledCallback, + user_data: *mut std::os::raw::c_void, + ) { + __plugin_set_log_enabled_callback(handle, callback, user_data); + } + extern "C" fn __plugin_get_metadata() -> *const $crate::types::CNodeMetadata { - unsafe { - let metadata = METADATA.get_or_init(|| { + $crate::ffi_guard::guard_ptr("get_metadata", || { + let storage = METADATA.get_or_init(|| { let meta = <$plugin_type as $crate::NativeProcessorNode>::metadata(); - - // Convert inputs - let mut c_inputs = Vec::new(); - let mut input_names = Vec::new(); - let mut input_types = Vec::new(); - let mut input_audio_formats = Vec::new(); - let mut input_custom_type_ids = Vec::new(); - let mut input_video_formats = Vec::new(); - - for input in &meta.inputs { - let name = std::ffi::CString::new(input.name.as_str()) - .expect("Input pin name should not contain null bytes"); - let mut types_info = Vec::new(); - let mut audio_formats = Vec::new(); - let mut custom_type_ids = Vec::new(); - let mut video_formats = Vec::new(); - - for pt in &input.accepts_types { - let audio_format = match pt { - $crate::streamkit_core::types::PacketType::RawAudio(af) => { - Some($crate::conversions::audio_format_to_c(af)) - } - _ => None, - }; - audio_formats.push(audio_format); - let custom_type_id = match pt { - $crate::streamkit_core::types::PacketType::Custom { type_id } => { - Some(std::ffi::CString::new(type_id.as_str()).expect( - "Custom type_id should not contain null bytes", - )) - } - $crate::streamkit_core::types::PacketType::EncodedAudio(format) => { - Some($crate::conversions::codec_name_to_cstring(format.codec.as_c_name())) - } - $crate::streamkit_core::types::PacketType::EncodedVideo(format) => { - Some($crate::conversions::codec_name_to_cstring(format.codec.as_c_name())) - } - _ => None, - }; - custom_type_ids.push(custom_type_id); - let video_format = match pt { - $crate::streamkit_core::types::PacketType::RawVideo(vf) => { - Some($crate::conversions::raw_video_format_to_c(vf)) - } - _ => None, - }; - video_formats.push(video_format); - } - - // Now create CPacketTypeInfo with stable pointers to the stored formats - for (idx, pt) in input.accepts_types.iter().enumerate() { - let type_discriminant = match pt { - $crate::streamkit_core::types::PacketType::RawAudio(_) => { - $crate::types::CPacketType::RawAudio - } - $crate::streamkit_core::types::PacketType::EncodedAudio(format) => { - if format.codec - == $crate::streamkit_core::types::AudioCodec::Opus - && format.codec_private.is_none() - { - $crate::types::CPacketType::OpusAudio - } else { - $crate::types::CPacketType::EncodedAudio - } - } - $crate::streamkit_core::types::PacketType::RawVideo(_) => { - $crate::types::CPacketType::RawVideo - } - $crate::streamkit_core::types::PacketType::EncodedVideo(_) => { - $crate::types::CPacketType::EncodedVideo - } - $crate::streamkit_core::types::PacketType::Text => { - $crate::types::CPacketType::Text - } - $crate::streamkit_core::types::PacketType::Transcription => { - $crate::types::CPacketType::Transcription - } - $crate::streamkit_core::types::PacketType::Custom { .. } => { - $crate::types::CPacketType::Custom - } - $crate::streamkit_core::types::PacketType::Binary => { - $crate::types::CPacketType::Binary - } - $crate::streamkit_core::types::PacketType::Any => { - $crate::types::CPacketType::Any - } - $crate::streamkit_core::types::PacketType::Passthrough => { - $crate::types::CPacketType::Any - } - }; - - let audio_format_ptr = if let Some(ref fmt) = audio_formats[idx] { - fmt as *const $crate::types::CAudioFormat - } else { - std::ptr::null() - }; - - let custom_type_id_ptr = if let Some(ref s) = custom_type_ids[idx] { - s.as_ptr() - } else { - std::ptr::null() - }; - - let video_format_ptr = if let Some(ref vf) = video_formats[idx] { - vf as *const $crate::types::CRawVideoFormat - } else { - std::ptr::null() - }; - - types_info.push($crate::types::CPacketTypeInfo { - type_discriminant, - audio_format: audio_format_ptr, - custom_type_id: custom_type_id_ptr, - raw_video_format: video_format_ptr, - }); - } - - c_inputs.push($crate::types::CInputPin { - name: name.as_ptr(), - accepts_types: types_info.as_ptr(), - accepts_types_count: types_info.len(), - }); - - input_names.push(name); - input_types.push(types_info); - input_audio_formats.push(audio_formats); - input_custom_type_ids.push(custom_type_ids); - input_video_formats.push(video_formats); - } - - // Convert outputs - let mut c_outputs = Vec::new(); - let mut output_names = Vec::new(); - let mut output_audio_formats = Vec::new(); - let mut output_custom_type_ids = Vec::new(); - let mut output_video_formats = Vec::new(); - - for output in &meta.outputs { - let name = std::ffi::CString::new(output.name.as_str()) - .expect("Output pin name should not contain null bytes"); - - let audio_format = match &output.produces_type { - $crate::streamkit_core::types::PacketType::RawAudio(af) => { - Some($crate::conversions::audio_format_to_c(af)) - } - _ => None, - }; - output_audio_formats.push(audio_format); - let output_custom_type_id = match &output.produces_type { - $crate::streamkit_core::types::PacketType::Custom { type_id } => { - Some(std::ffi::CString::new(type_id.as_str()).expect( - "Custom type_id should not contain null bytes", - )) - } - $crate::streamkit_core::types::PacketType::EncodedAudio(format) => { - Some($crate::conversions::codec_name_to_cstring(format.codec.as_c_name())) - } - $crate::streamkit_core::types::PacketType::EncodedVideo(format) => { - Some($crate::conversions::codec_name_to_cstring(format.codec.as_c_name())) - } - _ => None, - }; - output_custom_type_ids.push(output_custom_type_id); - let video_format = match &output.produces_type { - $crate::streamkit_core::types::PacketType::RawVideo(vf) => { - Some($crate::conversions::raw_video_format_to_c(vf)) - } - _ => None, - }; - output_video_formats.push(video_format); - - // Now create CPacketTypeInfo with stable pointer to the stored format - let type_discriminant = match &output.produces_type { - $crate::streamkit_core::types::PacketType::RawAudio(_) => { - $crate::types::CPacketType::RawAudio - } - $crate::streamkit_core::types::PacketType::EncodedAudio(format) => { - if format.codec - == $crate::streamkit_core::types::AudioCodec::Opus - && format.codec_private.is_none() - { - $crate::types::CPacketType::OpusAudio - } else { - $crate::types::CPacketType::EncodedAudio - } - } - $crate::streamkit_core::types::PacketType::RawVideo(_) => { - $crate::types::CPacketType::RawVideo - } - $crate::streamkit_core::types::PacketType::EncodedVideo(_) => { - $crate::types::CPacketType::EncodedVideo - } - $crate::streamkit_core::types::PacketType::Text => { - $crate::types::CPacketType::Text - } - $crate::streamkit_core::types::PacketType::Transcription => { - $crate::types::CPacketType::Transcription - } - $crate::streamkit_core::types::PacketType::Custom { .. } => { - $crate::types::CPacketType::Custom - } - $crate::streamkit_core::types::PacketType::Binary => { - $crate::types::CPacketType::Binary - } - $crate::streamkit_core::types::PacketType::Any => { - $crate::types::CPacketType::Any - } - $crate::streamkit_core::types::PacketType::Passthrough => { - $crate::types::CPacketType::Any - } - }; - - // SAFETY: We just pushed an element, so last() is guaranteed to be Some - #[allow(clippy::unwrap_used)] - let audio_format_ptr = - if let Some(ref fmt) = output_audio_formats.last().unwrap() { - fmt as *const $crate::types::CAudioFormat - } else { - std::ptr::null() - }; - - // SAFETY: We just pushed an element, so last() is guaranteed to be Some - #[allow(clippy::unwrap_used)] - let custom_type_id_ptr = - if let Some(ref s) = output_custom_type_ids.last().unwrap() { - s.as_ptr() - } else { - std::ptr::null() - }; - - // SAFETY: We just pushed an element, so last() is guaranteed to be Some - #[allow(clippy::unwrap_used)] - let video_format_ptr = - if let Some(ref vf) = output_video_formats.last().unwrap() { - vf as *const $crate::types::CRawVideoFormat - } else { - std::ptr::null() - }; - - let type_info = $crate::types::CPacketTypeInfo { - type_discriminant, - audio_format: audio_format_ptr, - custom_type_id: custom_type_id_ptr, - raw_video_format: video_format_ptr, - }; - - c_outputs.push($crate::types::COutputPin { - name: name.as_ptr(), - produces_type: type_info, - }); - output_names.push(name); - } - - // Convert categories - let mut category_strings = Vec::new(); - let mut category_ptrs = Vec::new(); - - for cat in &meta.categories { - let c_str = std::ffi::CString::new(cat.as_str()) - .expect("Category name should not contain null bytes"); - category_ptrs.push(c_str.as_ptr()); - category_strings.push(c_str); - } - - let kind = std::ffi::CString::new(meta.kind.as_str()) - .expect("Node kind should not contain null bytes"); - let description = meta.description.as_ref().map(|d| { - std::ffi::CString::new(d.as_str()) - .expect("Description should not contain null bytes") - }); - let param_schema = std::ffi::CString::new(meta.param_schema.to_string()) - .expect("Param schema JSON should not contain null bytes"); - - let c_metadata = $crate::types::CNodeMetadata { - kind: kind.as_ptr(), - description: description.as_ref().map_or(std::ptr::null(), |d| d.as_ptr()), - inputs: c_inputs.as_ptr(), - inputs_count: c_inputs.len(), - outputs: c_outputs.as_ptr(), - outputs_count: c_outputs.len(), - param_schema: param_schema.as_ptr(), - categories: category_ptrs.as_ptr(), - categories_count: category_ptrs.len(), - }; - - ( - c_metadata, - c_inputs, - c_outputs, - input_names, - input_types, - input_audio_formats, - input_custom_type_ids, - input_video_formats, - output_names, - output_audio_formats, - output_custom_type_ids, - output_video_formats, - category_strings, - category_ptrs, - kind, - description, - param_schema, - ) + $crate::metadata_storage::PluginMetadataStorage::from_node_metadata(&meta) }); - - &metadata.0 - } + &storage.c_metadata + }) } extern "C" fn __plugin_create_instance( @@ -1190,26 +911,30 @@ macro_rules! native_plugin_entry { log_callback: $crate::types::CLogCallback, log_user_data: *mut std::os::raw::c_void, ) -> $crate::types::CPluginHandle { - let params_json = if params.is_null() { - None - } else { - match unsafe { $crate::conversions::c_str_to_string(params) } { - Ok(s) if s.is_empty() => None, - Ok(s) => match serde_json::from_str(&s) { - Ok(v) => Some(v), + $crate::ffi_guard::guard_handle(|| { + let params_json = if params.is_null() { + None + } else { + match unsafe { $crate::conversions::c_str_to_string(params) } { + Ok(s) if s.is_empty() => None, + Ok(s) => match serde_json::from_str(&s) { + Ok(v) => Some(v), + Err(_) => return std::ptr::null_mut(), + }, Err(_) => return std::ptr::null_mut(), - }, - Err(_) => return std::ptr::null_mut(), - } - }; + } + }; - // Create logger for this plugin instance - let logger = $crate::logger::Logger::new(log_callback, log_user_data, module_path!()); + // Create logger using the plugin's kind as target (e.g. "whisper") + // instead of module_path! which is opaque to the user. + let kind = <$plugin_type as $crate::NativeProcessorNode>::metadata().kind; + let logger = $crate::logger::Logger::new(log_callback, log_user_data, &kind); - match <$plugin_type as $crate::NativeProcessorNode>::new(params_json, logger) { - Ok(instance) => Box::into_raw(Box::new(instance)) as $crate::types::CPluginHandle, - Err(_) => std::ptr::null_mut(), - } + match <$plugin_type as $crate::NativeProcessorNode>::new(params_json, logger) { + Ok(instance) => Box::into_raw(Box::new(instance)) as $crate::types::CPluginHandle, + Err(_) => std::ptr::null_mut(), + } + }) } extern "C" fn __plugin_process_packet( @@ -1218,108 +943,114 @@ macro_rules! native_plugin_entry { packet: *const $crate::types::CPacket, callbacks: *const $crate::types::CNodeCallbacks, ) -> $crate::types::CResult { - if handle.is_null() || input_pin.is_null() || packet.is_null() || callbacks.is_null() { - return $crate::types::CResult::error(std::ptr::null()); - } + $crate::ffi_guard::guard_result(|| { + if handle.is_null() || input_pin.is_null() || packet.is_null() || callbacks.is_null() { + return $crate::types::CResult::error(std::ptr::null()); + } - let instance = unsafe { &mut *(handle as *mut $plugin_type) }; + let instance = unsafe { &mut *(handle as *mut $plugin_type) }; - let pin_name = match unsafe { $crate::conversions::c_str_to_string(input_pin) } { - Ok(s) => s, - Err(e) => { - let err_msg = $crate::conversions::error_to_c(format!("Invalid pin name: {}", e)); - return $crate::types::CResult::error(err_msg); - } - }; + let pin_name = match unsafe { $crate::conversions::c_str_to_string(input_pin) } { + Ok(s) => s, + Err(e) => { + let err_msg = $crate::conversions::error_to_c(format!("Invalid pin name: {}", e)); + return $crate::types::CResult::error(err_msg); + } + }; - let rust_packet = match unsafe { $crate::conversions::packet_from_c(packet) } { - Ok(p) => p, - Err(e) => { - let err_msg = $crate::conversions::error_to_c(format!("Invalid packet: {}", e)); - return $crate::types::CResult::error(err_msg); - } - }; + let rust_packet = match unsafe { $crate::conversions::packet_from_c(packet) } { + Ok(p) => p, + Err(e) => { + let err_msg = $crate::conversions::error_to_c(format!("Invalid packet: {}", e)); + return $crate::types::CResult::error(err_msg); + } + }; - let output = unsafe { $crate::OutputSender::from_node_callbacks(callbacks) }; + let output = unsafe { $crate::OutputSender::from_node_callbacks(callbacks) }; - match instance.process(&pin_name, rust_packet, &output) { - Ok(()) => $crate::types::CResult::success(), - Err(e) => { - let err_msg = $crate::conversions::error_to_c(e); - $crate::types::CResult::error(err_msg) + match instance.process(&pin_name, rust_packet, &output) { + Ok(()) => $crate::types::CResult::success(), + Err(e) => { + let err_msg = $crate::conversions::error_to_c(e); + $crate::types::CResult::error(err_msg) + } } - } + }) } extern "C" fn __plugin_update_params( handle: $crate::types::CPluginHandle, params: *const std::os::raw::c_char, ) -> $crate::types::CResult { - if handle.is_null() { - let err_msg = $crate::conversions::error_to_c("Invalid handle (null)"); - return $crate::types::CResult::error(err_msg); - } - - let instance = unsafe { &mut *(handle as *mut $plugin_type) }; - - let params_json = if params.is_null() { - None - } else { - match unsafe { $crate::conversions::c_str_to_string(params) } { - Ok(s) if s.is_empty() => None, - Ok(s) => match serde_json::from_str(&s) { - Ok(v) => Some(v), + $crate::ffi_guard::guard_result(|| { + if handle.is_null() { + let err_msg = $crate::conversions::error_to_c("Invalid handle (null)"); + return $crate::types::CResult::error(err_msg); + } + + let instance = unsafe { &mut *(handle as *mut $plugin_type) }; + + let params_json = if params.is_null() { + None + } else { + match unsafe { $crate::conversions::c_str_to_string(params) } { + Ok(s) if s.is_empty() => None, + Ok(s) => match serde_json::from_str(&s) { + Ok(v) => Some(v), + Err(e) => { + let err_msg = + $crate::conversions::error_to_c(format!("Invalid params JSON: {e}")); + return $crate::types::CResult::error(err_msg); + }, + }, Err(e) => { let err_msg = - $crate::conversions::error_to_c(format!("Invalid params JSON: {e}")); + $crate::conversions::error_to_c(format!("Invalid params string: {e}")); return $crate::types::CResult::error(err_msg); }, - }, + } + }; + + match instance.update_params(params_json) { + Ok(()) => $crate::types::CResult::success(), Err(e) => { - let err_msg = - $crate::conversions::error_to_c(format!("Invalid params string: {e}")); - return $crate::types::CResult::error(err_msg); + let err_msg = $crate::conversions::error_to_c(e); + $crate::types::CResult::error(err_msg) }, } - }; - - match instance.update_params(params_json) { - Ok(()) => $crate::types::CResult::success(), - Err(e) => { - let err_msg = $crate::conversions::error_to_c(e); - $crate::types::CResult::error(err_msg) - }, - } + }) } extern "C" fn __plugin_flush( handle: $crate::types::CPluginHandle, callbacks: *const $crate::types::CNodeCallbacks, ) -> $crate::types::CResult { - tracing::trace!("__plugin_flush called"); - if handle.is_null() || callbacks.is_null() { - tracing::error!("Handle or callbacks is null"); - let err_msg = $crate::conversions::error_to_c("Invalid handle or callbacks (null)"); - return $crate::types::CResult::error(err_msg); - } - - let instance = unsafe { &mut *(handle as *mut $plugin_type) }; - tracing::trace!("Got instance pointer"); - - let output_sender = unsafe { $crate::OutputSender::from_node_callbacks(callbacks) }; - tracing::trace!("Created OutputSender, calling instance.flush()"); - - match instance.flush(&output_sender) { - Ok(()) => { - tracing::trace!("instance.flush() returned Ok"); - $crate::types::CResult::success() - }, - Err(e) => { - tracing::error!(error = %e, "instance.flush() returned Err"); - let err_msg = $crate::conversions::error_to_c(e); - $crate::types::CResult::error(err_msg) - }, - } + $crate::ffi_guard::guard_result(|| { + tracing::trace!("__plugin_flush called"); + if handle.is_null() || callbacks.is_null() { + tracing::error!("Handle or callbacks is null"); + let err_msg = $crate::conversions::error_to_c("Invalid handle or callbacks (null)"); + return $crate::types::CResult::error(err_msg); + } + + let instance = unsafe { &mut *(handle as *mut $plugin_type) }; + tracing::trace!("Got instance pointer"); + + let output_sender = unsafe { $crate::OutputSender::from_node_callbacks(callbacks) }; + tracing::trace!("Created OutputSender, calling instance.flush()"); + + match instance.flush(&output_sender) { + Ok(()) => { + tracing::trace!("instance.flush() returned Ok"); + $crate::types::CResult::success() + }, + Err(e) => { + tracing::error!(error = %e, "instance.flush() returned Err"); + let err_msg = $crate::conversions::error_to_c(e); + $crate::types::CResult::error(err_msg) + }, + } + }) } $crate::__plugin_shared_ffi!($plugin_type); @@ -1351,26 +1082,8 @@ macro_rules! native_plugin_entry { #[macro_export] macro_rules! native_source_plugin_entry { ($plugin_type:ty) => { - // Static metadata storage (same layout as processor macro + video format vecs) - static mut METADATA: std::sync::OnceLock<( - $crate::types::CNodeMetadata, - Vec<$crate::types::CInputPin>, - Vec<$crate::types::COutputPin>, - Vec, - Vec>, - Vec>>, - Vec>>, - Vec>>, - Vec, - Vec>, - Vec>, - Vec>, - Vec, - Vec<*const std::os::raw::c_char>, - std::ffi::CString, - Option, - std::ffi::CString, - )> = std::sync::OnceLock::new(); + static METADATA: std::sync::OnceLock<$crate::metadata_storage::PluginMetadataStorage> = + std::sync::OnceLock::new(); #[no_mangle] pub extern "C" fn streamkit_native_plugin_api() -> *const $crate::types::CNativePluginAPI { @@ -1390,327 +1103,23 @@ macro_rules! native_source_plugin_entry { &API } - // ── Metadata ──────────────────────────────────────────────────── - // Reuse the same metadata-building logic as the processor macro. - // Source nodes typically have zero inputs and one or more outputs. + #[no_mangle] + pub extern "C" fn streamkit_native_plugin_set_log_enabled_callback( + handle: $crate::types::CPluginHandle, + callback: $crate::types::CLogEnabledCallback, + user_data: *mut std::os::raw::c_void, + ) { + __plugin_set_log_enabled_callback(handle, callback, user_data); + } extern "C" fn __plugin_get_metadata() -> *const $crate::types::CNodeMetadata { - unsafe { - let metadata = METADATA.get_or_init(|| { + $crate::ffi_guard::guard_ptr("get_metadata", || { + let storage = METADATA.get_or_init(|| { let meta = <$plugin_type as $crate::NativeSourceNode>::metadata(); - - // Convert inputs (usually empty for source nodes) - let mut c_inputs = Vec::new(); - let mut input_names = Vec::new(); - let mut input_types = Vec::new(); - let mut input_audio_formats = Vec::new(); - let mut input_custom_type_ids = Vec::new(); - let mut input_video_formats = Vec::new(); - - for input in &meta.inputs { - let name = std::ffi::CString::new(input.name.as_str()) - .expect("Input pin name should not contain null bytes"); - let mut types_info = Vec::new(); - let mut audio_formats = Vec::new(); - let mut custom_type_ids = Vec::new(); - let mut video_formats = Vec::new(); - - for pt in &input.accepts_types { - let audio_format = match pt { - $crate::streamkit_core::types::PacketType::RawAudio(af) => { - Some($crate::conversions::audio_format_to_c(af)) - }, - _ => None, - }; - audio_formats.push(audio_format); - let custom_type_id = match pt { - $crate::streamkit_core::types::PacketType::Custom { type_id } => { - Some( - std::ffi::CString::new(type_id.as_str()) - .expect("Custom type_id should not contain null bytes"), - ) - }, - $crate::streamkit_core::types::PacketType::EncodedAudio(format) => { - Some($crate::conversions::codec_name_to_cstring( - format.codec.as_c_name(), - )) - }, - $crate::streamkit_core::types::PacketType::EncodedVideo(format) => { - Some($crate::conversions::codec_name_to_cstring( - format.codec.as_c_name(), - )) - }, - _ => None, - }; - custom_type_ids.push(custom_type_id); - let video_format = match pt { - $crate::streamkit_core::types::PacketType::RawVideo(vf) => { - Some($crate::conversions::raw_video_format_to_c(vf)) - }, - _ => None, - }; - video_formats.push(video_format); - } - - for (idx, pt) in input.accepts_types.iter().enumerate() { - let type_discriminant = match pt { - $crate::streamkit_core::types::PacketType::RawAudio(_) => { - $crate::types::CPacketType::RawAudio - }, - $crate::streamkit_core::types::PacketType::EncodedAudio(format) => { - if format.codec - == $crate::streamkit_core::types::AudioCodec::Opus - && format.codec_private.is_none() - { - $crate::types::CPacketType::OpusAudio - } else { - $crate::types::CPacketType::EncodedAudio - } - }, - $crate::streamkit_core::types::PacketType::RawVideo(_) => { - $crate::types::CPacketType::RawVideo - }, - $crate::streamkit_core::types::PacketType::EncodedVideo(_) => { - $crate::types::CPacketType::EncodedVideo - }, - $crate::streamkit_core::types::PacketType::Text => { - $crate::types::CPacketType::Text - }, - $crate::streamkit_core::types::PacketType::Transcription => { - $crate::types::CPacketType::Transcription - }, - $crate::streamkit_core::types::PacketType::Custom { .. } => { - $crate::types::CPacketType::Custom - }, - $crate::streamkit_core::types::PacketType::Binary => { - $crate::types::CPacketType::Binary - }, - $crate::streamkit_core::types::PacketType::Any => { - $crate::types::CPacketType::Any - }, - $crate::streamkit_core::types::PacketType::Passthrough => { - $crate::types::CPacketType::Any - }, - }; - - let audio_format_ptr = if let Some(ref fmt) = audio_formats[idx] { - fmt as *const $crate::types::CAudioFormat - } else { - std::ptr::null() - }; - - let custom_type_id_ptr = if let Some(ref s) = custom_type_ids[idx] { - s.as_ptr() - } else { - std::ptr::null() - }; - - let video_format_ptr = if let Some(ref vf) = video_formats[idx] { - vf as *const $crate::types::CRawVideoFormat - } else { - std::ptr::null() - }; - - types_info.push($crate::types::CPacketTypeInfo { - type_discriminant, - audio_format: audio_format_ptr, - custom_type_id: custom_type_id_ptr, - raw_video_format: video_format_ptr, - }); - } - - c_inputs.push($crate::types::CInputPin { - name: name.as_ptr(), - accepts_types: types_info.as_ptr(), - accepts_types_count: types_info.len(), - }); - - input_names.push(name); - input_types.push(types_info); - input_audio_formats.push(audio_formats); - input_custom_type_ids.push(custom_type_ids); - input_video_formats.push(video_formats); - } - - // Convert outputs - let mut c_outputs = Vec::new(); - let mut output_names = Vec::new(); - let mut output_audio_formats = Vec::new(); - let mut output_custom_type_ids = Vec::new(); - let mut output_video_formats = Vec::new(); - - for output in &meta.outputs { - let name = std::ffi::CString::new(output.name.as_str()) - .expect("Output pin name should not contain null bytes"); - - let audio_format = match &output.produces_type { - $crate::streamkit_core::types::PacketType::RawAudio(af) => { - Some($crate::conversions::audio_format_to_c(af)) - }, - _ => None, - }; - output_audio_formats.push(audio_format); - let output_custom_type_id = match &output.produces_type { - $crate::streamkit_core::types::PacketType::Custom { type_id } => Some( - std::ffi::CString::new(type_id.as_str()) - .expect("Custom type_id should not contain null bytes"), - ), - $crate::streamkit_core::types::PacketType::EncodedAudio(format) => { - Some($crate::conversions::codec_name_to_cstring( - format.codec.as_c_name(), - )) - }, - $crate::streamkit_core::types::PacketType::EncodedVideo(format) => { - Some($crate::conversions::codec_name_to_cstring( - format.codec.as_c_name(), - )) - }, - _ => None, - }; - output_custom_type_ids.push(output_custom_type_id); - let video_format = match &output.produces_type { - $crate::streamkit_core::types::PacketType::RawVideo(vf) => { - Some($crate::conversions::raw_video_format_to_c(vf)) - }, - _ => None, - }; - output_video_formats.push(video_format); - - let type_discriminant = match &output.produces_type { - $crate::streamkit_core::types::PacketType::RawAudio(_) => { - $crate::types::CPacketType::RawAudio - }, - $crate::streamkit_core::types::PacketType::EncodedAudio(format) => { - if format.codec == $crate::streamkit_core::types::AudioCodec::Opus - && format.codec_private.is_none() - { - $crate::types::CPacketType::OpusAudio - } else { - $crate::types::CPacketType::EncodedAudio - } - }, - $crate::streamkit_core::types::PacketType::RawVideo(_) => { - $crate::types::CPacketType::RawVideo - }, - $crate::streamkit_core::types::PacketType::EncodedVideo(_) => { - $crate::types::CPacketType::EncodedVideo - }, - $crate::streamkit_core::types::PacketType::Text => { - $crate::types::CPacketType::Text - }, - $crate::streamkit_core::types::PacketType::Transcription => { - $crate::types::CPacketType::Transcription - }, - $crate::streamkit_core::types::PacketType::Custom { .. } => { - $crate::types::CPacketType::Custom - }, - $crate::streamkit_core::types::PacketType::Binary => { - $crate::types::CPacketType::Binary - }, - $crate::streamkit_core::types::PacketType::Any => { - $crate::types::CPacketType::Any - }, - $crate::streamkit_core::types::PacketType::Passthrough => { - $crate::types::CPacketType::Any - }, - }; - - // SAFETY: We just pushed an element, so last() is guaranteed to be Some - #[allow(clippy::unwrap_used)] - let audio_format_ptr = - if let Some(ref fmt) = output_audio_formats.last().unwrap() { - fmt as *const $crate::types::CAudioFormat - } else { - std::ptr::null() - }; - - // SAFETY: We just pushed an element, so last() is guaranteed to be Some - #[allow(clippy::unwrap_used)] - let custom_type_id_ptr = - if let Some(ref s) = output_custom_type_ids.last().unwrap() { - s.as_ptr() - } else { - std::ptr::null() - }; - - // SAFETY: We just pushed an element, so last() is guaranteed to be Some - #[allow(clippy::unwrap_used)] - let video_format_ptr = - if let Some(ref vf) = output_video_formats.last().unwrap() { - vf as *const $crate::types::CRawVideoFormat - } else { - std::ptr::null() - }; - - let type_info = $crate::types::CPacketTypeInfo { - type_discriminant, - audio_format: audio_format_ptr, - custom_type_id: custom_type_id_ptr, - raw_video_format: video_format_ptr, - }; - - c_outputs.push($crate::types::COutputPin { - name: name.as_ptr(), - produces_type: type_info, - }); - output_names.push(name); - } - - // Convert categories - let mut category_strings = Vec::new(); - let mut category_ptrs = Vec::new(); - - for cat in &meta.categories { - let c_str = std::ffi::CString::new(cat.as_str()) - .expect("Category name should not contain null bytes"); - category_ptrs.push(c_str.as_ptr()); - category_strings.push(c_str); - } - - let kind = std::ffi::CString::new(meta.kind.as_str()) - .expect("Node kind should not contain null bytes"); - let description = meta.description.as_ref().map(|d| { - std::ffi::CString::new(d.as_str()) - .expect("Description should not contain null bytes") - }); - let param_schema = std::ffi::CString::new(meta.param_schema.to_string()) - .expect("Param schema JSON should not contain null bytes"); - - let c_metadata = $crate::types::CNodeMetadata { - kind: kind.as_ptr(), - description: description.as_ref().map_or(std::ptr::null(), |d| d.as_ptr()), - inputs: c_inputs.as_ptr(), - inputs_count: c_inputs.len(), - outputs: c_outputs.as_ptr(), - outputs_count: c_outputs.len(), - param_schema: param_schema.as_ptr(), - categories: category_ptrs.as_ptr(), - categories_count: category_ptrs.len(), - }; - - ( - c_metadata, - c_inputs, - c_outputs, - input_names, - input_types, - input_audio_formats, - input_custom_type_ids, - input_video_formats, - output_names, - output_audio_formats, - output_custom_type_ids, - output_video_formats, - category_strings, - category_ptrs, - kind, - description, - param_schema, - ) + $crate::metadata_storage::PluginMetadataStorage::from_node_metadata(&meta) }); - - &metadata.0 - } + &storage.c_metadata + }) } // ── Instance lifecycle ────────────────────────────────────────── @@ -1720,25 +1129,32 @@ macro_rules! native_source_plugin_entry { log_callback: $crate::types::CLogCallback, log_user_data: *mut std::os::raw::c_void, ) -> $crate::types::CPluginHandle { - let params_json = if params.is_null() { - None - } else { - match unsafe { $crate::conversions::c_str_to_string(params) } { - Ok(s) if s.is_empty() => None, - Ok(s) => match serde_json::from_str(&s) { - Ok(v) => Some(v), + $crate::ffi_guard::guard_handle(|| { + let params_json = if params.is_null() { + None + } else { + match unsafe { $crate::conversions::c_str_to_string(params) } { + Ok(s) if s.is_empty() => None, + Ok(s) => match serde_json::from_str(&s) { + Ok(v) => Some(v), + Err(_) => return std::ptr::null_mut(), + }, Err(_) => return std::ptr::null_mut(), - }, - Err(_) => return std::ptr::null_mut(), - } - }; + } + }; - let logger = $crate::logger::Logger::new(log_callback, log_user_data, module_path!()); + // Create logger using the plugin's kind as target (e.g. "my_source") + // instead of module_path! which is opaque to the user. + let kind = <$plugin_type as $crate::NativeSourceNode>::metadata().kind; + let logger = $crate::logger::Logger::new(log_callback, log_user_data, &kind); - match <$plugin_type as $crate::NativeSourceNode>::new(params_json, logger) { - Ok(instance) => Box::into_raw(Box::new(instance)) as $crate::types::CPluginHandle, - Err(_) => std::ptr::null_mut(), - } + match <$plugin_type as $crate::NativeSourceNode>::new(params_json, logger) { + Ok(instance) => { + Box::into_raw(Box::new(instance)) as $crate::types::CPluginHandle + }, + Err(_) => std::ptr::null_mut(), + } + }) } // ── Source-specific entry points ───────────────────────────────── @@ -1746,47 +1162,51 @@ macro_rules! native_source_plugin_entry { extern "C" fn __plugin_get_source_config( handle: $crate::types::CPluginHandle, ) -> $crate::types::CSourceConfig { - if handle.is_null() { - return $crate::types::CSourceConfig { - is_source: false, - tick_interval_us: 0, - max_ticks: 0, - }; - } - let instance = unsafe { &*(handle as *const $plugin_type) }; - let cfg = instance.source_config(); - $crate::types::CSourceConfig { - is_source: true, - tick_interval_us: cfg.tick_interval_us, - max_ticks: cfg.max_ticks, - } + $crate::ffi_guard::guard_source_config(|| { + if handle.is_null() { + return $crate::types::CSourceConfig { + is_source: false, + tick_interval_us: 0, + max_ticks: 0, + }; + } + let instance = unsafe { &*(handle as *const $plugin_type) }; + let cfg = instance.source_config(); + $crate::types::CSourceConfig { + is_source: true, + tick_interval_us: cfg.tick_interval_us, + max_ticks: cfg.max_ticks, + } + }) } extern "C" fn __plugin_tick( handle: $crate::types::CPluginHandle, callbacks: *const $crate::types::CNodeCallbacks, ) -> $crate::types::CTickResult { - if handle.is_null() || callbacks.is_null() { - let err = $crate::conversions::error_to_c("Invalid handle or callbacks (null)"); - return $crate::types::CTickResult::error(err); - } - - let instance = unsafe { &mut *(handle as *mut $plugin_type) }; - let output = unsafe { $crate::OutputSender::from_node_callbacks(callbacks) }; - - match instance.tick(&output) { - Ok(done) => { - if done { - $crate::types::CTickResult::done() - } else { - $crate::types::CTickResult::ok() - } - }, - Err(e) => { - let err = $crate::conversions::error_to_c(e); - $crate::types::CTickResult::error(err) - }, - } + $crate::ffi_guard::guard_tick(|| { + if handle.is_null() || callbacks.is_null() { + let err = $crate::conversions::error_to_c("Invalid handle or callbacks (null)"); + return $crate::types::CTickResult::error(err); + } + + let instance = unsafe { &mut *(handle as *mut $plugin_type) }; + let output = unsafe { $crate::OutputSender::from_node_callbacks(callbacks) }; + + match instance.tick(&output) { + Ok(done) => { + if done { + $crate::types::CTickResult::done() + } else { + $crate::types::CTickResult::ok() + } + }, + Err(e) => { + let err = $crate::conversions::error_to_c(e); + $crate::types::CTickResult::error(err) + }, + } + }) } // ── No-op processor stubs (required by CNativePluginAPI) ──────── @@ -1797,17 +1217,19 @@ macro_rules! native_source_plugin_entry { _packet: *const $crate::types::CPacket, _callbacks: *const $crate::types::CNodeCallbacks, ) -> $crate::types::CResult { - let err = $crate::conversions::error_to_c( - "process_packet called on source plugin (not supported)", - ); - $crate::types::CResult::error(err) + $crate::ffi_guard::guard_result(|| { + let err = $crate::conversions::error_to_c( + "process_packet called on source plugin (not supported)", + ); + $crate::types::CResult::error(err) + }) } extern "C" fn __plugin_flush_noop( _handle: $crate::types::CPluginHandle, _callbacks: *const $crate::types::CNodeCallbacks, ) -> $crate::types::CResult { - $crate::types::CResult::success() + $crate::ffi_guard::guard_result(|| $crate::types::CResult::success()) } // ── Upstream hint delivery (v5) ───────────────────────────────── @@ -1816,27 +1238,33 @@ macro_rules! native_source_plugin_entry { handle: $crate::types::CPluginHandle, hint_json: *const std::os::raw::c_char, ) -> $crate::types::CResult { - if handle.is_null() { - let err = $crate::conversions::error_to_c("Invalid handle (null)"); - return $crate::types::CResult::error(err); - } - let hint_str = match unsafe { $crate::conversions::c_str_to_string(hint_json) } { - Ok(s) => s, - Err(e) => { - let err = $crate::conversions::error_to_c(format!("Invalid hint JSON: {e}")); + $crate::ffi_guard::guard_result(|| { + if handle.is_null() { + let err = $crate::conversions::error_to_c("Invalid handle (null)"); return $crate::types::CResult::error(err); - }, - }; - let hint: $crate::streamkit_core::UpstreamHint = match serde_json::from_str(&hint_str) { - Ok(h) => h, - Err(e) => { - let err = $crate::conversions::error_to_c(format!("Failed to parse hint: {e}")); - return $crate::types::CResult::error(err); - }, - }; - let instance = unsafe { &mut *(handle as *mut $plugin_type) }; - instance.on_upstream_hint(hint); - $crate::types::CResult::success() + } + let hint_str = match unsafe { $crate::conversions::c_str_to_string(hint_json) } { + Ok(s) => s, + Err(e) => { + let err = + $crate::conversions::error_to_c(format!("Invalid hint JSON: {e}")); + return $crate::types::CResult::error(err); + }, + }; + let hint: $crate::streamkit_core::UpstreamHint = + match serde_json::from_str(&hint_str) { + Ok(h) => h, + Err(e) => { + let err = $crate::conversions::error_to_c(format!( + "Failed to parse hint: {e}" + )); + return $crate::types::CResult::error(err); + }, + }; + let instance = unsafe { &mut *(handle as *mut $plugin_type) }; + instance.on_upstream_hint(hint); + $crate::types::CResult::success() + }) } // ── Shared ────────────────────────────────────────────────────── @@ -1845,42 +1273,45 @@ macro_rules! native_source_plugin_entry { handle: $crate::types::CPluginHandle, params: *const std::os::raw::c_char, ) -> $crate::types::CResult { - if handle.is_null() { - let err_msg = $crate::conversions::error_to_c("Invalid handle (null)"); - return $crate::types::CResult::error(err_msg); - } - - let instance = unsafe { &mut *(handle as *mut $plugin_type) }; - - let params_json = if params.is_null() { - None - } else { - match unsafe { $crate::conversions::c_str_to_string(params) } { - Ok(s) if s.is_empty() => None, - Ok(s) => match serde_json::from_str(&s) { - Ok(v) => Some(v), + $crate::ffi_guard::guard_result(|| { + if handle.is_null() { + let err_msg = $crate::conversions::error_to_c("Invalid handle (null)"); + return $crate::types::CResult::error(err_msg); + } + + let instance = unsafe { &mut *(handle as *mut $plugin_type) }; + + let params_json = if params.is_null() { + None + } else { + match unsafe { $crate::conversions::c_str_to_string(params) } { + Ok(s) if s.is_empty() => None, + Ok(s) => match serde_json::from_str(&s) { + Ok(v) => Some(v), + Err(e) => { + let err_msg = $crate::conversions::error_to_c(format!( + "Invalid params JSON: {e}" + )); + return $crate::types::CResult::error(err_msg); + }, + }, Err(e) => { let err_msg = $crate::conversions::error_to_c(format!( - "Invalid params JSON: {e}" + "Invalid params string: {e}" )); return $crate::types::CResult::error(err_msg); }, - }, + } + }; + + match instance.update_params(params_json) { + Ok(()) => $crate::types::CResult::success(), Err(e) => { - let err_msg = - $crate::conversions::error_to_c(format!("Invalid params string: {e}")); - return $crate::types::CResult::error(err_msg); + let err_msg = $crate::conversions::error_to_c(e); + $crate::types::CResult::error(err_msg) }, } - }; - - match instance.update_params(params_json) { - Ok(()) => $crate::types::CResult::success(), - Err(e) => { - let err_msg = $crate::conversions::error_to_c(e); - $crate::types::CResult::error(err_msg) - }, - } + }) } $crate::__plugin_shared_ffi!($plugin_type); diff --git a/sdks/plugin-sdk/native/src/logger.rs b/sdks/plugin-sdk/native/src/logger.rs index e307549e..3bf8c3d6 100644 --- a/sdks/plugin-sdk/native/src/logger.rs +++ b/sdks/plugin-sdk/native/src/logger.rs @@ -6,33 +6,79 @@ //! //! Provides a logger that sends log messages back to the host via callback. -use crate::types::{CLogCallback, CLogLevel}; +use crate::types::{CLogCallback, CLogEnabledCallback, CLogLevel}; use std::ffi::CString; use std::os::raw::c_void; +use std::sync::Arc; /// Logger for sending log messages to the host #[derive(Clone)] pub struct Logger { callback: CLogCallback, + enabled_callback: Option, user_data: *mut c_void, - target: String, + enabled_user_data: *mut c_void, + target: Arc, + /// Cached C-string of `target` for the enabled callback, avoiding + /// a `CString::new` allocation on every `enabled()` call. + /// `None` if the target contains a null byte (always returns + /// "enabled" in that edge case). + target_cstr: Option>, } -// SAFETY: The callback is a C function pointer which is thread-safe, -// and user_data is managed by the host which ensures thread-safety +// SAFETY: `callback` and `enabled_callback` are plain C function +// pointers (inherently Send+Sync). The two raw-pointer fields — +// `user_data` and `enabled_user_data` — are the sole reason this impl +// is manual. The host is responsible for ensuring the data they point +// to is safe to share across threads; current hosts always pass null +// for `enabled_user_data` and a thread-safe context for `user_data`. +// `Clone` shallow-copies both pointers — the host contract applies +// equally to the clone. unsafe impl Send for Logger {} unsafe impl Sync for Logger {} impl Logger { /// Create a new logger pub fn new(callback: CLogCallback, user_data: *mut c_void, target: &str) -> Self { - Self { callback, user_data, target: target.to_string() } + let target_cstr = CString::new(target).ok().map(Arc::new); + Self { + callback, + enabled_callback: None, + user_data, + enabled_user_data: std::ptr::null_mut(), + target: Arc::from(target), + target_cstr, + } + } + + /// Set the enabled-check callback (called by the host for v9 plugins). + /// + /// Uses a separate `user_data` pointer so that it does not interfere + /// with the log-output callback's `user_data`. + pub fn set_enabled_callback(&mut self, callback: CLogEnabledCallback, user_data: *mut c_void) { + self.enabled_callback = Some(callback); + self.enabled_user_data = user_data; + } + + /// Check whether the given log level is enabled. + /// + /// When no enabled callback is set (v8 host), always returns `true`. + pub fn enabled(&self, level: CLogLevel) -> bool { + match self.enabled_callback { + Some(cb) => { + let Some(ref cstr) = self.target_cstr else { + return true; + }; + cb(level, cstr.as_ptr(), self.enabled_user_data) + }, + None => true, + } } /// Log a message at the given level pub fn log(&self, level: CLogLevel, message: &str) { // Convert strings to C strings - let Ok(target_cstr) = CString::new(self.target.as_str()) else { + let Ok(target_cstr) = CString::new(self.target.as_ref()) else { return; // Silently ignore if target has null bytes }; @@ -137,34 +183,44 @@ macro_rules! plugin_log { #[macro_export] macro_rules! plugin_trace { ($logger:expr, $($arg:tt)*) => { - $logger.trace(&$crate::__format_fields!($($arg)*)) + if $logger.enabled($crate::types::CLogLevel::Trace) { + $logger.trace(&$crate::__format_fields!($($arg)*)) + } }; } #[macro_export] macro_rules! plugin_debug { ($logger:expr, $($arg:tt)*) => { - $logger.debug(&$crate::__format_fields!($($arg)*)) + if $logger.enabled($crate::types::CLogLevel::Debug) { + $logger.debug(&$crate::__format_fields!($($arg)*)) + } }; } #[macro_export] macro_rules! plugin_info { ($logger:expr, $($arg:tt)*) => { - $logger.info(&$crate::__format_fields!($($arg)*)) + if $logger.enabled($crate::types::CLogLevel::Info) { + $logger.info(&$crate::__format_fields!($($arg)*)) + } }; } #[macro_export] macro_rules! plugin_warn { ($logger:expr, $($arg:tt)*) => { - $logger.warn(&$crate::__format_fields!($($arg)*)) + if $logger.enabled($crate::types::CLogLevel::Warn) { + $logger.warn(&$crate::__format_fields!($($arg)*)) + } }; } #[macro_export] macro_rules! plugin_error { ($logger:expr, $($arg:tt)*) => { - $logger.error(&$crate::__format_fields!($($arg)*)) + if $logger.enabled($crate::types::CLogLevel::Error) { + $logger.error(&$crate::__format_fields!($($arg)*)) + } }; } diff --git a/sdks/plugin-sdk/native/src/metadata_storage.rs b/sdks/plugin-sdk/native/src/metadata_storage.rs new file mode 100644 index 00000000..ee404f2c --- /dev/null +++ b/sdks/plugin-sdk/native/src/metadata_storage.rs @@ -0,0 +1,395 @@ +// SPDX-FileCopyrightText: © 2025 StreamKit Contributors +// +// SPDX-License-Identifier: MPL-2.0 + +//! Internal backing storage for plugin metadata. +//! +//! [`PluginMetadataStorage`] owns all C-string and C-struct allocations +//! needed by the [`CNodeMetadata`] pointer returned from +//! `__plugin_get_metadata`. It lives in a `static OnceLock` inside the +//! plugin's dylib — initialized once, never moved. + +use std::ffi::CString; +use std::os::raw::c_char; + +use crate::conversions; +use crate::ffi_guard; +use crate::types::{ + CAudioFormat, CInputPin, CNodeMetadata, COutputPin, CPacketType, CPacketTypeInfo, + CRawVideoFormat, +}; +use crate::NodeMetadata; + +/// Owns all heap allocations backing a [`CNodeMetadata`]. +/// +/// Raw pointers inside `c_metadata`, `inputs`, and `outputs` point into +/// the sibling `Vec`/`CString` fields. Because the struct lives in a +/// `static OnceLock` and is never moved after initialization, the +/// pointers remain valid for the lifetime of the plugin. +pub struct PluginMetadataStorage { + pub c_metadata: CNodeMetadata, + pub inputs: Vec, + pub input_names: Vec, + pub input_types: Vec>, + pub input_audio_formats: Vec>>, + pub input_custom_type_ids: Vec>>, + pub input_video_formats: Vec>>, + pub outputs: Vec, + pub output_names: Vec, + pub output_audio_formats: Vec>, + pub output_custom_type_ids: Vec>, + pub output_video_formats: Vec>, + pub category_strings: Vec, + pub category_ptrs: Vec<*const c_char>, + pub kind: CString, + pub description: Option, + pub param_schema: CString, +} + +// SAFETY: All raw pointers in PluginMetadataStorage point to owned data +// within the same struct instance. The struct is stored in a static OnceLock +// and never moved after initialization. +#[allow(clippy::non_send_fields_in_send_ty)] +unsafe impl Send for PluginMetadataStorage {} +unsafe impl Sync for PluginMetadataStorage {} + +impl PluginMetadataStorage { + /// Build storage from a [`NodeMetadata`] value. + /// + /// This extracts the ~200 lines of metadata conversion that was + /// previously duplicated between `native_plugin_entry!` and + /// `native_source_plugin_entry!`. + /// + /// # Panics + /// + /// Cannot panic in practice — the `unwrap()` calls on `last()` are + /// reached only immediately after pushing an element to the same `Vec`. + pub fn from_node_metadata(meta: &NodeMetadata) -> Self { + // Invariant: the raw pointers stored in `c_inputs`/`c_outputs` + // are captured via `.as_ptr()` on Vec elements immediately after + // pushing. These pointers remain valid because: + // 1. Each Vec is only ever pushed to (never popped or cleared). + // 2. The Vecs are moved into the returned struct and never + // reallocated after that. + // 3. The struct lives in a `OnceLock` and is never moved again. + // A future refactor that calls `.to_vec()`, sorts, or otherwise + // reallocates a Vec would invalidate these pointers — use this + // comment as a guard rail. + // + // ── Convert inputs ────────────────────────────────────────── + let mut c_inputs = Vec::new(); + let mut input_names = Vec::new(); + let mut input_types = Vec::new(); + let mut input_audio_formats = Vec::new(); + let mut input_custom_type_ids = Vec::new(); + let mut input_video_formats = Vec::new(); + + for input in &meta.inputs { + let name = ffi_guard::cstring_lossy(input.name.as_str(), "input pin name"); + let mut types_info = Vec::new(); + let mut audio_formats = Vec::new(); + let mut custom_type_ids = Vec::new(); + let mut video_formats = Vec::new(); + + for pt in &input.accepts_types { + let audio_format = match pt { + streamkit_core::types::PacketType::RawAudio(af) => { + Some(conversions::audio_format_to_c(af)) + }, + _ => None, + }; + audio_formats.push(audio_format); + + let custom_type_id = match pt { + streamkit_core::types::PacketType::Custom { type_id } => { + Some(ffi_guard::cstring_lossy(type_id.as_str(), "custom type_id")) + }, + streamkit_core::types::PacketType::EncodedAudio(format) + if format.codec == streamkit_core::types::AudioCodec::Opus + && format.codec_private.is_none() => + { + None + }, + streamkit_core::types::PacketType::EncodedAudio(format) => { + Some(conversions::codec_name_to_cstring(format.codec.as_c_name())) + }, + streamkit_core::types::PacketType::EncodedVideo(format) => { + Some(conversions::codec_name_to_cstring(format.codec.as_c_name())) + }, + _ => None, + }; + custom_type_ids.push(custom_type_id); + + let video_format = match pt { + streamkit_core::types::PacketType::RawVideo(vf) => { + Some(conversions::raw_video_format_to_c(vf)) + }, + _ => None, + }; + video_formats.push(video_format); + } + + for (idx, pt) in input.accepts_types.iter().enumerate() { + let type_discriminant = packet_type_to_c_discriminant(pt); + + let audio_format_ptr = + audio_formats[idx].as_ref().map_or(std::ptr::null(), std::ptr::from_ref); + + let custom_type_id_ptr = + custom_type_ids[idx].as_ref().map_or(std::ptr::null(), |s| s.as_ptr()); + + let video_format_ptr = + video_formats[idx].as_ref().map_or(std::ptr::null(), std::ptr::from_ref); + + types_info.push(CPacketTypeInfo { + type_discriminant, + audio_format: audio_format_ptr, + custom_type_id: custom_type_id_ptr, + raw_video_format: video_format_ptr, + }); + } + + c_inputs.push(CInputPin { + name: name.as_ptr(), + accepts_types: types_info.as_ptr(), + accepts_types_count: types_info.len(), + }); + + input_names.push(name); + input_types.push(types_info); + input_audio_formats.push(audio_formats); + input_custom_type_ids.push(custom_type_ids); + input_video_formats.push(video_formats); + } + + // ── Convert outputs ───────────────────────────────────────── + let mut c_outputs = Vec::with_capacity(meta.outputs.len()); + let mut output_names = Vec::with_capacity(meta.outputs.len()); + let mut output_audio_formats: Vec> = + Vec::with_capacity(meta.outputs.len()); + let mut output_custom_type_ids: Vec> = + Vec::with_capacity(meta.outputs.len()); + let mut output_video_formats: Vec> = + Vec::with_capacity(meta.outputs.len()); + + for output in &meta.outputs { + output_names.push(ffi_guard::cstring_lossy(output.name.as_str(), "output pin name")); + output_audio_formats.push(match &output.produces_type { + streamkit_core::types::PacketType::RawAudio(af) => { + Some(conversions::audio_format_to_c(af)) + }, + _ => None, + }); + output_custom_type_ids.push(match &output.produces_type { + streamkit_core::types::PacketType::Custom { type_id } => { + Some(ffi_guard::cstring_lossy(type_id.as_str(), "output custom type_id")) + }, + streamkit_core::types::PacketType::EncodedAudio(format) + if format.codec == streamkit_core::types::AudioCodec::Opus + && format.codec_private.is_none() => + { + None + }, + streamkit_core::types::PacketType::EncodedAudio(format) => { + Some(conversions::codec_name_to_cstring(format.codec.as_c_name())) + }, + streamkit_core::types::PacketType::EncodedVideo(format) => { + Some(conversions::codec_name_to_cstring(format.codec.as_c_name())) + }, + _ => None, + }); + output_video_formats.push(match &output.produces_type { + streamkit_core::types::PacketType::RawVideo(vf) => { + Some(conversions::raw_video_format_to_c(vf)) + }, + _ => None, + }); + } + + for (idx, output) in meta.outputs.iter().enumerate() { + let type_discriminant = packet_type_to_c_discriminant(&output.produces_type); + + let audio_format_ptr = output_audio_formats + .get(idx) + .and_then(Option::as_ref) + .map_or(std::ptr::null(), std::ptr::from_ref); + + let custom_type_id_ptr = output_custom_type_ids + .get(idx) + .and_then(Option::as_ref) + .map_or(std::ptr::null(), |s| s.as_ptr()); + + let video_format_ptr = output_video_formats + .get(idx) + .and_then(Option::as_ref) + .map_or(std::ptr::null(), std::ptr::from_ref); + + let type_info = CPacketTypeInfo { + type_discriminant, + audio_format: audio_format_ptr, + custom_type_id: custom_type_id_ptr, + raw_video_format: video_format_ptr, + }; + + let name = output_names.get(idx).as_ref().map_or(std::ptr::null(), |s| s.as_ptr()); + + c_outputs.push(COutputPin { name, produces_type: type_info }); + } + + // ── Convert categories ────────────────────────────────────── + let mut category_strings = Vec::new(); + let mut category_ptrs = Vec::new(); + + for cat in &meta.categories { + let c_str = ffi_guard::cstring_lossy(cat.as_str(), "category name"); + category_ptrs.push(c_str.as_ptr()); + category_strings.push(c_str); + } + + // ── Scalar fields ─────────────────────────────────────────── + let kind = ffi_guard::cstring_lossy(meta.kind.as_str(), "node kind"); + let description = + meta.description.as_ref().map(|d| ffi_guard::cstring_lossy(d.as_str(), "description")); + let param_schema = + ffi_guard::cstring_lossy(&meta.param_schema.to_string(), "param schema JSON"); + + let c_metadata = CNodeMetadata { + kind: kind.as_ptr(), + description: description.as_ref().map_or(std::ptr::null(), |d| d.as_ptr()), + inputs: c_inputs.as_ptr(), + inputs_count: c_inputs.len(), + outputs: c_outputs.as_ptr(), + outputs_count: c_outputs.len(), + param_schema: param_schema.as_ptr(), + categories: category_ptrs.as_ptr(), + categories_count: category_ptrs.len(), + }; + + Self { + c_metadata, + inputs: c_inputs, + input_names, + input_types, + input_audio_formats, + input_custom_type_ids, + input_video_formats, + outputs: c_outputs, + output_names, + output_audio_formats, + output_custom_type_ids, + output_video_formats, + category_strings, + category_ptrs, + kind, + description, + param_schema, + } + } +} + +/// Map a Rust [`PacketType`] to the corresponding [`CPacketType`] discriminant. +fn packet_type_to_c_discriminant(pt: &streamkit_core::types::PacketType) -> CPacketType { + match pt { + streamkit_core::types::PacketType::RawAudio(_) => CPacketType::RawAudio, + streamkit_core::types::PacketType::EncodedAudio(format) => { + if format.codec == streamkit_core::types::AudioCodec::Opus + && format.codec_private.is_none() + { + CPacketType::OpusAudio + } else { + CPacketType::EncodedAudio + } + }, + streamkit_core::types::PacketType::RawVideo(_) => CPacketType::RawVideo, + streamkit_core::types::PacketType::EncodedVideo(_) => CPacketType::EncodedVideo, + streamkit_core::types::PacketType::Text => CPacketType::Text, + streamkit_core::types::PacketType::Transcription => CPacketType::Transcription, + streamkit_core::types::PacketType::Custom { .. } => CPacketType::Custom, + streamkit_core::types::PacketType::Binary => CPacketType::Binary, + streamkit_core::types::PacketType::Any | streamkit_core::types::PacketType::Passthrough => { + CPacketType::Any + }, + } +} + +#[cfg(test)] +mod tests { + use streamkit_core::types::{ + AudioCodec, AudioFormat, EncodedAudioFormat, PacketType, PixelFormat, RawVideoFormat, + SampleFormat, + }; + + use super::*; + + #[test] + fn output_format_pointers_reference_final_storage() { + let meta = NodeMetadata::builder("multi_output") + .output( + "audio", + PacketType::RawAudio(AudioFormat { + sample_rate: 48_000, + channels: 2, + sample_format: SampleFormat::F32, + }), + ) + .output( + "video", + PacketType::RawVideo(RawVideoFormat { + width: Some(1920), + height: Some(1080), + pixel_format: PixelFormat::Rgba8, + }), + ) + .build(); + + let storage = PluginMetadataStorage::from_node_metadata(&meta); + + let audio_ptr = storage.outputs[0].produces_type.audio_format; + let expected_audio_ptr = + storage.output_audio_formats[0].as_ref().map_or(std::ptr::null(), std::ptr::from_ref); + assert_eq!(audio_ptr, expected_audio_ptr); + + let video_ptr = storage.outputs[1].produces_type.raw_video_format; + let expected_video_ptr = + storage.output_video_formats[1].as_ref().map_or(std::ptr::null(), std::ptr::from_ref); + assert_eq!(video_ptr, expected_video_ptr); + + assert!(matches!( + conversions::packet_type_from_c(storage.outputs[0].produces_type), + Ok(PacketType::RawAudio(_)) + )); + assert!(matches!( + conversions::packet_type_from_c(storage.outputs[1].produces_type), + Ok(PacketType::RawVideo(_)) + )); + } + + #[test] + fn legacy_opus_metadata_does_not_set_custom_type_id() { + let meta = NodeMetadata::builder("opus") + .input( + "in", + &[PacketType::EncodedAudio(EncodedAudioFormat { + codec: AudioCodec::Opus, + codec_private: None, + })], + ) + .output( + "out", + PacketType::EncodedAudio(EncodedAudioFormat { + codec: AudioCodec::Opus, + codec_private: None, + }), + ) + .build(); + + let storage = PluginMetadataStorage::from_node_metadata(&meta); + + assert_eq!(storage.inputs[0].accepts_types_count, 1); + let input_type = unsafe { *storage.inputs[0].accepts_types }; + assert_eq!(input_type.type_discriminant, CPacketType::OpusAudio); + assert!(input_type.custom_type_id.is_null()); + assert_eq!(storage.outputs[0].produces_type.type_discriminant, CPacketType::OpusAudio); + assert!(storage.outputs[0].produces_type.custom_type_id.is_null()); + } +} diff --git a/sdks/plugin-sdk/native/src/resource_cache.rs b/sdks/plugin-sdk/native/src/resource_cache.rs new file mode 100644 index 00000000..cde3a18b --- /dev/null +++ b/sdks/plugin-sdk/native/src/resource_cache.rs @@ -0,0 +1,340 @@ +// SPDX-FileCopyrightText: © 2025 StreamKit Contributors +// +// SPDX-License-Identifier: MPL-2.0 + +//! Generic resource cache for native plugins. +//! +//! Replaces the hand-rolled `LazyLock>>>` pattern +//! used across all native plugins for caching expensive shared resources +//! (ML models, inference engines, etc.). +//! +//! # Relationship to `ResourceManager` +//! +//! [`crate::streamkit_core::ResourceManager`] provides server-side LRU +//! eviction and memory-budget accounting. `ResourceCache` is intentionally +//! simpler: it lives inside the plugin `.so` and owns resources for the +//! lifetime of the process — there is currently no bridge between the two. + +use std::collections::hash_map::Entry; +use std::collections::HashMap; +use std::fmt; +use std::hash::Hash; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, LazyLock, Mutex}; + +/// Errors returned by [`ResourceCache`] operations. +#[derive(Debug)] +pub enum CacheError { + /// The internal mutex is poisoned (a thread panicked while holding it). + Poisoned, + /// The user-supplied init closure failed. + Init(String), +} + +impl fmt::Display for CacheError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Poisoned => write!(f, "resource cache mutex poisoned"), + Self::Init(msg) => write!(f, "resource init failed: {msg}"), + } + } +} + +impl std::error::Error for CacheError {} + +/// Statistics for a resource cache instance. +/// +/// # Counter semantics +/// +/// * **`hits`** — `get_or_init` found the key in the map and returned +/// immediately without calling the init closure. +/// * **`misses`** — `get_or_init` did **not** find the key on the fast +/// path, called the init closure, and inserted the result (no race). +/// * **`init_races`** — the init closure ran, but another thread had +/// already inserted the same key by the time re-lock occurred. The +/// losing value is dropped. This counter lets dashboards spot +/// redundant resource loads under contention. +#[derive(Debug, Clone, Default)] +pub struct CacheStats { + /// Number of cache hits (returned existing value without calling init). + pub hits: u64, + /// Number of cache misses (called init and inserted the result). + pub misses: u64, + /// Number of init races (called init but another thread won insertion). + pub init_races: u64, + /// Current number of cached entries. + pub entries: usize, +} + +/// A thread-safe, lazily-initialized cache for expensive shared resources. +/// +/// Designed for blocking FFI contexts — uses `std::sync::Mutex` (not tokio). +/// The init closure runs **outside** the lock to avoid blocking other threads +/// during slow model loads. +/// +/// # Example +/// +/// ```ignore +/// use streamkit_plugin_sdk_native::resource_cache::ResourceCache; +/// use std::sync::Arc; +/// +/// static ENGINE_CACHE: ResourceCache = ResourceCache::new(); +/// +/// let engine: Arc = ENGINE_CACHE +/// .get_or_init("model-v2".to_string(), |key| { +/// MyEngine::load(key).map_err(|e| e.to_string()) +/// })?; +/// ``` +pub struct ResourceCache { + inner: LazyLock>>>, + hits: AtomicU64, + misses: AtomicU64, + init_races: AtomicU64, +} + +impl Default for ResourceCache { + fn default() -> Self { + Self::new() + } +} + +impl ResourceCache { + /// Creates a new, empty cache. + /// + /// This is `const` so it can be used in `static` declarations. + pub const fn new() -> Self { + Self { + inner: LazyLock::new(|| Mutex::new(HashMap::new())), + hits: AtomicU64::new(0), + misses: AtomicU64::new(0), + init_races: AtomicU64::new(0), + } + } + + /// Returns the number of cached entries. + /// + /// Returns `0` if the internal mutex is poisoned (a thread panicked + /// while holding the lock). Callers that need to distinguish + /// "empty" from "poisoned" should use [`stats`](Self::stats) or + /// inspect [`get_or_init`](Self::get_or_init) errors instead. + pub fn len(&self) -> usize { + self.inner.lock().map(|guard| guard.len()).unwrap_or(0) + } + + /// Returns `true` if the cache contains no entries. + /// + /// Returns `true` on a poisoned mutex (same as [`len`](Self::len) + /// returning `0`). Use [`is_poisoned`](Self::is_poisoned) first if + /// the distinction matters. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Returns `true` if the internal mutex has been poisoned. + /// + /// A poisoned mutex means a thread panicked while holding the lock. + /// After poisoning, [`len`](Self::len) and [`stats`](Self::stats) + /// return zero-valued results and [`get_or_init`](Self::get_or_init) + /// returns [`CacheError::Poisoned`]. + pub fn is_poisoned(&self) -> bool { + self.inner.is_poisoned() + } + + /// Removes all cached entries. + /// + /// Existing `Arc` clones held by node instances remain alive until those + /// clones are dropped. An init closure that started **before** this call + /// may also re-insert its key after `clear` returns (the insert happens + /// under a second lock acquisition). This is intended for tests and + /// best-effort cache maintenance, not as a synchronization primitive. + /// + /// No-ops silently if the mutex is poisoned. Use + /// [`is_poisoned`](Self::is_poisoned) to check beforehand. + pub fn clear(&self) { + if let Ok(mut guard) = self.inner.lock() { + guard.clear(); + } + } + + /// Returns current cache statistics. + /// + /// Counters are read with `Relaxed` ordering independently of one + /// another and of `entries` (which acquires the mutex), so the + /// returned snapshot is **not** globally consistent — e.g. `hits + + /// misses + init_races` may not equal the total number of + /// `get_or_init` calls observed so far. This is acceptable for + /// diagnostic dashboards; callers needing exact accounting should + /// synchronize externally. + pub fn stats(&self) -> CacheStats { + CacheStats { + hits: self.hits.load(Ordering::Relaxed), + misses: self.misses.load(Ordering::Relaxed), + init_races: self.init_races.load(Ordering::Relaxed), + entries: self.len(), + } + } +} + +impl ResourceCache { + /// Returns the cached value for `key`, or initializes it with `init`. + /// + /// The init closure runs **outside** the mutex lock so that slow + /// resource loads (model files, GPU init) do not block other threads. + /// If two threads race on the same key, both may call `init` but only + /// one value is stored; the other is dropped. See [`CacheStats`] for + /// the precise hit / miss / init-race counting semantics. + /// + /// # Errors + /// + /// Returns [`CacheError::Poisoned`] if the mutex is poisoned, or + /// [`CacheError::Init`] if `init` fails. + pub fn get_or_init(&self, key: K, init: F) -> Result, CacheError> + where + F: FnOnce(&K) -> Result, + { + // Fast path: check if key already exists. + { + let guard = self.inner.lock().map_err(|_| CacheError::Poisoned)?; + if let Some(value) = guard.get(&key) { + self.hits.fetch_add(1, Ordering::Relaxed); + return Ok(Arc::clone(value)); + } + } + // Lock dropped — run init outside the lock. + + let value = init(&key).map_err(CacheError::Init)?; + + // Re-lock and insert if still missing (another thread may have won). + let mut guard = self.inner.lock().map_err(|_| CacheError::Poisoned)?; + + let arc = match guard.entry(key) { + Entry::Occupied(e) => { + self.init_races.fetch_add(1, Ordering::Relaxed); + Arc::clone(e.get()) + }, + Entry::Vacant(e) => { + self.misses.fetch_add(1, Ordering::Relaxed); + let arc = Arc::new(value); + e.insert(Arc::clone(&arc)); + arc + }, + }; + drop(guard); + + Ok(arc) + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::needless_collect)] +mod tests { + use super::*; + + #[test] + fn get_or_init_caches_value() { + let cache: ResourceCache = ResourceCache::new(); + let v1 = cache.get_or_init("key".into(), |_| Ok("value".into())).unwrap(); + let v2 = cache.get_or_init("key".into(), |_| panic!("should not be called")).unwrap(); + assert!(Arc::ptr_eq(&v1, &v2)); + } + + #[test] + fn different_keys_different_values() { + let cache: ResourceCache = ResourceCache::new(); + let a = cache.get_or_init("a".into(), |_| Ok(1)).unwrap(); + let b = cache.get_or_init("b".into(), |_| Ok(2)).unwrap(); + assert_eq!(*a, 1); + assert_eq!(*b, 2); + assert!(!Arc::ptr_eq(&a, &b)); + } + + #[test] + fn init_error_not_cached() { + let cache: ResourceCache = ResourceCache::new(); + let err = cache.get_or_init("key".into(), |_| Err("fail".into())); + assert!(err.is_err()); + // Should retry on next call + let ok = cache.get_or_init("key".into(), |_| Ok("recovered".into())).unwrap(); + assert_eq!(&*ok, "recovered"); + } + + #[test] + fn clear_removes_entries() { + let cache: ResourceCache = ResourceCache::new(); + cache.get_or_init("a".into(), |_| Ok("x".into())).unwrap(); + assert_eq!(cache.len(), 1); + cache.clear(); + assert_eq!(cache.len(), 0); + } + + #[test] + fn stats_tracks_hits_and_misses() { + let cache: ResourceCache = ResourceCache::new(); + cache.get_or_init("a".into(), |_| Ok("x".into())).unwrap(); + cache.get_or_init("a".into(), |_| panic!("no")).unwrap(); + cache.get_or_init("b".into(), |_| Ok("y".into())).unwrap(); + let stats = cache.stats(); + assert_eq!(stats.misses, 2); + assert_eq!(stats.hits, 1); + assert_eq!(stats.init_races, 0); + assert_eq!(stats.entries, 2); + } + + #[test] + fn concurrent_access() { + use std::sync::Barrier; + use std::thread; + + let cache = Arc::new(ResourceCache::::new()); + let barrier = Arc::new(Barrier::new(10)); + let handles: Vec<_> = (0..10) + .map(|i| { + let b = barrier.clone(); + let c = cache.clone(); + thread::spawn(move || { + b.wait(); + c.get_or_init(0, |_| Ok(format!("from-thread-{i}"))).unwrap() + }) + }) + .collect(); + let results: Vec<_> = handles.into_iter().map(|h| h.join().unwrap()).collect(); + for r in &results { + assert!(Arc::ptr_eq(r, &results[0])); + } + } + + #[test] + fn init_races_counted() { + use std::sync::Barrier; + use std::thread; + + let cache = Arc::new(ResourceCache::::new()); + let thread_count = 10; + let start = Arc::new(Barrier::new(thread_count)); + let inside_init = Arc::new(Barrier::new(thread_count)); + let handles: Vec<_> = (0..thread_count) + .map(|i| { + let s = start.clone(); + let ii = inside_init.clone(); + let c = cache.clone(); + thread::spawn(move || { + s.wait(); + c.get_or_init(42, |_| { + ii.wait(); + Ok(format!("from-{i}")) + }) + .unwrap() + }) + }) + .collect(); + let results: Vec<_> = handles.into_iter().map(|h| h.join().unwrap()).collect(); + for r in &results { + assert!(Arc::ptr_eq(r, &results[0])); + } + let stats = cache.stats(); + assert_eq!(stats.entries, 1); + assert_eq!(stats.misses, 1); + assert!(stats.init_races >= 1, "expected at least 1 init race, got {}", stats.init_races); + assert_eq!(stats.misses + stats.init_races, thread_count as u64); + } +} diff --git a/sdks/plugin-sdk/native/src/types.rs b/sdks/plugin-sdk/native/src/types.rs index f1303d5c..799b48d7 100644 --- a/sdks/plugin-sdk/native/src/types.rs +++ b/sdks/plugin-sdk/native/src/types.rs @@ -32,7 +32,24 @@ use std::os::raw::{c_char, c_void}; /// with MoQ transport nodes. The codec name is carried as a /// null-terminated string via the `custom_type_id` pointer in /// [`CPacketTypeInfo`]. -pub const NATIVE_PLUGIN_API_VERSION: u32 = 8; +/// v9: Zero-copy binary packets (`CBinaryPacket::buffer_handle` + `free_fn`) +/// and logger overhaul (`CLogEnabledCallback`, `set_log_enabled_callback`, +/// logger target set to plugin kind instead of `module_path!()`). +/// **Wire change:** For v9 hosts, plain `Binary` packets are upgraded to +/// `BinaryWithMeta` (with null `content_type` / `metadata`) on the wire +/// to attach the zero-copy buffer handle. C plugins that `switch` on +/// `packet_type` must handle `BinaryWithMeta` even when those fields +/// are null. +/// **ABI note:** `CBinaryPacket` grew by 16 bytes (buffer_handle + +/// free_fn). v8 plugins compiled against the old 40-byte layout that +/// validate `len == sizeof(CBinaryPacket)` will reject BinaryWithMeta +/// packets from a v9 host. Plugins using `len >= sizeof(…)` or bare +/// pointer casts are unaffected. +/// **Logger target change:** The logger target moved from +/// `module_path!()` (e.g. `whisper_plugin::inner`) to +/// `metadata().kind` (e.g. `whisper`). Existing `RUST_LOG` directives +/// that filter on the old module path will need updating. +pub const NATIVE_PLUGIN_API_VERSION: u32 = 9; /// Opaque handle to a plugin instance pub type CPluginHandle = *mut c_void; @@ -56,6 +73,18 @@ pub enum CLogLevel { /// - user_data: Opaque pointer passed by host pub type CLogCallback = extern "C" fn(CLogLevel, *const c_char, *const c_char, *mut c_void); +/// Callback to check whether a given log level is enabled for a target. +/// +/// Parameters: (level, target, user_data) -> bool +/// +/// The host implements this by consulting the tracing subscriber. If +/// this returns `false`, the plugin can skip formatting the log message +/// entirely. +pub type CLogEnabledCallback = extern "C" fn(CLogLevel, *const c_char, *mut c_void) -> bool; + +/// Function exported by v9 plugins to install the host's log-enabled callback. +pub type CSetLogEnabledCallback = extern "C" fn(CPluginHandle, CLogEnabledCallback, *mut c_void); + /// Result type for C ABI functions #[repr(C)] #[derive(Debug, Copy, Clone)] @@ -65,8 +94,9 @@ pub struct CResult { /// /// # Ownership /// - /// This pointer is **borrowed** and must not be freed by the caller. - /// Callers should copy it immediately if they need to keep it. + /// In both plugin→host and host→plugin directions, this pointer is + /// **borrowed** and must not be freed by the caller. Callers should copy + /// it immediately if they need to keep it. pub error_message: *const c_char, } @@ -306,6 +336,11 @@ pub struct CAudioFrame { /// `packet_type == BinaryWithMeta`. Unlike the plain `Binary` variant this /// preserves MIME content-type (e.g. `"audio/aac"`) and timing metadata /// across the plugin ↔ host boundary. +/// +/// When `buffer_handle` is non-null (v9 host), the plugin can reclaim the +/// original `bytes::Bytes` via `Box::from_raw` for zero-copy transfer. +/// When null (v8 host or legacy), the plugin falls back to +/// `Bytes::copy_from_slice`. #[repr(C)] pub struct CBinaryPacket { pub data: *const u8, @@ -314,6 +349,15 @@ pub struct CBinaryPacket { pub content_type: *const c_char, /// Nullable. Per-packet timing metadata. pub metadata: *const CPacketMetadata, + /// Opaque handle to a `Box` for zero-copy transfer. + /// NULL for v8 hosts or when the packet was not allocated from a + /// `Bytes` buffer. The plugin reclaims the `Bytes` via + /// `Box::from_raw(handle.cast::())`. + pub buffer_handle: *mut c_void, + /// Releases the `buffer_handle` without using the buffer (e.g. on + /// error paths where `packet_from_c` is never called). NULL when + /// `buffer_handle` is NULL. + pub free_fn: Option, } /// Generic packet container @@ -419,8 +463,12 @@ impl CTickResult { /// The main plugin API structure. /// /// Plugins export a function that returns a pointer to this struct. -/// Fields added in v3 (`get_source_config`, `tick`) are `Option` for -/// backward compatibility; processor plugins set them to `None`. +/// Fields added after the required v6 layout are `Option` for backward +/// compatibility; processor plugins set source-only functions to `None`. +/// +/// v9 extensions that would grow this struct live behind separate exported +/// symbols so a v9 host can safely load v6–v8 plugins compiled with the +/// smaller layout. #[repr(C)] pub struct CNativePluginAPI { /// API version for compatibility checking. @@ -605,3 +653,23 @@ pub struct CNodeCallbacks { /// Symbol name that plugins must export pub const PLUGIN_API_SYMBOL: &[u8] = b"streamkit_native_plugin_api\0"; + +/// Optional v9 symbol for installing the log-enabled callback. +pub const PLUGIN_SET_LOG_ENABLED_SYMBOL: &[u8] = + b"streamkit_native_plugin_set_log_enabled_callback\0"; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn native_plugin_api_layout_stays_v8_sized_for_old_plugins() { + let pointer_size = std::mem::size_of::<*const ()>(); + let version_with_padding = pointer_size; + let function_fields = 10; + assert_eq!( + std::mem::size_of::(), + version_with_padding + function_fields * pointer_size + ); + } +}