ext/pdo: Convert def_stmt_ctor_args field from zval to HashTable pointer

[Girgias]

Attempt number 3 on #12154

I think the reason I was getting the double free/inconsistent HT was because I didn't set the pointer to the ctor_args to NULL in the gc handler.

[nielsdos]

You probably didn't understand the root cause of the issue properly as this is almost certainly wrong.

[nielsdos]

This is indeed wrong. If cycles are registered by not collected then you lose the link to the ctor_args and future invocations may have wrong results.

The problem is that on shutdown the cycles are collected, the array is destroyed first, and then again in free_storage. You can solve this by using a dtor:

diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c
index be24b23a5ad..8566f58b789 100644
--- a/ext/pdo/pdo_dbh.c
+++ b/ext/pdo/pdo_dbh.c
@@ -1425,7 +1425,6 @@ static HashTable *dbh_get_gc(zend_object *object, zval **gc_data, int *gc_count)
 	zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create();
 	if (dbh->def_stmt_ctor_args != NULL) {
 		zend_get_gc_buffer_add_ht(gc_buffer, dbh->def_stmt_ctor_args);
-		dbh->def_stmt_ctor_args = NULL;
 	}
 	if (dbh->methods && dbh->methods->get_gc) {
 		dbh->methods->get_gc(dbh, gc_buffer);
@@ -1438,6 +1437,18 @@ static zend_object_handlers pdo_dbh_object_handlers;
 
 static void pdo_dbh_free_storage(zend_object *std);
 
+static void pdo_dbh_dtor(zend_object *object)
+{
+	pdo_dbh_t *dbh = php_pdo_dbh_fetch_inner(object);
+
+	zend_objects_destroy_object(object);
+
+	if (dbh->def_stmt_ctor_args != NULL) {
+		zend_array_release(dbh->def_stmt_ctor_args);
+		dbh->def_stmt_ctor_args = NULL;
+	}
+}
+
 void pdo_dbh_init(int module_number)
 {
 	pdo_dbh_ce = register_class_PDO();
@@ -1447,6 +1458,7 @@ void pdo_dbh_init(int module_number)
 	memcpy(&pdo_dbh_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
 	pdo_dbh_object_handlers.offset = XtOffsetOf(pdo_dbh_object_t, std);
 	pdo_dbh_object_handlers.free_obj = pdo_dbh_free_storage;
+	pdo_dbh_object_handlers.dtor_obj = pdo_dbh_dtor;
 	pdo_dbh_object_handlers.clone_obj = NULL;
 	pdo_dbh_object_handlers.get_method = dbh_method_get;
 	pdo_dbh_object_handlers.compare = zend_objects_not_comparable;
@@ -1490,11 +1502,6 @@ static void dbh_free(pdo_dbh_t *dbh, bool free_persistent)
 		pefree((char *)dbh->persistent_id, dbh->is_persistent);
 	}
 
-	if (dbh->def_stmt_ctor_args != NULL) {
-		zend_array_release(dbh->def_stmt_ctor_args);
-		dbh->def_stmt_ctor_args = NULL;
-	}
-
 	for (i = 0; i < PDO_DBH_DRIVER_METHOD_KIND__MAX; i++) {
 		if (dbh->cls_methods[i]) {
 			zend_hash_destroy(dbh->cls_methods[i]);

Not sure if the size save is worth the complexity though.