docs(prop): document the Exception::getMessage refcount leak

crates/macros/src/lib.rs

@@ -76,6 +76,44 @@ extern crate proc_macro;
 /// - `flags` - Sets property visibility flags, e.g. `#[php(prop, flags =
 ///   ext_php_rs::flags::PropertyFlags::Private)]`
 ///
+/// ### Known limitation: `#[php(prop)]` on owned refcounted types accessed via
+/// `Exception::getMessage`-style C methods leaks one `zend_string` per call.
+///
+/// The generated read-property handler writes a fresh `zend_string` with
+/// refcount=1 into the `rv` slot via `set_zval`. PHP's `Exception::getMessage`
+/// (and siblings such as `getFile`) read this with `zval_get_string + RETURN_STR`,
+/// which addrefs to 2 and transfers the pointer to `return_value` without
+/// changing the refcount. The stack `rv` then goes out of scope without
+/// `zval_ptr_dtor`, orphaning one refcount per call.
+///
+/// This affects any `#[php_class]` extending `\Exception` with a `#[php(prop)]`
+/// field whose type allocates a `zend_string` (e.g. `String`, `Vec<u8>` when
+/// converted to a binary string, or any `IntoZval` impl producing `IS_STRING_EX`)
+/// — most commonly when the field shadows the parent `\Exception::$message`.
+/// Direct property access (`$obj->field`) via the `FETCH_OBJ_R` opcode is
+/// **not** affected because the bytecode handler properly consumes the rv ref.
+///
+/// **Workarounds, in order of preference:**
+///
+/// 1. **Do not shadow the inherited property name.** Rename the field
+///    (e.g. `payload` instead of `message`) and expose it through a
+///    `#[php_method]` getter. The method-return path is not affected by
+///    this leak.
+///    Note: `\Exception::getMessage` is `final` in PHP, so overriding it
+///    directly via `#[php_method] fn get_message(...)` is rejected at
+///    class registration.
+///
+/// 2. **Write the value into the parent's real property slot via
+///    `zend_update_property_stringl`** (raw FFI). PHP's `getMessage`
+///    then reads from real storage through `zend_std_read_property`,
+///    bypassing the leaky `rv` path entirely. This requires dropping
+///    `#[php(prop)]` from the shadow field and populating the parent
+///    slot at construction time. See `biscuit-php` (`src/errors.rs`)
+///    for a worked example.
+///
+/// Tracked by the
+/// `prop_string_field_does_not_leak_on_repeated_get_message` regression test.
+///
 /// ## Restrictions
 ///
 /// ### No lifetime parameters

tests/src/integration/class/mod.rs

@@ -160,6 +160,50 @@ pub fn throw_class_object_exception_with_prop() -> PhpResult<i32> {
     Err(PhpException::from_class::<TestClassExtendsWithProp>("ignored".into()).with_object(zval))
 }
 
+/// Regression coverage for a refcount leak in the `#[php(prop)]` field getter
+/// when the field is an owned refcounted type (here `String`) AND the property
+/// is read via a C-level method that uses `zval_get_string` + `RETURN_STR`
+/// (e.g. `Exception::getMessage`).
+///
+/// The generated getter writes a fresh `zend_string` with refcount=1 into the
+/// `rv` slot. PHP's `getMessage` then calls `zval_get_string(prop)` which
+/// addrefs (→2) and `RETURN_STR` transfers the pointer to `return_value`
+/// without changing the refcount. When the method returns, the stack `rv`
+/// goes out of scope without being dtor'd, orphaning one refcount per call.
+/// Each `$e->getMessage()` therefore leaks a `zend_string`.
+///
+/// Surfaced first in production by biscuit-php's `DatalogException` subclasses,
+/// which declare `#[php(prop, flags = Protected)] message: String` and shadow
+/// the parent `\Exception::$message`.
+#[php_class]
+#[php(extends(ce = ce::exception, stub = "\\Exception"))]
+#[derive(Default)]
+pub struct TestExceptionMessageLeak {
+    /// Public to keep the test focused on the refcount leak rather than the
+    /// visibility-check path. The leak reproduces on any `#[php(prop)]` field
+    /// whose type allocates a `zend_string` via `IntoZval`; biscuit-php's
+    /// real-world trigger happens to use Protected, but the codegen bug is the
+    /// same shape regardless of visibility.
+    #[php(prop)]
+    pub message: String,
+}
+
+#[php_impl]
+impl TestExceptionMessageLeak {
+    pub fn __construct() -> Self {
+        Self::default()
+    }
+}
+
+#[php_function]
+pub fn throw_exception_with_message_prop() -> PhpResult<i32> {
+    let payload = TestExceptionMessageLeak {
+        message: "leak-bait message contents".to_string(),
+    };
+    let zval = ZendClassObject::new(payload).into_zval(false)?;
+    Err(PhpException::from_class::<TestExceptionMessageLeak>("ignored".into()).with_object(zval))
+}
+
 #[php_class]
 #[php(implements(ce = ce::arrayaccess, stub = "ArrayAccess"))]
 #[php(extends(ce = ce::exception, stub = "\\Exception"))]
