Merge pull request #568 from madsmtm/fix-block-memory-management

Fix block2 memory management

crates/block2/CHANGELOG.md

@@ -6,16 +6,30 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased - YYYY-MM-DD
 
+### Added
+* Added `RcBlock::new(closure)` as a more efficient and flexible alternative
+  to `StackBlock::new(closure).to_rc()`.
+* Added `StackBlock::to_rc` to convert stack blocks to `RcBlock`.
+
 ### Changed
+* **BREAKING**: Renamed `RcBlock::new(ptr)` to `RcBlock::from_raw(ptr)`.
+* **BREAKING**: Made `RcBlock` use the null-pointer optimization;
+  `RcBlock::from_raw` and `RcBlock::copy` now return an `Option`.
 * **BREAKING**: Only expose the actually public symbols `_Block_copy`,
   `_Block_release`, `_Block_object_assign`, `_Block_object_dispose`,
   `_NSConcreteGlobalBlock`, `_NSConcreteStackBlock` and `Class` in `ffi`
   module.
+* **BREAKING**: Renamed `IntoConcreteBlock` to `IntoBlock`.
 * No longer use the `block-sys` crate for linking to the blocks runtime.
 * Renamed `ConcreteBlock` to `StackBlock`. The old name is deprecated.
-* **BREAKING**: Renamed `IntoConcreteBlock` to `IntoBlock`.
+* Added `Copy` implementation for `StackBlock`.
+
+### Deprecated
+* Deprecated `StackBlock::copy`, it is no longer necessary.
 
 ### Fixed
+* **BREAKING**: `StackBlock::new` now requires the closure to be `Clone`. If
+  this is not desired, use `RcBlock::new` instead.
 * Relaxed the `F: Debug` bound on `StackBlock`'s `Debug` implementation.
 
 

crates/block2/src/abi.rs

@@ -254,8 +254,14 @@ pub(crate) struct BlockDescriptorCopyDispose {
     pub(crate) size: c_ulong,
 
     /// Helper to copy the block if it contains nontrivial copy operations.
+    ///
+    /// This may be NULL since macOS 11.0.1 in Apple's runtime, but this
+    /// should not be relied on.
     pub(crate) copy: Option<unsafe extern "C" fn(dst: *mut c_void, src: *const c_void)>,
     /// Helper to destroy the block after being copied.
+    ///
+    /// This may be NULL since macOS 11.0.1 in Apple's runtime, but this
+    /// should not be relied on.
     pub(crate) dispose: Option<unsafe extern "C" fn(src: *mut c_void)>,
 }
 
