PDO - Integrate regular resource with persistent connection for end-of-request hook to invoke persistent shutdown and rollback.

Rework connectivity checks to generate SQLSTATE error 01002 according to error mode, rather than always throw error.

Author: rposky

ext/pdo/pdo.c

@@ -247,7 +247,7 @@ PHP_MINIT_FUNCTION(pdo)
 
 	zend_hash_init(&pdo_driver_hash, 0, NULL, NULL, 1);
 
-	le_ppdo = zend_register_list_destructors_ex(NULL, php_pdo_pdbh_dtor,
+	le_ppdo = zend_register_list_destructors_ex(php_pdo_pdbh_request_dtor, php_pdo_pdbh_dtor,
 		"PDO persistent database", module_number);
 
 	pdo_exception_ce = register_class_PDOException(spl_ce_RuntimeException);

ext/pdo/pdo_dbh.c

@@ -313,6 +313,7 @@ PHP_METHOD(PDO, __construct)
 		int plen = 0;
 		char *hashkey = NULL;
 		pdo_dbh_t *pdbh = NULL;
+		pdo_dbh_t **pdbh_ref = NULL;
 		zval *v;
 
 		if ((v = zend_hash_index_find_deref(Z_ARRVAL_P(options), PDO_ATTR_PERSISTENT)) != NULL) {
@@ -338,9 +339,22 @@ PHP_METHOD(PDO, __construct)
 			/* is the connection still alive ? */
 			if (!pdbh || pdbh->is_closed ||
 				(pdbh->methods->check_liveness && FAILURE == (pdbh->methods->check_liveness)(pdbh))) {
+				/* clean up prior dbh reference */
+				if (pdbh && pdbh->persistent_resource) {
+					pdbh_ref = (pdo_dbh_t**)pdbh->persistent_resource->ptr;
+					/* clear dbh reference to forestall end-of-request actions in destructor */
+					*pdbh_ref = NULL;
+					zend_list_delete(pdbh->persistent_resource);
+					pdbh->persistent_resource = NULL;
+				}
 				/* need a brand new pdbh */
 				pdbh = pecalloc(1, sizeof(*pdbh), 1);
-
+				pdbh_ref = emalloc(sizeof(*pdbh_ref));
+				*pdbh_ref = pdbh;
+				pdbh->persistent_resource = zend_register_resource(pdbh_ref, php_pdo_list_entry());
+				if (!pdbh->persistent_resource) {
+					php_error_docref(NULL, E_ERROR, "Failed to register resource entry");
+				}
 				pdbh->is_persistent = 1;
 				pdbh->persistent_id = pemalloc(plen + 1, 1);
 				memcpy((char *)pdbh->persistent_id, hashkey, plen+1);
@@ -394,7 +408,7 @@ PHP_METHOD(PDO, __construct)
 			/* we should also need to replace the object store entry,
 			   since it was created with emalloc */
 			/* if a resource is already registered, then it failed a liveness check
-			 * and will be replaced, prompting destruct. */
+			   and will be replaced, prompting destruct. */
 			if ((zend_register_persistent_resource(
 						(char*)dbh->persistent_id, dbh->persistent_id_len, dbh, php_pdo_list_entry())) == NULL) {
 				php_error_docref(NULL, E_ERROR, "Failed to register persistent entry");
@@ -517,6 +531,8 @@ PHP_METHOD(PDO, prepare)
 
 	PDO_DBH_CLEAR_ERR();
 
+	PDO_CLOSE_CHECK;
+
 	if (options && (value = zend_hash_index_find(Z_ARRVAL_P(options), PDO_ATTR_STATEMENT_CLASS)) != NULL) {
 		if (Z_TYPE_P(value) != IS_ARRAY) {
 			zend_type_error("PDO::ATTR_STATEMENT_CLASS value must be of type array, %s given",
@@ -604,6 +620,8 @@ PHP_METHOD(PDO, beginTransaction)
 
 	PDO_CONSTRUCT_CHECK;
 
+	PDO_CLOSE_CHECK;
+
 	if (pdo_is_in_transaction(dbh)) {
 		zend_throw_exception_ex(php_pdo_get_exception(), 0, "There is already an active transaction");
 		RETURN_THROWS();
@@ -634,6 +652,8 @@ PHP_METHOD(PDO, commit)
 
 	PDO_CONSTRUCT_CHECK;
 
+	PDO_CLOSE_CHECK;
+
 	if (!pdo_is_in_transaction(dbh)) {
 		zend_throw_exception_ex(php_pdo_get_exception(), 0, "There is no active transaction");
 		RETURN_THROWS();
@@ -658,6 +678,8 @@ PHP_METHOD(PDO, rollBack)
 
 	PDO_CONSTRUCT_CHECK;
 
+	PDO_CLOSE_CHECK;
+
 	if (!pdo_is_in_transaction(dbh)) {
 		zend_throw_exception_ex(php_pdo_get_exception(), 0, "There is no active transaction");
 		RETURN_THROWS();
@@ -682,6 +704,8 @@ PHP_METHOD(PDO, inTransaction)
 
 	PDO_CONSTRUCT_CHECK;
 
+	PDO_CLOSE_CHECK;
+
 	RETURN_BOOL(pdo_is_in_transaction(dbh));
 }
 /* }}} */
@@ -693,6 +717,8 @@ PHP_METHOD(PDO, isConnected)
 
 	ZEND_PARSE_PARAMETERS_NONE();
 
+	PDO_CONSTRUCT_CHECK;
+
 	RETURN_BOOL(!dbh->is_closed);
 }
 /* }}} */
@@ -900,6 +926,7 @@ PHP_METHOD(PDO, setAttribute)
 
 	PDO_DBH_CLEAR_ERR();
 	PDO_CONSTRUCT_CHECK;
+	PDO_CLOSE_CHECK;
 
 	RETURN_BOOL(pdo_dbh_attribute_set(dbh, attr, value));
 }
@@ -954,6 +981,12 @@ PHP_METHOD(PDO, getAttribute)
 			break;
 	}
 
+	if (!dbh->methods) {
+		pdo_raise_impl_error(dbh, NULL, "IM001",
+			"driver attributes not initialized, possibly due to disconnect");
+		RETURN_FALSE;
+	}
+
 	if (!dbh->methods->get_attribute) {
 		pdo_raise_impl_error(dbh, NULL, "IM001", "driver does not support getting attributes");
 		RETURN_FALSE;
@@ -994,6 +1027,8 @@ PHP_METHOD(PDO, exec)
 
 	PDO_DBH_CLEAR_ERR();
 	PDO_CONSTRUCT_CHECK;
+	PDO_CLOSE_CHECK;
+
 	ret = dbh->methods->doer(dbh, statement);
 	if (ret == -1) {
 		PDO_HANDLE_DBH_ERR();
@@ -1020,6 +1055,8 @@ PHP_METHOD(PDO, lastInsertId)
 
 	PDO_DBH_CLEAR_ERR();
 
+	PDO_CLOSE_CHECK;
+
 	if (!dbh->methods->last_id) {
 		pdo_raise_impl_error(dbh, NULL, "IM001", "driver does not support lastInsertId()");
 		RETURN_FALSE;
@@ -1040,6 +1077,8 @@ PHP_METHOD(PDO, errorCode)
 
 	ZEND_PARSE_PARAMETERS_NONE();
 
+	PDO_CONSTRUCT_CHECK;
+
 	if (dbh->query_stmt) {
 		RETURN_STRING(dbh->query_stmt->error_code);
 	}
@@ -1067,6 +1106,8 @@ PHP_METHOD(PDO, errorInfo)
 
 	ZEND_PARSE_PARAMETERS_NONE();
 
+	PDO_CONSTRUCT_CHECK;
+
 	array_init(return_value);
 
 	if (dbh->query_stmt) {
@@ -1127,6 +1168,8 @@ PHP_METHOD(PDO, query)
 
 	PDO_DBH_CLEAR_ERR();
 
+	PDO_CLOSE_CHECK;
+
 	if (!pdo_stmt_instantiate(dbh, return_value, dbh->def_stmt_ce, &dbh->def_stmt_ctor_args)) {
 		RETURN_THROWS();
 	}
@@ -1194,6 +1237,9 @@ PHP_METHOD(PDO, quote)
 	PDO_CONSTRUCT_CHECK;
 
 	PDO_DBH_CLEAR_ERR();
+
+	PDO_CLOSE_CHECK;
+
 	if (!dbh->methods->quoter) {
 		pdo_raise_impl_error(dbh, NULL, "IM001", "driver does not support quoting");
 		RETURN_FALSE;
@@ -1379,11 +1425,6 @@ void pdo_dbh_init(int module_number)
 /* Disconnect from the database and free associated driver. */
 static void dbh_shutdown(pdo_dbh_t *dbh)
 {
-	if (dbh->driver_data && dbh->methods && dbh->methods->rollback && pdo_is_in_transaction(dbh)) {
-		dbh->methods->rollback(dbh);
-		dbh->in_txn = false;
-	}
-
 	if (dbh->methods) {
 		dbh->methods->closer(dbh);
 	}
@@ -1442,6 +1483,11 @@ static void dbh_free(pdo_dbh_t *dbh)
 		pefree((char *)dbh->persistent_id, dbh->is_persistent);
 	}
 
+	if (dbh->persistent_resource) {
+		dbh->persistent_resource->ptr = NULL;
+		dbh->persistent_resource = NULL;
+	}
+
 	if (!Z_ISUNDEF(dbh->def_stmt_ctor_args)) {
 		zval_ptr_dtor(&dbh->def_stmt_ctor_args);
 	}
@@ -1475,10 +1521,6 @@ static void pdo_dbh_free_storage(zend_object *std)
 
 	/* dbh might be null if we OOMed during object initialization. */
 	if (dbh) {
-		if (dbh->is_persistent && dbh->methods && dbh->methods->persistent_shutdown) {
-			dbh->methods->persistent_shutdown(dbh);
-		}
-
 		/* stmt is not persistent, even if dbh is, so it must be freed with pdo.
 		 * Consider copying stmt error code to dbh at this point, seemingly the reason
 		 * that the stmt is even being held, or even better, to do that at the time of
@@ -1488,12 +1530,14 @@ static void pdo_dbh_free_storage(zend_object *std)
 			dbh->query_stmt = NULL;
 		}
 
-		/* a persisted dbh will be freed when the resource is destructed. */
-		if (!(--dbh->refcount) && !pdo_is_persisted(dbh)) {
-			if (!dbh->is_closed) {
-				dbh_shutdown(dbh);
+		if (!(--dbh->refcount)) {
+			/* a persisted dbh will be freed when the resource is destructed. */
+			if (!pdo_is_persisted(dbh)) {
+				if (!dbh->is_closed) {
+					dbh_shutdown(dbh);
+				}
+				dbh_free(dbh);
 			}
-			dbh_free(dbh);
 		}
 	}
 
@@ -1517,6 +1561,27 @@ zend_object *pdo_dbh_new(zend_class_entry *ce)
 
 /* }}} */
 
+ZEND_RSRC_DTOR_FUNC(php_pdo_pdbh_request_dtor) /* {{{ */
+{
+	if (res->ptr) {
+		pdo_dbh_t **dbh_ref = (pdo_dbh_t**)res->ptr;
+		if (*dbh_ref) {
+			pdo_dbh_t *dbh = (pdo_dbh_t*)*dbh_ref;
+			if (dbh->methods && dbh->methods->persistent_shutdown) {
+				dbh->methods->persistent_shutdown(dbh);
+			}
+			if (dbh->methods && dbh->methods->rollback && pdo_is_in_transaction(dbh)) {
+				dbh->methods->rollback(dbh);
+				dbh->in_txn = false;
+			}
+			dbh->persistent_resource = NULL;
+		}
+		efree(dbh_ref);
+		res->ptr = NULL;
+	}
+}
+/* }}} */
+
 ZEND_RSRC_DTOR_FUNC(php_pdo_pdbh_dtor) /* {{{ */
 {
 	if (res->ptr) {

ext/pdo/php_pdo.h

@@ -56,14 +56,20 @@ PHP_MINFO_FUNCTION(pdo);
 #define LONG_CONST(c) (zend_long) c
 
 #define PDO_CONSTRUCT_CHECK \
-	if (dbh->is_closed) { \
-		zend_throw_exception_ex(php_pdo_get_exception(), 0, "connection is closed"); \
-		RETURN_THROWS(); \
-	} \
 	if (!dbh->driver) { \
 		zend_throw_error(NULL, "PDO object is not initialized, constructor was not called"); \
 		RETURN_THROWS(); \
 	} \
 
+#define PDO_CLOSE_CHECK \
+  if (dbh->is_closed) { \
+    pdo_raise_impl_error(dbh, NULL, "01002", NULL); \
+    if (dbh->error_mode == PDO_ERRMODE_EXCEPTION) { \
+      RETURN_THROWS(); \
+    } else { \
+      RETURN_FALSE; \
+    } \
+  } \
+
 
 #endif	/* PHP_PDO_H */

ext/pdo/php_pdo_driver.h

@@ -478,6 +478,8 @@ struct _pdo_dbh_t {
 	/* persistent hash key associated with this handle */
 	const char *persistent_id;
 	size_t persistent_id_len;
+	/* a regular resource to prompt end-of-request actions */
+	zend_resource *persistent_resource;
 
 	/* counter of _pdo_dbh_object_t referencing this handle */
 	unsigned int refcount;

ext/pdo/php_pdo_int.h

@@ -31,6 +31,7 @@ void pdo_stmt_init(void);
 extern zend_object *pdo_dbh_new(zend_class_entry *ce);
 extern const zend_function_entry pdo_dbh_functions[];
 extern zend_class_entry *pdo_dbh_ce;
+extern ZEND_RSRC_DTOR_FUNC(php_pdo_pdbh_request_dtor);
 extern ZEND_RSRC_DTOR_FUNC(php_pdo_pdbh_dtor);
 
 extern zend_object *pdo_dbstmt_new(zend_class_entry *ce);

ext/pdo/tests/pdo_040.phpt

@@ -29,35 +29,38 @@ putenv("PDOTEST_ATTR=" . serialize(array(PDO::ATTR_PERSISTENT => true)));
 $pdb3 = PDOTest::factory('PDO', false);
 var_dump($pdb3->isConnected());
 $pdb4 = PDOTest::factory('PDO', false);
+$pdb4->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
 var_dump($pdb4->isConnected());
 
 /* disconnect regular PDO, confirm other PDOs still connected */
 var_dump($db1->disconnect());
 var_dump($db1->isConnected());
-try {
-    $db1->beginTransaction();
-} catch (PDOException $e) {
-    echo $e->getMessage(), \PHP_EOL;
-}
+var_dump($db1->beginTransaction());
+var_dump($db1->errorCode());
+var_dump($db1->errorInfo());
 var_dump($db2->isConnected());
 var_dump($pdb3->isConnected());
 
 /* disconnect persistent PDO, confirm other persistent PDO disconnected */
 var_dump($pdb3->disconnect());
 var_dump($pdb3->isConnected());
 var_dump($pdb4->isConnected());
-try {
-    $pdb4->disconnect();
-} catch (PDOException $e) {
-    echo $e->getMessage(), \PHP_EOL;
-}
+var_dump($pdb4->disconnect());
 
 /* new persistent PDO should prompt new connection */
 $pdb5 = PDOTest::factory('PDO', false);
 var_dump($pdb5->isConnected());
 var_dump($pdb5->inTransaction());
 var_dump($pdb5->beginTransaction());
 
+/* new persistent connection should not be inherited */
+var_dump($pdb4->isConnected());
+try {
+	$pdb4->beginTransaction();
+} catch (PDOException $e) {
+	var_dump($e->getMessage());
+}
+
 $db1 = null;
 $db2 = null; /* trigger shutdown without explicit disconnect */
 $pdb3 = null;
@@ -76,14 +79,27 @@ bool(true)
 bool(true)
 bool(true)
 bool(false)
-connection is closed
+
+Warning: PDO::beginTransaction(): SQLSTATE[01002]: Disconnect error in %s on line %d
+bool(false)
+string(5) "01002"
+array(3) {
+  [0]=>
+  string(5) "01002"
+  [1]=>
+  NULL
+  [2]=>
+  NULL
+}
 bool(true)
 bool(true)
 bool(true)
 bool(false)
 bool(false)
-connection is closed
+bool(true)
 bool(true)
 bool(false)
 bool(true)
+bool(false)
+string(33) "SQLSTATE[01002]: Disconnect error"
 bool(true)