@@ -652,9 +696,11 @@ pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
         .class::<TestChildClass>()
         .class::<TestCloneableClass>()
         .class::<TestUncloneableClass>()
+        .class::<TestExceptionMessageLeak>()
         .function(wrap_function!(test_class))
         .function(wrap_function!(throw_exception))
-        .function(wrap_function!(throw_class_object_exception_with_prop));
+        .function(wrap_function!(throw_class_object_exception_with_prop))
+        .function(wrap_function!(throw_exception_with_message_prop));
 
     #[cfg(php84)]
     let builder = builder
@@ -671,6 +717,26 @@ pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
 
 #[cfg(test)]
 mod tests {
+    /// Documents an outstanding bug in the `#[php(prop)]` field-property
+    /// getter codegen: when an owned refcounted field (e.g. `String`) is read
+    /// via the `Exception::getMessage` pattern (`zval_get_string` +
+    /// `RETURN_STR`), the getter orphans one refcount on the `rv` stack zval
+    /// per call. See the `#[php(prop)]` docs in `crates/macros/src/lib.rs`
+    /// for the full mechanic and the recommended `#[php_method]` workaround.
+    ///
+    /// Ignored because a proper fix requires either an upstream PHP patch
+    /// (`zval_ptr_dtor(&rv)` in `Exception::getMessage` when `retval == &rv`)
+    /// or mirroring `#[php(prop)]` shadow fields to the parent's real
+    /// property slot via `zend_update_property_stringl`. Run with
+    /// `cargo test -- --ignored` to reproduce.
+    #[test]
+    #[ignore = "documents the #[php(prop)] String getter leak on the Exception::getMessage path; see crate-level docs"]
+    fn prop_string_field_does_not_leak_on_repeated_get_message() {
+        assert!(crate::integration::test::run_php(
+            "class/prop_string_leak.php"
+        ));
+    }
+
     #[test]
     fn class_works() {
         assert!(crate::integration::test::run_php("class/class.php"));

tests/src/integration/class/prop_string_leak.php

@@ -0,0 +1,53 @@
+<?php
+
+// Regression coverage for a refcount leak in `#[php(prop)]` field getters
+// holding owned refcounted types (e.g. `String`).
+//
+// Mechanic: the generated getter writes a fresh `zend_string` (refcount=1)
+// into the `rv` slot via `set_zval`. PHP's `Exception::getMessage` reads it
+// with `zval_get_string(prop) + RETURN_STR`. `zval_get_string` addrefs to 2;
+// `RETURN_STR` transfers the pointer to `return_value` without changing the
+// refcount; the stack `rv` goes out of scope without `zval_ptr_dtor`. One
+// refcount is orphaned per call, leaking the underlying zend_string.
+//
+// Detection: 500 calls grow the emalloc heap by ~75KB on release builds, and
+// trigger `_zend_hash_str_add_or_update_i` assertions on debug builds. Either
+// signal fails the test. Hosted in its own PHP file so a debug-build SIGABRT
+// does not silently drop later assertions in class.php.
+//
+// Surfaced first in production by biscuit-php's `DatalogException` subclasses,
+// which declare `#[php(prop, flags = Protected)] message: String` and shadow
+// the parent `\Exception::$message`.
+
+$first_call_returned = null;
+$leak_observed = null;
+try {
+    throw_exception_with_message_prop();
+} catch (TestExceptionMessageLeak $e) {
+    $first_call_returned = $e->getMessage();
+    // Warm up so any one-shot allocations (cache slots, opcode cache,
+    // hashtable resizes) settle before measurement.
+    for ($i = 0; $i < 16; $i++) {
+        $e->getMessage();
+    }
+    gc_collect_cycles();
+    $mem_before = memory_get_usage();
+    for ($i = 0; $i < 500; $i++) {
+        $e->getMessage();
+    }
+    gc_collect_cycles();
+    $mem_after = memory_get_usage();
+    $leak_observed = $mem_after - $mem_before;
+}
+assert(
+    $first_call_returned === 'leak-bait message contents',
+    "Sanity check: one getMessage() call should return the stored message " .
+        "verbatim. Got: " . var_export($first_call_returned, true)
+);
+assert(
+    $leak_observed !== null && $leak_observed < 8192,
+    "getMessage() on a #[php(prop)] String field leaks zend_strings: 500 " .
+        "calls grew the emalloc heap by " . var_export($leak_observed, true) .
+        " bytes. Each call orphans one zend_string refcount (~150 bytes); " .
+        "with the fix this delta should be near zero."
+);