@@ -293,8 +299,14 @@ pub(crate) struct BlockDescriptorCopyDisposeSignature {
     pub(crate) size: c_ulong,
 
     /// Helper to copy the block if it contains nontrivial copy operations.
+    ///
+    /// This may be NULL since macOS 11.0.1 in Apple's runtime, but this
+    /// should not be relied on.
     pub(crate) copy: Option<unsafe extern "C" fn(dst: *mut c_void, src: *const c_void)>,
     /// Helper to destroy the block after being copied.
+    ///
+    /// This may be NULL since macOS 11.0.1 in Apple's runtime, but this
+    /// should not be relied on.
     pub(crate) dispose: Option<unsafe extern "C" fn(src: *mut c_void)>,
 
     /// Objective-C type encoding of the block.

crates/block2/src/global.rs

@@ -23,10 +23,10 @@ const GLOBAL_DESCRIPTOR: BlockDescriptor = BlockDescriptor {
 /// This is effectively a glorified function pointer, and can created and
 /// stored in static memory using the [`global_block!`] macro.
 ///
-/// If [`StackBlock`] is the [`Fn`]-block equivalent, this is likewise the
+/// If [`RcBlock`] is the [`Fn`]-block equivalent, this is likewise the
 /// [`fn`]-block equivalent.
 ///
-/// [`StackBlock`]: crate::StackBlock
+/// [`RcBlock`]: crate::RcBlock
 /// [`global_block!`]: crate::global_block
 #[repr(C)]
 pub struct GlobalBlock<A, R = ()> {

crates/block2/src/lib.rs

@@ -45,35 +45,112 @@
 //!
 //! ## Creating blocks
 //!
-//! Creating a block to pass to Objective-C can be done with the
-//! [`StackBlock`] struct. For example, to create a block that adds two
-//! integers, we could write:
+//! Creating a block to pass to Objective-C can be done with [`RcBlock`] or
+//! [`StackBlock`], depending on if you want to move the block to the heap,
+//! or let the callee decide if it needs to do that.
+//!
+//! To declare external functions or methods that takes blocks, use
+//! `&Block<A, R>` or `Option<&Block<A, R>>`, where `A` is a tuple with the
+//! argument types, and `R` is the return type.
+//!
+//! As an example, we're going to work with a block that adds two integers.
 //!
 //! ```
-//! use block2::StackBlock;
-//! let block = StackBlock::new(|a: i32, b: i32| a + b);
-//! let block = block.copy();
-//! assert_eq!(unsafe { block.call((5, 8)) }, 13);
+//! use block2::Block;
+//!
+//! // External function that takes a block
+//! extern "C" {
+//!     fn add_numbers_using_block(block: &Block<(i32, i32), i32>);
+//! }
+//! #
+//! # use objc2::ClassType;
+//! # objc2::extern_class!(
+//! #     struct MyClass;
+//! #
+//! #     unsafe impl ClassType for MyClass {
+//! #         type Super = objc2::runtime::NSObject;
+//! #         type Mutability = objc2::mutability::InteriorMutable;
+//! #         const NAME: &'static str = "NSObject";
+//! #     }
+//! # );
+//!
+//! // External method that takes a block
+//! objc2::extern_methods!(
+//!     unsafe impl MyClass {
+//!         #[method(addNumbersUsingBlock:)]
+//!         pub fn addNumbersUsingBlock(&self, block: &Block<(i32, i32), i32>);
+//!     }
+//! );
 //! ```
 //!
-//! It is important to copy your block to the heap (with the [`copy`] method)
-//! before passing it to Objective-C; this is because our [`StackBlock`] is
-//! only meant to be copied once, and we can enforce this in Rust, but if
-//! Objective-C code were to copy it twice we could have a double free.
+//! To call such a function / method, we could create a new block from a
+//! closure using [`RcBlock::new`].
+//!
+//! ```
+//! use block2::RcBlock;
+//! #
+//! # extern "C" {
+//! #     fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>);
+//! # }
+//! # mod imp {
+//! #     #[no_mangle]
+//! #     extern "C" fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>) {
+//! #         assert_eq!(unsafe { block.call((5, 8)) }, 13);
+//! #     }
+//! # }
+//!
+//! let block = RcBlock::new(|a: i32, b: i32| a + b);
+//! unsafe { add_numbers_using_block(&block) };
+//! ```
 //!
-//! [`copy`]: StackBlock::copy
+//! This creates the block on the heap. If the external function you're
+//! calling is not going to copy the block, it may be more performant if you
+//! construct a [`StackBlock`] directly, using [`StackBlock::new`].
 //!
-//! As an optimization if your block doesn't capture any variables, you can
-//! use the [`global_block!`] macro to create a static block:
+//! Note though that this requires that the closure is [`Clone`], as the
+//! external code may want to copy the block to the heap in the future.
+//!
+//! ```
+//! use block2::StackBlock;
+//! #
+//! # extern "C" {
+//! #     fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>);
+//! # }
+//! # mod imp {
+//! #     #[no_mangle]
+//! #     extern "C" fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>) {
+//! #         assert_eq!(unsafe { block.call((5, 8)) }, 13);
+//! #     }
+//! # }
+//!
+//! let block = StackBlock::new(|a: i32, b: i32| a + b);
+//! unsafe { add_numbers_using_block(&block) };
+//! ```
+//!
+//! As an optimization if your block doesn't capture any variables (as in the
+//! above examples), you can use the [`global_block!`] macro to create a
+//! static block.
 //!
 //! ```
 //! use block2::global_block;
+//! #
+//! # extern "C" {
+//! #     fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>);
+//! # }
+//! # mod imp {
+//! #     #[no_mangle]
+//! #     extern "C" fn add_numbers_using_block(block: &block2::Block<(i32, i32), i32>) {
+//! #         assert_eq!(unsafe { block.call((5, 8)) }, 13);
+//! #     }
+//! # }
+//!
 //! global_block! {
-//!     static MY_BLOCK = || -> f32 {
-//!         10.0
+//!     static MY_BLOCK = |a: i32, b: i32| -> i32 {
+//!         a + b
 //!     };
 //! }
-//! assert_eq!(unsafe { MY_BLOCK.call(()) }, 10.0);
+//!
+//! unsafe { add_numbers_using_block(&MY_BLOCK) };
 //! ```
 //!
 //!