diff --git a/CHANGELOG.md b/CHANGELOG.md index 83ebe40b..8d81ba92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,26 @@ Enhancements: Internal: * Refactor type handling by merging `TypeMapper` into `TypeDetail` and simplifying class hierarchy -Incomptatible Changes: +Incompatible Changes: +* `Address_Registration_Guard` has been replaced by `Pin`. If you are using `Address_Registration_Guard` + to protect Ruby VALUEs from garbage collection, update your code to use `Pin` instead: + + Before: + ```cpp + VALUE value_; + Address_Registration_Guard guard_; + MyClass() : value_(rb_str_new2("test")), guard_(&value_) {} + ``` + + After: + ```cpp + Pin pin_; + MyClass() : pin_(rb_str_new2("test")) {} + VALUE getValue() { return pin_.get(); } + ``` + + `Pin` is self-contained and provides `get()` and `set()` methods to access the pinned VALUE. + * Rice converts Ruby blocks to procs. Thus if you have a method that expects a block, you must add a VALUE parameter and tell Rice it is a value. For example: diff --git a/Rice.cmake b/Rice.cmake index 572957d5..ef60d0b8 100644 --- a/Rice.cmake +++ b/Rice.cmake @@ -1,11 +1,18 @@ -add_library(Rice INTERFACE) -add_library(Rice::Rice ALIAS Rice) +include_guard(GLOBAL) -target_include_directories(Rice INTERFACE - $:${CMAKE_CURRENT_SOURCE_DIR}>> - $:${CMAKE_CURRENT_SOURCE_DIR}/include>> - $ -) +if(NOT TARGET Rice) + add_library(Rice INTERFACE) -# Need C++17 or higher -target_compile_features(Rice INTERFACE cxx_std_17) + target_include_directories(Rice INTERFACE + $:${CMAKE_CURRENT_SOURCE_DIR}>> + $:${CMAKE_CURRENT_SOURCE_DIR}/include>> + $ + ) + + # Need C++17 or higher + target_compile_features(Rice INTERFACE cxx_std_17) +endif() + +if(NOT TARGET Rice::Rice) + add_library(Rice::Rice ALIAS Rice) +endif() diff --git a/docs/bindings/callbacks.md b/docs/bindings/callbacks.md index 91434802..475a76e2 100644 --- a/docs/bindings/callbacks.md +++ b/docs/bindings/callbacks.md @@ -111,3 +111,14 @@ abort "libffi not found" unless have_libffi ``` If you are using CMake, you will need to add a C++ preprocessor define called `HAVE_LIBFFI` and link to libffi. + +## Memory Management + +Callback wrappers (NativeCallback instances) are intentionally never freed. This is because: + +1. C code may call the callback at any time, even after the Ruby code that registered it has finished executing +2. Ruby users commonly pass blocks to C callbacks, which Rice converts to Procs internally. Without special handling, these Procs would be garbage collected and the callback would crash when invoked + +Rice uses `Pin` internally to prevent the Ruby Proc from being garbage collected, ensuring the callback remains valid for the lifetime of the program. + +In practice, this means each unique callback registration consumes a small amount of memory that is never reclaimed. For most applications this is not a concern, as callbacks are typically registered once during initialization. diff --git a/docs/bindings/memory_management.md b/docs/bindings/memory_management.md index 304e945b..6049f260 100644 --- a/docs/bindings/memory_management.md +++ b/docs/bindings/memory_management.md @@ -217,7 +217,49 @@ If you are working with VALUEs or Objects stored on the stack, the Ruby garbage ### Heap -If you allocate an Object on the heap or if it is a member of an object that might be allocated on the heap, use `Rice::Address_Registration_Guard` to register the object with the garbage collector. +If a C++ object holds a Ruby VALUE and that C++ object is *not* wrapped by Ruby (i.e., it's allocated on the heap or is a standalone object), use `Rice::Pin` to prevent the garbage collector from collecting the Ruby object. + +```cpp +class Container +{ +public: + Container(VALUE value) : pin_(value) {} + VALUE getValue() const { return pin_.get(); } + void setValue(VALUE value) { pin_.set(value); } + +private: + Rice::Pin pin_; +}; +``` + +#### Pin API + +* `Pin(VALUE value)` - Construct a Pin that protects the given VALUE from garbage collection +* `VALUE get() const` - Retrieve the pinned VALUE +* `void set(VALUE value)` - Replace the pinned VALUE + +#### Copy Semantics + +`Pin` uses shared ownership internally. When you copy a `Pin`, both copies share the same underlying GC anchor: + +```cpp +Pin pin1(some_value); +Pin pin2 = pin1; // pin1 and pin2 share the same anchor + +pin1.set(other_value); // Both pin1.get() and pin2.get() now return other_value +``` + +This is useful when multiple C++ objects need to reference the same Ruby object - only one GC registration is needed. + +#### When to Use Pin vs ruby_mark + +Use `Pin` when: +* The C++ object is **not** wrapped by Ruby (e.g., created with `new` in C++, stored in a global, or part of a C++ library's internal data structures) +* You want self-contained protection without manual GC registration + +Use `ruby_mark` (see below) when: +* The C++ object **is** wrapped by Ruby via `Data_Type` +* Ruby owns the C++ object and will call the mark function during garbage collection ### Member Variables diff --git a/include/rice/rice.hpp b/include/rice/rice.hpp index 89c2881b..9612aa1a 100644 --- a/include/rice/rice.hpp +++ b/include/rice/rice.hpp @@ -1388,6 +1388,7 @@ namespace Rice { public: ArgBuffer(std::string name); + using Arg::operator=; // Inherit the templated operator= }; } // Rice @@ -1687,9 +1688,7 @@ namespace Rice operator VALUE() const { return value_; } //! Explicitly get the encapsulated VALUE. - // Returns a const ref so that Address_Registration_Guard can access - // the address where the VALUE is stored - VALUE const volatile& value() const { return value_; } + VALUE value() const { return value_; } //! Get the class of an object. /*! \return the object's Class. @@ -4088,9 +4087,27 @@ namespace Rice::detail template inline VALUE Parameter::defaultValueRuby() { + using To_Ruby_T = remove_cv_recursive_t; + + if (!this->arg()->hasDefaultValue()) + { + throw std::runtime_error("No default value set for " + this->arg()->name); + } + if constexpr (!is_complete_v>) { - // Incomplete types can't have default values (std::any requires complete types) + // Only pointers to incomplete types can have + // default values (e.g., void* or Impl*), since the pointer itself is complete. + // References and values of incomplete types cannot be stored in std::any. + if constexpr (std::is_pointer_v>) + { + T defaultValue = this->arg()->template defaultValue(); + return this->toRuby_.convert((To_Ruby_T)defaultValue); + } + else + { + throw std::runtime_error("Default value not allowed for incomple type. Parameter " + this->arg()->name); + } } else if constexpr (std::is_constructible_v, std::remove_cv_t>&>) { @@ -4100,21 +4117,20 @@ namespace Rice::detail { if constexpr (std::is_copy_constructible_v::value_type>) { - if (this->arg()->hasDefaultValue()) - { - T defaultValue = this->arg()->template defaultValue(); - return this->toRuby_.convert(defaultValue); - } + T defaultValue = this->arg()->template defaultValue(); + return this->toRuby_.convert(defaultValue); } } - else if (this->arg()->hasDefaultValue()) + else { T defaultValue = this->arg()->template defaultValue(); - return this->toRuby_.convert((remove_cv_recursive_t)defaultValue); + return this->toRuby_.convert((To_Ruby_T)defaultValue); } } - - throw std::runtime_error("No default value set or allowed for parameter " + this->arg()->name); + else + { + throw std::runtime_error("Default value not allowed for parameter " + this->arg()->name); + } } template @@ -5411,19 +5427,22 @@ namespace Rice define_method("data", &Buffer_T::ptr, ReturnBuffer()). define_method("release", &Buffer_T::release, ReturnBuffer()); - if constexpr (!std::is_pointer_v && !std::is_void_v && !std::is_const_v && std::is_copy_assignable_v) + if constexpr (detail::is_complete_v>) { - klass.define_method("[]=", [](Buffer_T& self, size_t index, T& value) -> void + if constexpr (!std::is_pointer_v && !std::is_void_v && !std::is_const_v && std::is_copy_assignable_v) { - self[index] = value; - }); - } - else if constexpr (std::is_pointer_v && !std::is_const_v> && std::is_copy_assignable_v>) - { - klass.define_method("[]=", [](Buffer_T& self, size_t index, T value) -> void + klass.define_method("[]=", [](Buffer_T& self, size_t index, T& value) -> void + { + self[index] = value; + }); + } + else if constexpr (std::is_pointer_v && !std::is_const_v> && std::is_copy_assignable_v>) { - *self[index] = *value; - }); + klass.define_method("[]=", [](Buffer_T& self, size_t index, T value) -> void + { + *self[index] = *value; + }); + } } return klass; @@ -9931,11 +9950,14 @@ namespace Rice::detail { Registries::instance.instances.remove(this->get(this->rb_data_type_)); - if constexpr (std::is_destructible_v) + if constexpr (is_complete_v) { - if (this->isOwner_) + if constexpr (std::is_destructible_v) { - delete this->data_; + if (this->isOwner_) + { + delete this->data_; + } } } } @@ -11265,7 +11287,12 @@ namespace Rice::detail detail::To_Ruby toRuby; for (; it != end; ++it) { - protect(rb_yield, toRuby.convert(*it)); + // Use auto&& to accept both reference- and value-returning iterators. + // - If *it is an lvalue (T&), auto&& deduces to T&, no copy made. + // - If *it is a prvalue (T), auto&& deduces to T&&, value binds to the temporary for the scope of this loop iteration. + // This also avoids MSVC C4239 when convert expects a non-const lvalue reference. + auto&& value = *it; + protect(rb_yield, toRuby.convert(value)); } return self; @@ -11857,6 +11884,168 @@ namespace Rice::detail } } } +// ========= Anchor.hpp ========= + +#include + +namespace Rice +{ + namespace detail + { + //! Internal GC anchor for a Ruby VALUE. + /*! + * Anchor is a low-level adapter around the Ruby GC API. + * It owns a stable VALUE slot whose address is registered + * with the Ruby garbage collector. + * + * This class encapsulates all GC registration logic and is + * not part of the public Rice API. + */ + class Anchor + { + public: + //! Construct an anchor for the given Ruby VALUE. + /*! + * The address of the internal VALUE is registered with the + * Ruby GC, preventing collection while this Anchor exists. + */ + explicit Anchor(VALUE value); + + //! Unregister the VALUE from the Ruby GC. + ~Anchor(); + + Anchor(const Anchor&) = delete; + Anchor& operator=(const Anchor&) = delete; + + //! Retrieve the currently anchored VALUE. + VALUE get() const; + + //! Replace the anchored VALUE. + /*! + * The GC root (address) remains unchanged; only the VALUE + * stored at that address is updated. + */ + void set(VALUE value); + + private: + static void disable(VALUE); + static void registerExitHandler(); + + inline static bool enabled_ = true; + inline static bool exitHandlerRegistered_ = false; + + private: + //! GC-visible Ruby VALUE slot. + VALUE value_; + }; + } +} + +// ========= Anchor.ipp ========= +namespace Rice +{ + namespace detail + { + inline Anchor::Anchor(VALUE value) : value_(value) + { + Anchor::registerExitHandler(); + detail::protect(rb_gc_register_address, &this->value_); + } + + inline Anchor::~Anchor() + { + if (Anchor::enabled_) + { + detail::protect(rb_gc_unregister_address, &this->value_); + } + } + + inline VALUE Anchor::get() const + { + return this->value_; + } + + inline void Anchor::set(VALUE value) + { + this->value_ = value; + } + + // This will be called by ruby at exit - we want to disable further unregistering + inline void Anchor::disable(VALUE) + { + Anchor::enabled_ = false; + } + + inline void Anchor::registerExitHandler() + { + if (!Anchor::exitHandlerRegistered_) + { + detail::protect(rb_set_end_proc, &Anchor::disable, Qnil); + Anchor::exitHandlerRegistered_ = true; + } + } + } +} +// ========= Pin.hpp ========= + +namespace Rice +{ + //! Strong lifetime policy for a Ruby VALUE. + /*! + * Pin represents a Ruby VALUE whose lifetime is explicitly + * extended by C++ code. + * + * Internally, Pin uses a GC Anchor to keep the VALUE alive. + * Copying a Pin shares the underlying anchor; moving is cheap. + * + * This type is intended for C++-owned objects that store Ruby + * values but are not themselves owned by Ruby and thus do not + * participate in the GC via a mark function. + */ + class Pin + { + public: + //! Construct a strong pin for a Ruby VALUE. + explicit Pin(VALUE value); + + // Shared-anchor semantics. + Pin(const Pin&) = default; + Pin& operator=(const Pin&) = default; + + Pin(Pin&&) noexcept = default; + Pin& operator=(Pin&&) noexcept = default; + + //! Replace the pinned Ruby VALUE. + void set(VALUE value); + + //! Retrieve the pinned Ruby VALUE. + VALUE get() const; + + private: + //! Shared ownership of the internal GC anchor. + std::shared_ptr anchor_; + }; +} + +// ========= Pin.ipp ========= +namespace Rice +{ + inline Pin::Pin(VALUE value) + : anchor_(std::make_shared(value)) + { + } + + inline void Pin::set(VALUE value) + { + this->anchor_->set(value); + } + + inline VALUE Pin::get() const + { + return this->anchor_->get(); + } +} + // ========= NativeCallback.hpp ========= #ifdef HAVE_LIBFFI @@ -11868,6 +12057,11 @@ namespace Rice::detail template class NativeCallback; + // NativeCallback instances are never freed because there is no way for us to know + // when they can be freed. At the same time, the Pin prevents the Ruby proc from being + // garbage collected, which is necessary because C code may call the callback + // at any time. This supports passing blocks to C callbacks without requiring the Ruby + // user to manually hold a reference to a proc. template class NativeCallback : public Native { @@ -11876,8 +12070,6 @@ namespace Rice::detail using NativeCallback_T = NativeCallback; using Tuple_T = std::tuple; - static VALUE finalizerCallback(VALUE yielded_arg, VALUE callback_arg, int argc, const VALUE* argv, VALUE blockarg); - template static void define(Arg_Ts&& ...args); @@ -11917,7 +12109,7 @@ namespace Rice::detail template Return_T callRuby(std::index_sequence& indices, Parameter_Ts...args); - VALUE proc_; + Pin proc_; From_Ruby fromRuby_; #ifdef HAVE_LIBFFI @@ -12096,14 +12288,6 @@ namespace Rice::detail return std::forward_as_tuple(extractArg(args[I])...); } - template - VALUE NativeCallback::finalizerCallback(VALUE, VALUE callback_arg, int, const VALUE*, VALUE) - { - NativeCallback_T* nativeCallback = (NativeCallback_T*)callback_arg; - delete nativeCallback; - return Qnil; - } - template template inline void NativeCallback::define(Arg_Ts&& ...args) @@ -12121,10 +12305,6 @@ namespace Rice::detail Native("callback", copyReturnInfo(), copyParameters()), proc_(proc), fromRuby_(returnInfo_.get()) { - // Tie the lifetime of the NativeCallback C++ instance to the lifetime of the Ruby proc object - VALUE finalizer = rb_proc_new(NativeCallback_T::finalizerCallback, (VALUE)this); - rb_define_finalizer(proc, finalizer); - #ifdef HAVE_LIBFFI // First setup description of callback if (cif_.bytes == 0) @@ -12151,8 +12331,6 @@ namespace Rice::detail NativeCallback_T::callback_ = nullptr; NativeCallback_T::native_ = nullptr; - - this->proc_ = Qnil; } template @@ -12171,7 +12349,7 @@ namespace Rice::detail convertToRuby(args)... }; static Identifier id("call"); - VALUE result = detail::protect(rb_funcallv, this->proc_, id.id(), (int)sizeof...(Parameter_Ts), values.data()); + VALUE result = detail::protect(rb_funcallv, this->proc_.get(), id.id(), (int)sizeof...(Parameter_Ts), values.data()); if constexpr (!std::is_void_v) { return this->fromRuby_.convert(result); @@ -14149,163 +14327,6 @@ namespace Rice::detail } }; } - -// ========= Address_Registration_Guard.hpp ========= - -namespace Rice -{ - //! A guard to register a given address with the GC. - /*! Calls rb_gc_register_address upon construction and - * rb_gc_unregister_address upon destruction. - * For example: - * \code - * Class Foo - * { - * public: - * Foo() - * : string_(rb_str_new2()) - * , guard_(&string_); - * - * private: - * VALUE string_; - * Address_Registration_Guard guard_; - * }; - * \endcode - */ - class Address_Registration_Guard - { - public: - //! Register an address with the GC. - /* \param address The address to register with the GC. The address - * must point to a valid ruby object (RObject). - */ - Address_Registration_Guard(VALUE* address); - - //! Register an Object with the GC. - /*! \param object The Object to register with the GC. The object must - * not be destroyed before the Address_Registration_Guard is - * destroyed. - */ - Address_Registration_Guard(Object* object); - - //! Unregister an address/Object with the GC. - /*! Destruct an Address_Registration_Guard. The address registered - * with the Address_Registration_Guard when it was constructed will - * be unregistered from the GC. - */ - ~Address_Registration_Guard(); - - // Disable copying - Address_Registration_Guard(Address_Registration_Guard const& other) = delete; - Address_Registration_Guard& operator=(Address_Registration_Guard const& other) = delete; - - // Enable moving - Address_Registration_Guard(Address_Registration_Guard&& other); - Address_Registration_Guard& operator=(Address_Registration_Guard&& other); - - //! Get the address that is registered with the GC. - VALUE* address() const; - - /** Called during Ruby's exit process since we should not call - * rb_gc unregister_address there - */ - static void disable(); - - private: - inline static bool enabled = true; - inline static bool exit_handler_registered = false; - static void registerExitHandler(); - - private: - void registerAddress() const; - void unregisterAddress(); - - VALUE* address_ = nullptr; - }; -} // namespace Rice - - -// ========= Address_Registration_Guard.ipp ========= -namespace Rice -{ - inline Address_Registration_Guard::Address_Registration_Guard(VALUE* address) : address_(address) - { - registerExitHandler(); - registerAddress(); - } - - inline Address_Registration_Guard::Address_Registration_Guard(Object* object) - : address_(const_cast(&object->value())) - { - registerExitHandler(); - registerAddress(); - } - - inline Address_Registration_Guard::~Address_Registration_Guard() - { - unregisterAddress(); - } - - inline Address_Registration_Guard::Address_Registration_Guard(Address_Registration_Guard&& other) - { - // We don't use the constructor because we don't want to double register this address - address_ = other.address_; - other.address_ = nullptr; - } - - inline Address_Registration_Guard& Address_Registration_Guard::operator=(Address_Registration_Guard&& other) - { - this->unregisterAddress(); - - this->address_ = other.address_; - other.address_ = nullptr; - return *this; - } - - inline void Address_Registration_Guard::registerAddress() const - { - if (enabled) - { - detail::protect(rb_gc_register_address, address_); - } - } - - inline void Address_Registration_Guard::unregisterAddress() - { - if (enabled && address_) - { - detail::protect(rb_gc_unregister_address, address_); - } - - address_ = nullptr; - } - - inline VALUE* Address_Registration_Guard::address() const - { - return address_; - } - - static void disable_all_guards(VALUE) - { - Address_Registration_Guard::disable(); - } - - inline void Address_Registration_Guard::registerExitHandler() - { - if (exit_handler_registered) - { - return; - } - - detail::protect(rb_set_end_proc, &disable_all_guards, Qnil); - exit_handler_registered = true; - } - - inline void Address_Registration_Guard::disable() - { - enabled = false; - } -} // Rice // ========= global_function.hpp ========= namespace Rice @@ -14411,16 +14432,13 @@ namespace Rice template inline void ruby_mark_internal(detail::WrapperBase* wrapper) { - detail::cpp_protect([&] - { - // Tell the wrapper to mark the objects its keeping alive - wrapper->ruby_mark(); + // Tell the wrapper to mark the objects its keeping alive + wrapper->ruby_mark(); - // Get the underlying data and call custom mark function (if any) - // Use the wrapper's stored rb_data_type to avoid type mismatch - T* data = static_cast(wrapper->get(Data_Type::ruby_data_type())); - ruby_mark(data); - }); + // Get the underlying data and call custom mark function (if any) + // Use the wrapper's stored rb_data_type to avoid type mismatch + T* data = static_cast(wrapper->get(Data_Type::ruby_data_type())); + ruby_mark(data); } template @@ -15571,15 +15589,13 @@ namespace Rice::detail double is_convertible(VALUE value) { - bool isBuffer = dynamic_cast(this->arg_) ? true : false; - switch (rb_type(value)) { case RUBY_T_NIL: return Convertible::Exact; break; case RUBY_T_DATA: - if (Data_Type::is_descendant(value) && isBuffer) + if (Data_Type::is_descendant(value)) { return Convertible::Exact; } @@ -15592,7 +15608,6 @@ namespace Rice::detail T** convert(VALUE value) { bool isOwner = this->arg_ && this->arg_->isOwner(); - bool isBuffer = dynamic_cast(this->arg_) ? true : false; switch (rb_type(value)) { @@ -15603,7 +15618,7 @@ namespace Rice::detail } case RUBY_T_DATA: { - if (Data_Type::is_descendant(value) && isBuffer) + if (Data_Type::is_descendant(value)) { T** result = detail::unwrap(value, Data_Type::ruby_data_type(), isOwner); return result; @@ -15612,14 +15627,7 @@ namespace Rice::detail } default: { - if (isBuffer) - { - throw create_type_exception(value); - } - else - { - throw create_type_exception(value); - } + throw create_type_exception(value); } } } diff --git a/rice/Address_Registration_Guard.hpp b/rice/Address_Registration_Guard.hpp deleted file mode 100644 index dc62034b..00000000 --- a/rice/Address_Registration_Guard.hpp +++ /dev/null @@ -1,76 +0,0 @@ -#ifndef Rice__Address_Registration_Guard__hpp_ -#define Rice__Address_Registration_Guard__hpp_ - -namespace Rice -{ - //! A guard to register a given address with the GC. - /*! Calls rb_gc_register_address upon construction and - * rb_gc_unregister_address upon destruction. - * For example: - * \code - * Class Foo - * { - * public: - * Foo() - * : string_(rb_str_new2()) - * , guard_(&string_); - * - * private: - * VALUE string_; - * Address_Registration_Guard guard_; - * }; - * \endcode - */ - class Address_Registration_Guard - { - public: - //! Register an address with the GC. - /* \param address The address to register with the GC. The address - * must point to a valid ruby object (RObject). - */ - Address_Registration_Guard(VALUE* address); - - //! Register an Object with the GC. - /*! \param object The Object to register with the GC. The object must - * not be destroyed before the Address_Registration_Guard is - * destroyed. - */ - Address_Registration_Guard(Object* object); - - //! Unregister an address/Object with the GC. - /*! Destruct an Address_Registration_Guard. The address registered - * with the Address_Registration_Guard when it was constructed will - * be unregistered from the GC. - */ - ~Address_Registration_Guard(); - - // Disable copying - Address_Registration_Guard(Address_Registration_Guard const& other) = delete; - Address_Registration_Guard& operator=(Address_Registration_Guard const& other) = delete; - - // Enable moving - Address_Registration_Guard(Address_Registration_Guard&& other); - Address_Registration_Guard& operator=(Address_Registration_Guard&& other); - - //! Get the address that is registered with the GC. - VALUE* address() const; - - /** Called during Ruby's exit process since we should not call - * rb_gc unregister_address there - */ - static void disable(); - - private: - inline static bool enabled = true; - inline static bool exit_handler_registered = false; - static void registerExitHandler(); - - private: - void registerAddress() const; - void unregisterAddress(); - - VALUE* address_ = nullptr; - }; -} // namespace Rice - -#endif // Rice__Address_Registration_Guard__hpp_ \ No newline at end of file diff --git a/rice/Address_Registration_Guard.ipp b/rice/Address_Registration_Guard.ipp deleted file mode 100644 index 33526353..00000000 --- a/rice/Address_Registration_Guard.ipp +++ /dev/null @@ -1,80 +0,0 @@ -namespace Rice -{ - inline Address_Registration_Guard::Address_Registration_Guard(VALUE* address) : address_(address) - { - registerExitHandler(); - registerAddress(); - } - - inline Address_Registration_Guard::Address_Registration_Guard(Object* object) - : address_(const_cast(&object->value())) - { - registerExitHandler(); - registerAddress(); - } - - inline Address_Registration_Guard::~Address_Registration_Guard() - { - unregisterAddress(); - } - - inline Address_Registration_Guard::Address_Registration_Guard(Address_Registration_Guard&& other) - { - // We don't use the constructor because we don't want to double register this address - address_ = other.address_; - other.address_ = nullptr; - } - - inline Address_Registration_Guard& Address_Registration_Guard::operator=(Address_Registration_Guard&& other) - { - this->unregisterAddress(); - - this->address_ = other.address_; - other.address_ = nullptr; - return *this; - } - - inline void Address_Registration_Guard::registerAddress() const - { - if (enabled) - { - detail::protect(rb_gc_register_address, address_); - } - } - - inline void Address_Registration_Guard::unregisterAddress() - { - if (enabled && address_) - { - detail::protect(rb_gc_unregister_address, address_); - } - - address_ = nullptr; - } - - inline VALUE* Address_Registration_Guard::address() const - { - return address_; - } - - static void disable_all_guards(VALUE) - { - Address_Registration_Guard::disable(); - } - - inline void Address_Registration_Guard::registerExitHandler() - { - if (exit_handler_registered) - { - return; - } - - detail::protect(rb_set_end_proc, &disable_all_guards, Qnil); - exit_handler_registered = true; - } - - inline void Address_Registration_Guard::disable() - { - enabled = false; - } -} // Rice \ No newline at end of file diff --git a/rice/Pin.hpp b/rice/Pin.hpp new file mode 100644 index 00000000..7a30500a --- /dev/null +++ b/rice/Pin.hpp @@ -0,0 +1,42 @@ +#ifndef Rice__Pin__hpp_ +#define Rice__Pin__hpp_ + +namespace Rice +{ + //! Strong lifetime policy for a Ruby VALUE. + /*! + * Pin represents a Ruby VALUE whose lifetime is explicitly + * extended by C++ code. + * + * Internally, Pin uses a GC Anchor to keep the VALUE alive. + * Copying a Pin shares the underlying anchor; moving is cheap. + * + * This type is intended for C++-owned objects that store Ruby + * values but are not themselves owned by Ruby and thus do not + * participate in the GC via a mark function. + */ + class Pin + { + public: + //! Construct a strong pin for a Ruby VALUE. + explicit Pin(VALUE value); + + // Shared-anchor semantics. + Pin(const Pin&) = default; + Pin& operator=(const Pin&) = default; + + Pin(Pin&&) noexcept = default; + Pin& operator=(Pin&&) noexcept = default; + + //! Replace the pinned Ruby VALUE. + void set(VALUE value); + + //! Retrieve the pinned Ruby VALUE. + VALUE get() const; + + private: + //! Shared ownership of the internal GC anchor. + std::shared_ptr anchor_; + }; +} +#endif // Rice__Pin__hpp_ diff --git a/rice/Pin.ipp b/rice/Pin.ipp new file mode 100644 index 00000000..e72c0ce6 --- /dev/null +++ b/rice/Pin.ipp @@ -0,0 +1,17 @@ +namespace Rice +{ + inline Pin::Pin(VALUE value) + : anchor_(std::make_shared(value)) + { + } + + inline void Pin::set(VALUE value) + { + this->anchor_->set(value); + } + + inline VALUE Pin::get() const + { + return this->anchor_->get(); + } +} diff --git a/rice/cpp_api/Object.hpp b/rice/cpp_api/Object.hpp index 96f7ea3e..4ea62ed9 100644 --- a/rice/cpp_api/Object.hpp +++ b/rice/cpp_api/Object.hpp @@ -52,9 +52,7 @@ namespace Rice operator VALUE() const { return value_; } //! Explicitly get the encapsulated VALUE. - // Returns a const ref so that Address_Registration_Guard can access - // the address where the VALUE is stored - VALUE const volatile& value() const { return value_; } + VALUE value() const { return value_; } //! Get the class of an object. /*! \return the object's Class. diff --git a/rice/detail/Anchor.hpp b/rice/detail/Anchor.hpp new file mode 100644 index 00000000..bf7c59cb --- /dev/null +++ b/rice/detail/Anchor.hpp @@ -0,0 +1,58 @@ +#ifndef Rice__detail__Anchor__hpp_ +#define Rice__detail__Anchor__hpp_ + +#include + +namespace Rice +{ + namespace detail + { + //! Internal GC anchor for a Ruby VALUE. + /*! + * Anchor is a low-level adapter around the Ruby GC API. + * It owns a stable VALUE slot whose address is registered + * with the Ruby garbage collector. + * + * This class encapsulates all GC registration logic and is + * not part of the public Rice API. + */ + class Anchor + { + public: + //! Construct an anchor for the given Ruby VALUE. + /*! + * The address of the internal VALUE is registered with the + * Ruby GC, preventing collection while this Anchor exists. + */ + explicit Anchor(VALUE value); + + //! Unregister the VALUE from the Ruby GC. + ~Anchor(); + + Anchor(const Anchor&) = delete; + Anchor& operator=(const Anchor&) = delete; + + //! Retrieve the currently anchored VALUE. + VALUE get() const; + + //! Replace the anchored VALUE. + /*! + * The GC root (address) remains unchanged; only the VALUE + * stored at that address is updated. + */ + void set(VALUE value); + + private: + static void disable(VALUE); + static void registerExitHandler(); + + inline static bool enabled_ = true; + inline static bool exitHandlerRegistered_ = false; + + private: + //! GC-visible Ruby VALUE slot. + VALUE value_; + }; + } +} +#endif // Rice__detail__Anchor__hpp_ diff --git a/rice/detail/Anchor.ipp b/rice/detail/Anchor.ipp new file mode 100644 index 00000000..bc9df57d --- /dev/null +++ b/rice/detail/Anchor.ipp @@ -0,0 +1,44 @@ +namespace Rice +{ + namespace detail + { + inline Anchor::Anchor(VALUE value) : value_(value) + { + Anchor::registerExitHandler(); + detail::protect(rb_gc_register_address, &this->value_); + } + + inline Anchor::~Anchor() + { + if (Anchor::enabled_) + { + detail::protect(rb_gc_unregister_address, &this->value_); + } + } + + inline VALUE Anchor::get() const + { + return this->value_; + } + + inline void Anchor::set(VALUE value) + { + this->value_ = value; + } + + // This will be called by ruby at exit - we want to disable further unregistering + inline void Anchor::disable(VALUE) + { + Anchor::enabled_ = false; + } + + inline void Anchor::registerExitHandler() + { + if (!Anchor::exitHandlerRegistered_) + { + detail::protect(rb_set_end_proc, &Anchor::disable, Qnil); + Anchor::exitHandlerRegistered_ = true; + } + } + } +} \ No newline at end of file diff --git a/rice/detail/NativeCallback.hpp b/rice/detail/NativeCallback.hpp index b5978d60..39cf55b2 100644 --- a/rice/detail/NativeCallback.hpp +++ b/rice/detail/NativeCallback.hpp @@ -10,6 +10,11 @@ namespace Rice::detail template class NativeCallback; + // NativeCallback instances are never freed because there is no way for us to know + // when they can be freed. At the same time, the Pin prevents the Ruby proc from being + // garbage collected, which is necessary because C code may call the callback + // at any time. This supports passing blocks to C callbacks without requiring the Ruby + // user to manually hold a reference to a proc. template class NativeCallback : public Native { @@ -18,8 +23,6 @@ namespace Rice::detail using NativeCallback_T = NativeCallback; using Tuple_T = std::tuple; - static VALUE finalizerCallback(VALUE yielded_arg, VALUE callback_arg, int argc, const VALUE* argv, VALUE blockarg); - template static void define(Arg_Ts&& ...args); @@ -59,7 +62,7 @@ namespace Rice::detail template Return_T callRuby(std::index_sequence& indices, Parameter_Ts...args); - VALUE proc_; + Pin proc_; From_Ruby fromRuby_; #ifdef HAVE_LIBFFI diff --git a/rice/detail/NativeCallback.ipp b/rice/detail/NativeCallback.ipp index 93e30a11..2be85e03 100644 --- a/rice/detail/NativeCallback.ipp +++ b/rice/detail/NativeCallback.ipp @@ -160,14 +160,6 @@ namespace Rice::detail return std::forward_as_tuple(extractArg(args[I])...); } - template - VALUE NativeCallback::finalizerCallback(VALUE, VALUE callback_arg, int, const VALUE*, VALUE) - { - NativeCallback_T* nativeCallback = (NativeCallback_T*)callback_arg; - delete nativeCallback; - return Qnil; - } - template template inline void NativeCallback::define(Arg_Ts&& ...args) @@ -185,10 +177,6 @@ namespace Rice::detail Native("callback", copyReturnInfo(), copyParameters()), proc_(proc), fromRuby_(returnInfo_.get()) { - // Tie the lifetime of the NativeCallback C++ instance to the lifetime of the Ruby proc object - VALUE finalizer = rb_proc_new(NativeCallback_T::finalizerCallback, (VALUE)this); - rb_define_finalizer(proc, finalizer); - #ifdef HAVE_LIBFFI // First setup description of callback if (cif_.bytes == 0) @@ -215,8 +203,6 @@ namespace Rice::detail NativeCallback_T::callback_ = nullptr; NativeCallback_T::native_ = nullptr; - - this->proc_ = Qnil; } template @@ -235,7 +221,7 @@ namespace Rice::detail convertToRuby(args)... }; static Identifier id("call"); - VALUE result = detail::protect(rb_funcallv, this->proc_, id.id(), (int)sizeof...(Parameter_Ts), values.data()); + VALUE result = detail::protect(rb_funcallv, this->proc_.get(), id.id(), (int)sizeof...(Parameter_Ts), values.data()); if constexpr (!std::is_void_v) { return this->fromRuby_.convert(result); diff --git a/rice/rice.hpp b/rice/rice.hpp index 2a241cf1..1d0875e2 100644 --- a/rice/rice.hpp +++ b/rice/rice.hpp @@ -117,6 +117,10 @@ #include "detail/NativeMethod.ipp" #include "detail/NativeProc.hpp" #include "detail/NativeProc.ipp" +#include "detail/Anchor.hpp" +#include "detail/Anchor.ipp" +#include "Pin.hpp" +#include "Pin.ipp" #include "detail/NativeCallback.hpp" #include "detail/NativeCallback.ipp" #include "detail/Proc.ipp" @@ -134,9 +138,6 @@ #include "cpp_api/Class.ipp" #include "cpp_api/Struct.hpp" #include "cpp_api/Struct.ipp" - -#include "Address_Registration_Guard.hpp" -#include "Address_Registration_Guard.ipp" #include "global_function.hpp" #include "global_function.ipp" diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e48813c9..c2ed8de1 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -8,13 +8,15 @@ project(unittest LANGUAGES CXX) add_executable(${PROJECT_NAME}) # ----- Dependencies ------ -#target_include_directories(${PROJECT_NAME} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) # Ruby -#find_package(Ruby Required) include("../FindRuby.cmake") target_link_libraries(${PROJECT_NAME} PRIVATE Ruby::Ruby) +# Rice +include("../Rice.cmake") +target_link_libraries(${PROJECT_NAME} PRIVATE Rice::Rice) + # FFI if (MSVC) # This will include Clang on Windows find_package(unofficial-libffi CONFIG REQUIRED) @@ -33,15 +35,17 @@ else() endif() target_compile_definitions(${PROJECT_NAME} PRIVATE HAVE_LIBFFI) -# Rice -target_include_directories(${PROJECT_NAME} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/..) +# ----- Defines ------ +target_compile_definitions(${PROJECT_NAME} PRIVATE + $<$:RICE_DEBUG> + $<$:RICE_RELEASE> +) # ----- Sources ------ # Add source to this project's executable. target_sources(${PROJECT_NAME} PRIVATE "unittest.cpp" "embed_ruby.cpp" - "test_Address_Registration_Guard.cpp" "test_Array.cpp" "test_Attribute.cpp" "test_Buffer.cpp" @@ -72,6 +76,7 @@ target_sources(${PROJECT_NAME} PRIVATE "test_Object.cpp" "test_Overloads.cpp" "test_Ownership.cpp" + "test_Pin.cpp" "test_Proc.cpp" "test_Reference.cpp" "test_Self.cpp" diff --git a/test/test_Address_Registration_Guard.cpp b/test/test_Address_Registration_Guard.cpp deleted file mode 100644 index 8c026f62..00000000 --- a/test/test_Address_Registration_Guard.cpp +++ /dev/null @@ -1,62 +0,0 @@ -#include "unittest.hpp" -#include "embed_ruby.hpp" -#include - -using namespace Rice; - -TESTSUITE(Address_Registration_Guard); - -SETUP(Address_Registration_Guard) -{ - embed_ruby(); -} - -TEARDOWN(Address_Registration_Guard) -{ - rb_gc_start(); -} - -TESTCASE(register_address) -{ - VALUE v = Qnil; - Address_Registration_Guard g(&v); -} - -TESTCASE(register_object) -{ - Object o; - Address_Registration_Guard g(&o); -} - -TESTCASE(get_address) -{ - VALUE v = Qnil; - Address_Registration_Guard g(&v); - ASSERT_EQUAL(&v, g.address()); -} - -TESTCASE(move_construct) -{ - VALUE value = detail::to_ruby("Value 1"); - - Address_Registration_Guard guard1(&value); - Address_Registration_Guard guard2(std::move(guard1)); - - ASSERT((guard1.address() == nullptr)); - ASSERT_EQUAL(&value, guard2.address()); -} - -TESTCASE(move_assign) -{ - VALUE value1 = detail::to_ruby("Value 1"); - VALUE value2 = detail::to_ruby("Value 2"); - - Address_Registration_Guard guard1(&value1); - Address_Registration_Guard guard2(&value2); - - guard2 = std::move(guard1); - - ASSERT((guard1.address() == nullptr)); - ASSERT_EQUAL(&value1, guard2.address()); -} - diff --git a/test/test_Pin.cpp b/test/test_Pin.cpp new file mode 100644 index 00000000..cdeb3bb1 --- /dev/null +++ b/test/test_Pin.cpp @@ -0,0 +1,97 @@ +#include "unittest.hpp" +#include "embed_ruby.hpp" +#include + +using namespace Rice; + +TESTSUITE(Pin); + +SETUP(Pin) +{ + embed_ruby(); +} + +TEARDOWN(Pin) +{ + rb_gc_start(); +} + +namespace +{ + // A C++ class that holds a Ruby VALUE as a member + class Container + { + public: + Container(VALUE value) : pin_(value) + { + } + + VALUE getValue() const + { + return pin_.get(); + } + + void setValue(VALUE value) + { + pin_.set(value); + } + + private: + Pin pin_; + }; +} + +TESTCASE(prevents_gc_collection) +{ + Container* container = nullptr; + + // Create container with a Ruby string in a nested scope + { + VALUE str = rb_str_new_cstr("prevent gc test"); + container = new Container(str); + } + + // Force garbage collection - the string should survive + rb_gc_start(); + + // Verify the string is still valid and accessible + VALUE retrieved = container->getValue(); + ASSERT_EQUAL("prevent gc test", detail::From_Ruby().convert(retrieved)); + + delete container; +} + +TESTCASE(update_pinned_value) +{ + Container container(rb_str_new_cstr("first")); + + rb_gc_start(); + ASSERT_EQUAL("first", detail::From_Ruby().convert(container.getValue())); + + container.setValue(rb_str_new_cstr("second")); + + rb_gc_start(); + ASSERT_EQUAL("second", detail::From_Ruby().convert(container.getValue())); +} + +TESTCASE(copy_container) +{ + Container c1(rb_str_new_cstr("shared value")); + Container c2 = c1; + + rb_gc_start(); + + // Both containers share the same anchor + ASSERT_EQUAL("shared value", detail::From_Ruby().convert(c1.getValue())); + ASSERT_EQUAL("shared value", detail::From_Ruby().convert(c2.getValue())); +} + +TESTCASE(move_container) +{ + Container c1(rb_str_new_cstr("moved value")); + Container c2 = std::move(c1); + + rb_gc_start(); + + ASSERT_EQUAL("moved value", detail::From_Ruby().convert(c2.getValue())); +} diff --git a/test/test_Proc.cpp b/test/test_Proc.cpp index f39e9dd9..6250500c 100644 --- a/test/test_Proc.cpp +++ b/test/test_Proc.cpp @@ -20,7 +20,7 @@ TEARDOWN(Proc) namespace { - int squareWithBlock(int i, VALUE block) + int squareWithBlock(int i, VALUE /*block*/) { // You must include a value parameter at the end that will be set to a proc // created from the passed in block. You can actually ingore the proc and yield