diff --git a/src/lib-cpp/include/Velopack.hpp b/src/lib-cpp/include/Velopack.hpp index 16a65941..114044cd 100644 --- a/src/lib-cpp/include/Velopack.hpp +++ b/src/lib-cpp/include/Velopack.hpp @@ -362,7 +362,7 @@ private: IUpdateSource(vpkc_update_source_t* pSource) : m_pSource(pSource) {} vpkc_update_source_t* m_pSource = 0; public: - ~IUpdateSource() { + virtual ~IUpdateSource() { vpkc_free_source(m_pSource); } IUpdateSource() { @@ -447,7 +447,8 @@ public: * @param options Optional extra configuration for update manager. * @param locator Override the default locator configuration (usually used for testing / mocks). */ - UpdateManager(std::unique_ptr pUpdateSource, const UpdateOptions* options = nullptr, const VelopackLocatorConfig* locator = nullptr) { + template >> + UpdateManager(std::unique_ptr pUpdateSource, const UpdateOptions* options = nullptr, const VelopackLocatorConfig* locator = nullptr) { vpkc_update_options_t vpkc_options; vpkc_update_options_t* pOptions = nullptr; if (options != nullptr) { @@ -462,7 +463,7 @@ public: pLocator = &vpkc_locator; } - m_pUpdateSource.swap(pUpdateSource); + m_pUpdateSource = std::unique_ptr(static_cast(pUpdateSource.release())); vpkc_update_source_t* pSource = m_pUpdateSource->m_pSource; if (!vpkc_new_update_manager_with_source(pSource, pOptions, pLocator, &m_pManager)) { throw_last_error(); diff --git a/src/lib-cpp/src/csource.rs b/src/lib-cpp/src/csource.rs index a9b06240..c03c87fe 100644 --- a/src/lib-cpp/src/csource.rs +++ b/src/lib-cpp/src/csource.rs @@ -14,7 +14,7 @@ use velopack::{bundle::Manifest, sources::UpdateSource, Error, VelopackAsset, Ve lazy_static! { static ref PROGRESS_CALLBACKS: RwLock>> = RwLock::new(HashMap::new()); - static ref PROGRESS_ID: AtomicUsize = AtomicUsize::new(0); + static ref PROGRESS_ID: AtomicUsize = AtomicUsize::new(1); } pub fn report_csource_progress(callback_id: size_t, progress: i16) { @@ -39,38 +39,53 @@ impl UpdateSource for CCallbackUpdateSource { fn get_release_feed(&self, channel: &str, _: &Manifest) -> Result { let releases_name = format!("releases.{}.json", channel); let releases_name_cstr = CString::new(releases_name).unwrap(); - let json_cstr_ptr = (self.cb_get_release_feed)(self.p_user_data, releases_name_cstr.as_ptr()); - let json = c_to_string_opt(json_cstr_ptr) - .ok_or(Error::Generic("User vpkc_release_feed_delegate_t returned a null pointer instead of an asset feed".to_string()))?; - (self.cb_free_release_feed)(self.p_user_data, json_cstr_ptr); // Free the C string returned by the callback - let feed: VelopackAssetFeed = serde_json::from_str(&json)?; - Ok(feed) + + if let Some(cb_get_release_feed) = self.cb_get_release_feed { + let json_cstr_ptr = (cb_get_release_feed)(self.p_user_data, releases_name_cstr.as_ptr()); + let json = c_to_string_opt(json_cstr_ptr) + .ok_or(Error::Generic("User vpkc_release_feed_delegate_t returned a null pointer instead of an asset feed".to_string()))?; + + if let Some(cb_free_release_feed) = self.cb_free_release_feed { + (cb_free_release_feed)(self.p_user_data, json_cstr_ptr); // Free the C string returned by the callback + } else { + log::error!("User vpkc_release_feed_delegate_t is null, this may be a memory leak"); + } + let feed: VelopackAssetFeed = serde_json::from_str(&json)?; + Ok(feed) + } else { + Err(Error::Generic("User vpkc_release_feed_delegate_t is null".to_string())) + } } fn download_release_entry(&self, asset: &VelopackAsset, local_file: &str, progress_sender: Option>) -> Result<(), Error> { - let local_file_cstr = CString::new(local_file).unwrap(); - let asset_ptr: *mut vpkc_asset_t = std::ptr::null_mut(); - unsafe { allocate_velopackasset(asset.clone(), asset_ptr) }; + if let Some(cb_download_release_entry) = self.cb_download_release_entry { + let local_file_cstr = CString::new(local_file).unwrap(); + let mut asset_c: vpkc_asset_t = unsafe { std::mem::zeroed() }; + let asset_ptr: *mut vpkc_asset_t = &mut asset_c as *mut vpkc_asset_t; + unsafe { allocate_velopackasset(asset.clone(), asset_ptr) }; - let progress_callback_id = PROGRESS_ID.fetch_add(1, Ordering::SeqCst); - if let Some(progress_sender) = &progress_sender { - let _ = progress_sender.send(0); - PROGRESS_CALLBACKS.write().unwrap().insert(progress_callback_id, progress_sender.clone()); + let progress_callback_id = PROGRESS_ID.fetch_add(1, Ordering::SeqCst); + if let Some(progress_sender) = &progress_sender { + let _ = progress_sender.send(0); + PROGRESS_CALLBACKS.write().unwrap().insert(progress_callback_id, progress_sender.clone()); + } + + let success = (cb_download_release_entry)(self.p_user_data, asset_ptr, local_file_cstr.as_ptr(), progress_callback_id); + + unsafe { free_velopackasset(asset_ptr) }; + + if let Some(sender) = PROGRESS_CALLBACKS.write().unwrap().remove(&progress_callback_id) { + let _ = sender.send(100); + } + + if !success { + return Err(Error::Generic("User vpkc_download_asset_delegate_t returned false to indicate download failed".to_owned())); + } + + Ok(()) + } else { + Err(Error::Generic("User vpkc_download_asset_delegate_t is null".to_string())) } - - let success = (self.cb_download_release_entry)(self.p_user_data, asset_ptr, local_file_cstr.as_ptr(), progress_callback_id); - - unsafe { free_velopackasset(asset_ptr) }; - - if let Some(sender) = PROGRESS_CALLBACKS.write().unwrap().remove(&progress_callback_id) { - let _ = sender.send(100); - } - - if !success { - return Err(Error::Generic("User vpkc_download_asset_delegate_t returned false to indicate download failed".to_owned())); - } - - Ok(()) } fn clone_boxed(&self) -> Box { diff --git a/src/lib-cpp/src/lib.rs b/src/lib-cpp/src/lib.rs index 55242872..bfee8d0a 100644 --- a/src/lib-cpp/src/lib.rs +++ b/src/lib-cpp/src/lib.rs @@ -48,18 +48,15 @@ pub extern "C" fn vpkc_new_source_custom_callback( cb_download_entry: vpkc_download_asset_delegate_t, p_user_data: *mut c_void, ) -> *mut vpkc_update_source_t { - let cb_release_feed = cb_release_feed.to_option(); - let cb_download_entry = cb_download_entry.to_option(); - let cb_free_release_feed = cb_free_release_feed.to_option(); if cb_release_feed.is_none() || cb_download_entry.is_none() || cb_free_release_feed.is_none() { return ptr::null_mut(); } let source = CCallbackUpdateSource { p_user_data, - cb_get_release_feed: cb_release_feed.unwrap(), - cb_download_release_entry: cb_download_entry.unwrap(), - cb_free_release_feed: cb_free_release_feed.unwrap(), + cb_get_release_feed: cb_release_feed, + cb_download_release_entry: cb_download_entry, + cb_free_release_feed: cb_free_release_feed, }; UpdateSourceRawPtr::new(Box::new(source)) @@ -216,7 +213,6 @@ pub extern "C" fn vpkc_download_updates( None => bail!("pManager must not be null"), }; - let cb_progress = cb_progress.to_option(); let update = c_to_updateinfo_opt(p_update).ok_or(anyhow!("pUpdate must not be null"))?; if let Some(cb_progress) = cb_progress { @@ -407,7 +403,6 @@ pub extern "C" fn vpkc_app_set_locator(p_locator: *mut vpkc_locator_config_t) { /// Only supported on windows; On other operating systems, this will never be called. #[no_mangle] pub extern "C" fn vpkc_app_set_hook_after_install(cb_after_install: vpkc_hook_callback_t) { - let cb_after_install = cb_after_install.to_option(); update_app_options(|opt| { opt.install_hook = cb_after_install; }); @@ -419,7 +414,6 @@ pub extern "C" fn vpkc_app_set_hook_after_install(cb_after_install: vpkc_hook_ca /// Only supported on windows; On other operating systems, this will never be called. #[no_mangle] pub extern "C" fn vpkc_app_set_hook_before_uninstall(cb_before_uninstall: vpkc_hook_callback_t) { - let cb_before_uninstall = cb_before_uninstall.to_option(); update_app_options(|opt| { opt.uninstall_hook = cb_before_uninstall; }); @@ -431,7 +425,6 @@ pub extern "C" fn vpkc_app_set_hook_before_uninstall(cb_before_uninstall: vpkc_h /// Only supported on windows; On other operating systems, this will never be called. #[no_mangle] pub extern "C" fn vpkc_app_set_hook_before_update(cb_before_update: vpkc_hook_callback_t) { - let cb_before_update = cb_before_update.to_option(); update_app_options(|opt| { opt.obsolete_hook = cb_before_update; }); @@ -443,7 +436,6 @@ pub extern "C" fn vpkc_app_set_hook_before_update(cb_before_update: vpkc_hook_ca /// Only supported on windows; On other operating systems, this will never be called. #[no_mangle] pub extern "C" fn vpkc_app_set_hook_after_update(cb_after_update: vpkc_hook_callback_t) { - let cb_after_update = cb_after_update.to_option(); update_app_options(|opt| { opt.update_hook = cb_after_update; }); @@ -452,7 +444,6 @@ pub extern "C" fn vpkc_app_set_hook_after_update(cb_after_update: vpkc_hook_call /// This hook is triggered when the application is started for the first time after installation. #[no_mangle] pub extern "C" fn vpkc_app_set_hook_first_run(cb_first_run: vpkc_hook_callback_t) { - let cb_first_run = cb_first_run.to_option(); update_app_options(|opt| { opt.firstrun_hook = cb_first_run; }); @@ -461,7 +452,6 @@ pub extern "C" fn vpkc_app_set_hook_first_run(cb_first_run: vpkc_hook_callback_t /// This hook is triggered when the application is restarted by Velopack after installing updates. #[no_mangle] pub extern "C" fn vpkc_app_set_hook_restarted(cb_restarted: vpkc_hook_callback_t) { - let cb_restarted = cb_restarted.to_option(); update_app_options(|opt| { opt.restarted_hook = cb_restarted; }); @@ -477,10 +467,5 @@ pub extern "C" fn vpkc_get_last_error(psz_error: *mut c_char, c_error: size_t) - /// Set a custom log callback. This will be called for all log messages generated by the Velopack library. #[no_mangle] pub extern "C" fn vpkc_set_logger(cb_log: vpkc_log_callback_t, p_user_data: *mut c_void) { - let cb_log = cb_log.to_option(); - if let Some(cb_log) = cb_log { - set_log_callback(cb_log, p_user_data); - } else { - clear_log_callback(); - } + set_log_callback(cb_log, p_user_data); } diff --git a/src/lib-cpp/src/raw.rs b/src/lib-cpp/src/raw.rs index f9ae4e74..55a68922 100644 --- a/src/lib-cpp/src/raw.rs +++ b/src/lib-cpp/src/raw.rs @@ -1,46 +1,6 @@ use crate::types::*; use velopack::{sources::UpdateSource, UpdateManager}; -pub trait CallbackExt: Sized { - fn to_option(self) -> Option; -} - -impl CallbackExt for vpkc_progress_callback_t { - fn to_option(self) -> Option { - unsafe { std::mem::transmute::>(self) } - } -} - -impl CallbackExt for vpkc_log_callback_t { - fn to_option(self) -> Option { - unsafe { std::mem::transmute::>(self) } - } -} - -impl CallbackExt for vpkc_hook_callback_t { - fn to_option(self) -> Option { - unsafe { std::mem::transmute::>(self) } - } -} - -impl CallbackExt for vpkc_release_feed_delegate_t { - fn to_option(self) -> Option { - unsafe { std::mem::transmute::>(self) } - } -} - -impl CallbackExt for vpkc_download_asset_delegate_t { - fn to_option(self) -> Option { - unsafe { std::mem::transmute::>(self) } - } -} - -impl CallbackExt for vpkc_free_release_feed_t { - fn to_option(self) -> Option { - unsafe { std::mem::transmute::>(self) } - } -} - pub trait RawPtrExt<'a, T>: Sized { fn to_opaque_ref(self) -> Option<&'a T>; } diff --git a/src/lib-cpp/src/statics.rs b/src/lib-cpp/src/statics.rs index eb4b161b..4cba48ce 100644 --- a/src/lib-cpp/src/statics.rs +++ b/src/lib-cpp/src/statics.rs @@ -8,12 +8,12 @@ use crate::types::*; #[derive(Debug, Default, Clone)] pub struct AppOptions { - pub install_hook: Option, - pub update_hook: Option, - pub obsolete_hook: Option, - pub uninstall_hook: Option, - pub firstrun_hook: Option, - pub restarted_hook: Option, + pub install_hook: vpkc_hook_callback_t, + pub update_hook: vpkc_hook_callback_t, + pub obsolete_hook: vpkc_hook_callback_t, + pub uninstall_hook: vpkc_hook_callback_t, + pub firstrun_hook: vpkc_hook_callback_t, + pub restarted_hook: vpkc_hook_callback_t, pub auto_apply: Option, pub args: Option>, pub locator: Option, @@ -21,7 +21,7 @@ pub struct AppOptions { lazy_static::lazy_static! { static ref LAST_ERROR: RwLock = RwLock::new(String::new()); - static ref LOG_CALLBACK: Mutex> = Mutex::new(None); + static ref LOG_CALLBACK: Mutex<(vpkc_log_callback_t, usize)> = Mutex::new((None, 0)); pub static ref VELOPACK_APP: RwLock = RwLock::new(Default::default()); } @@ -71,17 +71,13 @@ pub fn set_log_callback(callback: vpkc_log_callback_t, user_data: *mut c_void) { log::set_max_level(log::LevelFilter::Trace); let mut log_callback = LOG_CALLBACK.lock().unwrap(); - *log_callback = Some((callback, user_data as usize)); -} - -pub fn clear_log_callback() { - let mut log_callback = LOG_CALLBACK.lock().unwrap(); - *log_callback = None; + *log_callback = (callback, user_data as usize); } pub fn log_message(level: &str, message: &str) { let log_callback = LOG_CALLBACK.lock().unwrap(); - if let Some((callback, user_data)) = *log_callback { + let (callback, user_data) = *log_callback; + if let Some(callback) = callback { let c_level = CString::new(level).unwrap(); let c_message = CString::new(message).unwrap(); callback(user_data as *mut c_void, c_level.as_ptr(), c_message.as_ptr()); diff --git a/src/lib-cpp/src/types.rs b/src/lib-cpp/src/types.rs index ffdfe207..464c449f 100644 --- a/src/lib-cpp/src/types.rs +++ b/src/lib-cpp/src/types.rs @@ -19,30 +19,32 @@ pub type vpkc_update_manager_t = c_void; pub type vpkc_update_source_t = c_void; /// Progress callback function. -pub type vpkc_progress_callback_t = extern "C" fn(p_user_data: *mut c_void, progress: size_t); +pub type vpkc_progress_callback_t = Option; /// Log callback function. -pub type vpkc_log_callback_t = extern "C" fn(p_user_data: *mut c_void, psz_level: *const c_char, psz_message: *const c_char); +pub type vpkc_log_callback_t = Option; /// VelopackApp startup hook callback function. -pub type vpkc_hook_callback_t = extern "C" fn(p_user_data: *mut c_void, psz_app_version: *const c_char); +pub type vpkc_hook_callback_t = Option; /// User delegate for to fetch a release feed. This function should return the raw JSON string of the release.json feed. -pub type vpkc_release_feed_delegate_t = extern "C" fn(p_user_data: *mut c_void, psz_releases_name: *const c_char) -> *mut c_char; +pub type vpkc_release_feed_delegate_t = Option *mut c_char>; /// User delegate for freeing a release feed. This function should free the feed string returned by `vpkc_release_feed_delegate_t`. -pub type vpkc_free_release_feed_t = extern "C" fn(p_user_data: *mut c_void, psz_feed: *mut c_char); +pub type vpkc_free_release_feed_t = Option; /// User delegate for downloading an asset file. This function is expected to download the provided asset /// to the provided local file path. Througout, you can use the progress callback to write progress reports. /// The function should return true if the download was successful, false otherwise. /// Progress -pub type vpkc_download_asset_delegate_t = extern "C" fn( - p_user_data: *mut c_void, - p_asset: *const vpkc_asset_t, - psz_local_path: *const c_char, - progress_callback_id: size_t, -) -> bool; +pub type vpkc_download_asset_delegate_t = Option< + extern "C" fn( + p_user_data: *mut c_void, + p_asset: *const vpkc_asset_t, + psz_local_path: *const c_char, + progress_callback_id: size_t, + ) -> bool, +>; pub fn c_to_string_opt(psz: *const c_char) -> Option { if psz.is_null() {