Fix crash when performing rebuild via FFI

twackup-gui/Twackup/Sources/FFI Models/Dpkg.swift

@@ -90,17 +90,15 @@ actor Dpkg {
         }
         defer { buildParameters.out_dir.deallocate() }
 
-        let status = outDir.path.utf8CString.withUnsafeBufferPointer { pointer in
-            // safe to unwrap?
-            buildParameters.out_dir = pointer.baseAddress!
-
-            return packages.map { $0.pkg }.withUnsafeBufferPointer { pointer in
+        let status =  packages
+            .map { $0.inner }
+            .map { Optional($0) } // Apple moment
+            .withUnsafeBufferPointer { pointer in
                 // safe to unwrap?
-                buildParameters.packages = slice_ref_TwPackage_t(ptr: pointer.baseAddress!, len: pointer.count)
+                buildParameters.packages = slice_ref_TwPackageRef_t(ptr: pointer.baseAddress!, len: pointer.count)
 
                 return tw_rebuild_packages(innerDpkg, buildParameters)
             }
-        }
 
         if status != TW_RESULT_OK.clampedToU8 {
             tw_free_rebuild_results(ffiResults)
@@ -125,7 +123,7 @@ actor Dpkg {
 
         tw_free_rebuild_results(ffiResults)
 
-        return results
+        return []
     }
 
     private func createProgressFuncs() -> TwProgressFunctions {
@@ -134,21 +132,26 @@ actor Dpkg {
         // not a memory leak actually. It lives as long as self does
         funcs.context = Unmanaged<Dpkg>.passUnretained(self).toOpaque()
         funcs.started_processing = { context, package in
-            // Package is a stack pointer so it doesn't need to be released
-            guard let context, let package, let ffiPackage = FFIPackage(package.pointee) else { return }
+            guard let context,
+                  let ffiPackage = FFIPackage(package)
+            else {
+                tw_package_release(package.inner)
+                return
+            }
 
             let dpkg = Unmanaged<Dpkg>.fromOpaque(context).takeUnretainedValue()
             Task(priority: .utility) {
                 await dpkg.buildProgressDelegate?.startProcessing(package: ffiPackage)
             }
         }
         funcs.finished_processing = { context, package, debPath in
-            // Package is a stack pointer so it doesn't need to be released
             guard let context,
-                  let package,
-                  let ffiPackage = FFIPackage(package.pointee),
+                  let ffiPackage = FFIPackage(package),
                   let debPath = String(ffiSlice: debPath)
-            else { return }
+            else {
+                tw_package_release(package.inner)
+                return
+            }
 
             let dpkg = Unmanaged<Dpkg>.fromOpaque(context).takeUnretainedValue()
             Task(priority: .utility) {

twackup-gui/Twackup/Sources/FFI Models/Package/FFIPackage.swift

@@ -6,7 +6,7 @@
 //
 
 final class FFIPackage: Package, Sendable {
-    let pkg: TwPackage_t
+    private let pkg: TwPackage_t
 
     let id: String
 
@@ -15,7 +15,11 @@ final class FFIPackage: Package, Sendable {
     let version: String
 
     var section: PackageSection {
-        return pkg.section.swiftSection
+        pkg.section.swiftSection
+    }
+
+    var inner: TwPackageRef_t {
+        pkg.inner
     }
 
     var icon: URL? {

twackup/assets/ffi_test.cpp

@@ -2,20 +2,24 @@
 #include <iostream>
 #include <cassert>
 
-static void started_processing(void *context, TwPackage_t const *package) {
+static void started_processing(void *context, TwPackage_t package) {
   std::cout
   << "started_processing(\""
-  << std::string((char *)package->identifier.ptr, package->identifier.len)
+  << std::string((char *)package.identifier.ptr, package.identifier.len)
   << "\")"
   << std::endl;
+
+  tw_package_release(package.inner);
 }
 
-static void finished_processing(void *context, TwPackage_t const *package, slice_raw_uint8_t deb_path) {
+static void finished_processing(void *context, TwPackage_t package, slice_raw_uint8_t deb_path) {
   std::cout
   << "finished_processing(\""
-  << std::string((char *)package->identifier.ptr, package->identifier.len)
+  << std::string((char *)package.identifier.ptr, package.identifier.len)
   << "\")"
   << std::endl;
+
+  tw_package_release(package.inner);
 }
 
 static void finished_all(void *context) {
@@ -31,15 +35,16 @@ int main(int argc, char *argv[]) {
 
   auto dpkg = tw_init(argv[1], false);
 
-  TwPackage_t *packages = NULL;
-  size_t count = tw_get_packages(dpkg, false, TW_PACKAGES_SORT_UNSORTED, &packages);
+  TwPackage_t *packagesPtr = NULL;
+  size_t count = tw_get_packages(dpkg, false, TW_PACKAGES_SORT_UNSORTED, &packagesPtr);
+
   assert(count > 0);
+  std::cout << "packages_ptr = "<< packagesPtr << ", count = " << count << std::endl;
 
-  std::cout << "packages_ptr = "<< packages << ", count = " << count << std::endl;
+  std::vector<TwPackage_t> packages(packagesPtr,packagesPtr+count);
 
-  for (int i = 0; i < count; i++) {
-    auto package = packages[i];
-    std::cout << i + 1 << ". " << std::string((char *)package.identifier.ptr, package.identifier.len) << "; ";
+  for (auto package: packages) {
+    std::cout << ". " << std::string((char *)package.identifier.ptr, package.identifier.len) << "; ";
 
     auto section = tw_package_section_str(package.inner);
     std::cout << "section = " << std::string((char *)section.ptr, section.len) << "; ";
@@ -57,8 +62,14 @@ int main(int argc, char *argv[]) {
     std::cout << std::endl;
   }
 
-  slice_ref_TwPackage_t rebuild_packages;
-  rebuild_packages.ptr = packages;
+  auto rawPkgsPointers = new TwPackageRef_t[5];
+  auto rawPkgsPointersPtr = rawPkgsPointers;
+  for (auto package: packages) {
+    *rawPkgsPointersPtr++ = package.inner;
+  }
+
+  slice_ref_TwPackageRef_t rebuild_packages;
+  rebuild_packages.ptr = rawPkgsPointers;
   rebuild_packages.len = count;
 
   TwProgressFunctions_t functions;

twackup/src/ffi/builder/mod.rs

@@ -20,17 +20,19 @@
 pub(crate) mod progress;
 
 use self::progress::{TwProgressFunctions, TwProgressImpl};
+use super::ArcContainer;
 use crate::{
     builder::{Preferences, Worker},
     ffi::{c_dpkg::TwDpkg, package::TwPackage},
+    package::Package,
     progress::Progress,
     Result,
 };
 use safer_ffi::{
     derive_ReprC,
     prelude::{
         c_slice::{Box, Ref},
-        char_p,
+        char_p, Out,
     },
     ptr::NonNullMut,
 };
@@ -40,6 +42,7 @@ use std::{collections::LinkedList, os::unix::ffi::OsStringExt, sync::Arc};
 #[repr(C)]
 pub struct TwPackagesRebuildResult {
     success: bool,
+    package: ArcContainer<Package>,
     deb_path: Option<Box<u8>>,
     error: Option<Box<u8>>,
 }
@@ -96,11 +99,11 @@ pub struct TwBuildPreferences {
 #[derive_ReprC]
 #[repr(C)]
 pub struct TwBuildParameters<'a> {
-    packages: Ref<'a, TwPackage>,
+    packages: Ref<'a, ArcContainer<Package>>,
     functions: TwProgressFunctions,
     out_dir: char_p::Ref<'a>,
     preferences: TwBuildPreferences,
-    results: Option<NonNullMut<Box<TwPackagesRebuildResult>>>,
+    results: Option<Out<'a, Box<TwPackagesRebuildResult>>>,
 }
 
 pub(crate) fn rebuild_packages(dpkg: &TwDpkg, parameters: TwBuildParameters<'_>) -> Result<()> {
@@ -121,41 +124,44 @@ pub(crate) fn rebuild_packages(dpkg: &TwDpkg, parameters: TwBuildParameters<'_>)
 
     let mut workers = LinkedList::new();
     for package in parameters.packages.iter() {
-        let package = package.inner.clone();
+        let package = package.clone();
         let dpkg_contents = dpkg_contents.clone();
         let preferences = preferences.clone();
 
         workers.push_back(tokio_rt.spawn(async move {
-            let package = &*package;
-            let worker = Worker::new(package, progress, None, preferences, dpkg_contents);
-            worker.run().await
+            let worker = Worker::new(&*package, progress, None, preferences, dpkg_contents);
+            let result = worker.run().await;
+            (package, result)
         }));
     }
 
-    let mut errors_vec = vec![];
+    let mut results = vec![];
     tokio_rt.block_on(async {
         for worker in workers {
-            let Ok(result) = worker.await else { continue };
+            let Ok((package, result)) = worker.await else {
+                continue;
+            };
 
             log::debug!("rebuild result = {result:?}");
 
-            let (path, error) = match result {
+            let result = result
+                .map(|path| {
+                    let path = OsStringExt::into_vec(path.into_os_string());
+                    Box::from(path.into_boxed_slice())
+                })
+                .map_err(|error| {
+                    let error = error.to_string().into_bytes();
+                    Box::from(error.into_boxed_slice())
+                });
+
+            let (deb_path, error) = match result {
                 Ok(path) => (Some(path), None),
                 Err(error) => (None, Some(error)),
             };
 
-            let deb_path = path.map(|path| {
-                let path = OsStringExt::into_vec(path.into_os_string());
-                Box::from(path.into_boxed_slice())
-            });
-
-            let error = error.map(|error| {
-                let error = error.to_string().into_bytes();
-                Box::from(error.into_boxed_slice())
-            });
-
-            errors_vec.push(TwPackagesRebuildResult {
+            results.push(TwPackagesRebuildResult {
                 success: deb_path.is_some(),
+                package,
                 deb_path,
                 error,
             });
@@ -164,11 +170,9 @@ pub(crate) fn rebuild_packages(dpkg: &TwDpkg, parameters: TwBuildParameters<'_>)
 
     progress.finished_all();
 
-    if let Some(mut results_ptr) = parameters.results {
-        unsafe {
-            let boxed = Box::from(errors_vec.into_boxed_slice());
-            results_ptr.as_mut_ptr().write(boxed);
-        }
+    if let Some(results_out) = parameters.results {
+        let boxed = Box::from(results.into_boxed_slice());
+        results_out.write(boxed);
     }
 
     Ok(())

twackup/src/ffi/builder/progress.rs

@@ -19,24 +19,19 @@
 
 use crate::{ffi::package::TwPackage, package::Package, progress::Progress};
 use safer_ffi::{derive_ReprC, prelude::c_slice::Raw, slice::Ref};
-use std::{ffi::c_void, os::unix::ffi::OsStrExt, path::Path, ptr::addr_of};
+use std::{ffi::c_void, os::unix::ffi::OsStrExt, path::Path, ptr::NonNull};
+
+type Context = Option<NonNull<c_void>>;
 
 #[derive_ReprC]
 #[repr(C)]
 #[derive(Copy, Clone)]
 pub struct TwProgressFunctions {
-    context: Option<std::ptr::NonNull<c_void>>,
-    started_processing: Option<
-        unsafe extern "C" fn(context: Option<std::ptr::NonNull<c_void>>, package: *const TwPackage),
-    >,
-    finished_processing: Option<
-        unsafe extern "C" fn(
-            context: Option<std::ptr::NonNull<c_void>>,
-            package: *const TwPackage,
-            deb_path: Raw<u8>,
-        ),
-    >,
-    finished_all: Option<unsafe extern "C" fn(context: Option<std::ptr::NonNull<c_void>>)>,
+    context: Context,
+    started_processing: Option<unsafe extern "C" fn(context: Context, package: TwPackage)>,
+    finished_processing:
+        Option<unsafe extern "C" fn(context: Context, package: TwPackage, deb_path: Raw<u8>)>,
+    finished_all: Option<unsafe extern "C" fn(context: Context)>,
 }
 
 #[derive(Copy, Clone)]
@@ -48,7 +43,7 @@ impl Progress for TwProgressImpl {
     fn started_processing(&self, package: &Package) {
         if let Some(func) = self.functions.started_processing {
             let package = TwPackage::from(package);
-            unsafe { func(self.functions.context, addr_of!(package)) };
+            unsafe { func(self.functions.context, package) };
         }
     }
 
@@ -58,7 +53,7 @@ impl Progress for TwProgressImpl {
             let deb_path = deb_path.as_ref().as_os_str().as_bytes();
             let deb_path = Raw::from(Ref::from(deb_path));
 
-            unsafe { func(self.functions.context, addr_of!(package), deb_path) };
+            unsafe { func(self.functions.context, package, deb_path) };
         }
     }
 

twackup/src/ffi/mod.rs

@@ -175,6 +175,12 @@ fn tw_package_release(package: ArcContainer<Package>) {
     drop(package);
 }
 
+#[ffi_export]
+fn tw_package_ref_count(package: ArcContainer<Package>) -> u64 {
+    let package = ManuallyDrop::new(package);
+    package.ref_count() as u64
+}
+
 #[ffi_export]
 fn tw_package_retain(package: ArcContainer<Package>) {
     package.retain();