Skip to content

Commit d67fec5

Browse files
committed
Patch get_gc in-place on the shared handlers struct
Replacing the per-class handlers pointer with a copy broke ext-php-rs's FromZval, which uses the handlers pointer as a class-identity key. Patch the original struct in place instead — its memory is a heap Box::leak the extension owns, so the write is safe, and every native_ast lookup keeps working because the pointer identity is preserved. Idempotent guard via a process-level boolean; only the first AST does the write.
1 parent a5fe11f commit d67fec5

1 file changed

Lines changed: 31 additions & 30 deletions

File tree

  • packages/php-ext-wp-mysql-parser/src

packages/php-ext-wp-mysql-parser/src/lib.rs

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,13 +1240,12 @@ fn native_ast(native_ast: &Zval) -> PhpResult<&WpMySqlNativeAst> {
12401240
.ok_or_else(|| php_error("Missing native AST handle"))
12411241
}
12421242

1243-
/// Storage for the customised `zend_object_handlers` for `WP_MySQL_Native_Ast`.
1244-
///
1245-
/// We can't mutate the static handlers struct ext-php-rs registers, so we
1246-
/// clone it on startup, patch `get_gc`, and point the class entry's
1247-
/// `default_object_handlers` at this owned copy. Lives for the duration
1248-
/// of the module — written exactly once during MINIT.
1249-
static mut WP_MYSQL_NATIVE_AST_HANDLERS: Option<zend_object_handlers> = None;
1243+
/// Tracks whether the GC handler has been installed on `WP_MySQL_Native_Ast`'s
1244+
/// shared handlers struct. We patch in place rather than swapping the
1245+
/// handlers pointer because ext-php-rs's `FromZval` for registered
1246+
/// classes verifies identity via the handlers pointer; replacing it
1247+
/// would make every native_ast lookup fail.
1248+
static mut WP_MYSQL_NATIVE_AST_HANDLERS_PATCHED: bool = false;
12501249

12511250
/// Custom `get_gc` handler that exposes `WpMySqlNativeAst.node_cache` to
12521251
/// PHP's cycle collector.
@@ -1305,26 +1304,27 @@ unsafe extern "C" fn ast_get_gc(
13051304
}
13061305

13071306
/// Patch a freshly-constructed `WP_MySQL_Native_Ast` instance so its
1308-
/// `zend_object.handlers->get_gc` walks the Rust-side cache.
1307+
/// shared `zend_object_handlers->get_gc` walks the Rust-side cache.
13091308
///
13101309
/// PHP 8.3 introduced `zend_class_entry.default_object_handlers` which
1311-
/// would let us patch once per class on MINIT, but PHP 8.2 has no such
1312-
/// field; the only reliable place to install a custom `get_gc` across
1313-
/// both is per-instance, right after the object is allocated. The
1314-
/// patched handlers struct itself is computed lazily on first use and
1315-
/// shared across all subsequent ASTs.
1310+
/// would let us configure handlers once per class on MINIT, but PHP 8.2
1311+
/// has no such field. Within ext-php-rs the per-class handlers struct
1312+
/// is heap-allocated (via Box::leak), shared across all instances of
1313+
/// the class, and identified by its pointer. Swapping the pointer to a
1314+
/// copy would break ext-php-rs's `FromZval` identity check, so we
1315+
/// instead set `get_gc` in place on the original struct. We only need
1316+
/// to do it once per process; subsequent calls are cheap.
13161317
unsafe fn install_gc_handler_for(obj: *mut zend_object) {
1317-
if WP_MYSQL_NATIVE_AST_HANDLERS.is_none() {
1318-
let original = (*obj).handlers;
1319-
if original.is_null() {
1320-
return;
1321-
}
1322-
let mut patched = std::ptr::read(original);
1323-
patched.get_gc = Some(ast_get_gc);
1324-
WP_MYSQL_NATIVE_AST_HANDLERS = Some(patched);
1318+
if WP_MYSQL_NATIVE_AST_HANDLERS_PATCHED {
1319+
return;
1320+
}
1321+
let handlers = (*obj).handlers;
1322+
if handlers.is_null() {
1323+
return;
13251324
}
1326-
let stored = WP_MYSQL_NATIVE_AST_HANDLERS.as_ref().unwrap();
1327-
(*obj).handlers = stored as *const _;
1325+
let handlers_mut = handlers as *mut zend_object_handlers;
1326+
(*handlers_mut).get_gc = Some(ast_get_gc);
1327+
WP_MYSQL_NATIVE_AST_HANDLERS_PATCHED = true;
13281328
}
13291329

13301330
/// Build a Zval that references an existing PHP object with refcount bumped.
@@ -1943,13 +1943,14 @@ impl WpMySqlNativeParser {
19431943
.into_zval(false)
19441944
.map_err(php_error)?;
19451945
let native_ast = native_ast(&native_ast_zval)?;
1946-
// GC handler install temporarily disabled to localize a CI failure.
1947-
// unsafe {
1948-
// let obj_ref = native_ast_zval
1949-
// .object()
1950-
// .ok_or_else(|| php_error("Native AST zval is not an object"))?;
1951-
// install_gc_handler_for((obj_ref as *const ZendObject) as *mut zend_object);
1952-
// }
1946+
// Install our custom `get_gc` so PHP's cycle collector can
1947+
// see the cached wrappers held by the Rust-side HashMap.
1948+
unsafe {
1949+
let obj_ref = native_ast_zval
1950+
.object()
1951+
.ok_or_else(|| php_error("Native AST zval is not an object"))?;
1952+
install_gc_handler_for((obj_ref as *const ZendObject) as *mut zend_object);
1953+
}
19531954
native_ast.arena.create_php_ast(&native_ast_zval)
19541955
})
19551956
}

0 commit comments

Comments
 (0)