diff --git a/agent/php_mysqli.c b/agent/php_mysqli.c index 24a321b02..a6d3c8e76 100644 --- a/agent/php_mysqli.c +++ b/agent/php_mysqli.c @@ -688,3 +688,28 @@ const char* nr_php_mysqli_strip_persistent_prefix(const char* host) { return host; } + +void nr_php_mysqli_rshutdown() { + /* + * This frees mysqli metadata stored in the transaction. + * + * `mysqli_queries` contains duplicates of zvals. If + * `nr_php_txn_end` is called from the post-deactivate callback, request + * shutdown functions have already been called; and the Zend VM has already + * forcefully freed all dangling zvals that are not referenced by the global + * scope (regardless of their reference count), thus leaving the zvals stored + * in the mysqli_queries metadata in an "undefined" state. Consequently, + * freeing the zvals in `nr_php_txn_end` at this stage can result in undefined + * behavior. + * + * Calling this function during the RSHUTDOWN phase ensures that the zvals in + * `mysqli_queries` are cleaned up before Zend winds down the VM and + * forcefully frees zvals. + * + * If `nr_php_txn_end` is called outside the post-deactivate callback, + * it frees `mysqli_queries` by itself. + */ + if (nrlikely(NRPRG(txn))) { + nr_hashmap_destroy(&NRTXNGLOBAL(mysqli_queries)); + } +} diff --git a/agent/php_mysqli.h b/agent/php_mysqli.h index 03d42cf57..ca99d98a1 100644 --- a/agent/php_mysqli.h +++ b/agent/php_mysqli.h @@ -163,4 +163,10 @@ extern nr_datastore_instance_t* nr_php_mysqli_retrieve_datastore_instance( extern void nr_php_mysqli_remove_datastore_instance( const zval* mysqli_obj TSRMLS_DC); +/* + * Purpose : Frees reference incremented, transaction global zvals + * that must be cleaned up prior to postdeactivate + */ +extern void nr_php_mysqli_rshutdown(); + #endif /* PHP_MYSQLI_HDR */ diff --git a/agent/php_pdo.c b/agent/php_pdo.c index d850ffa82..18d01be3b 100644 --- a/agent/php_pdo.c +++ b/agent/php_pdo.c @@ -638,3 +638,28 @@ zval* nr_php_pdo_disable_persistence(const zval* options TSRMLS_DC) { nr_php_zval_free(&persistent); return result; } + +void nr_php_pdo_rshutdown() { + /* + * This frees pdo metadata stored in the transaction. + * + * `pdo_link_options` contains duplicates of zvals. If + * `nr_php_txn_end` is called from the post-deactivate callback, request + * shutdown functions have already been called; and the Zend VM has already + * forcefully freed all dangling zvals that are not referenced by the global + * scope (regardless of their reference count), thus leaving the zvals stored + * in the pdo_link_options metadata in an "undefined" state. Consequently, + * freeing the zvals in `nr_php_txn_end` at this stage can result in undefined + * behavior. + * + * Calling this function during the RSHUTDOWN phase ensures that the zvals in + * `pdo_link_options` are cleaned up before Zend winds down the VM and + * forcefully frees zvals. + * + * If `nr_php_txn_end` is called outside the post-deactivate callback, + * it frees `pdo_link_options` by itself. + */ + if (nrlikely(NRPRG(txn))) { + nr_hashmap_destroy(&NRTXNGLOBAL(pdo_link_options)); + } +} diff --git a/agent/php_pdo.h b/agent/php_pdo.h index e6e4d54d4..d5b6d6cbb 100644 --- a/agent/php_pdo.h +++ b/agent/php_pdo.h @@ -161,4 +161,9 @@ extern nr_status_t nr_php_pdo_parse_data_source( extern void nr_php_pdo_free_data_sources(struct pdo_data_src_parser* parsed, size_t nparams); +/* + * Purpose : Frees reference incremented, transaction global zvals + * that must be cleaned up prior to postdeactivate + */ +extern void nr_php_pdo_rshutdown(); #endif diff --git a/agent/php_rshutdown.c b/agent/php_rshutdown.c index a4165ed7a..7716324d7 100644 --- a/agent/php_rshutdown.c +++ b/agent/php_rshutdown.c @@ -12,6 +12,8 @@ #include "php_globals.h" #include "php_user_instrument.h" #include "php_wrapper.h" +#include "php_mysqli.h" +#include "php_pdo.h" #include "util_logging.h" #include "lib_guzzle4.h" @@ -49,6 +51,8 @@ PHP_RSHUTDOWN_FUNCTION(newrelic) { nr_guzzle4_rshutdown(TSRMLS_C); nr_curl_rshutdown(TSRMLS_C); + nr_php_pdo_rshutdown(); + nr_php_mysqli_rshutdown(); nrl_verbosedebug(NRL_INIT, "RSHUTDOWN processing done"); diff --git a/agent/php_txn.c b/agent/php_txn.c index 800cfb7f2..f432fb91d 100644 --- a/agent/php_txn.c +++ b/agent/php_txn.c @@ -1187,9 +1187,6 @@ static void nr_php_txn_do_shutdown(nrtxn_t* txn TSRMLS_DC) { * cannot be configured into the browser client config. */ nr_php_capture_request_parameters(txn TSRMLS_CC); - - nr_hashmap_destroy(&NRTXNGLOBAL(mysqli_queries)); - nr_hashmap_destroy(&NRTXNGLOBAL(pdo_link_options)); } void nr_php_txn_shutdown(TSRMLS_D) { @@ -1356,6 +1353,9 @@ nr_status_t nr_php_txn_end(int ignoretxn, int in_post_deactivate TSRMLS_DC) { nr_hashmap_destroy(&NRTXNGLOBAL(curl_multi_metadata)); nr_mysqli_metadata_destroy(&NRTXNGLOBAL(mysqli_links)); + nr_hashmap_destroy(&NRTXNGLOBAL(mysqli_queries)); + + nr_hashmap_destroy(&NRTXNGLOBAL(pdo_link_options)); return NR_SUCCESS; } diff --git a/tests/integration/external/curl_multi_exec/test_txn_restart_ignore.php b/tests/integration/external/curl_multi_exec/test_txn_restart_ignore.php new file mode 100644 index 000000000..c7ada36a5 --- /dev/null +++ b/tests/integration/external/curl_multi_exec/test_txn_restart_ignore.php @@ -0,0 +1,96 @@ + 0); + + curl_close($ch1); + curl_close($ch2); + curl_multi_close($mh); + + tap_ok("end of function reached without crash", true); +} + +test_txn_restart(); diff --git a/tests/integration/external/guzzle7/test_txn_ignore.php b/tests/integration/external/guzzle7/test_txn_ignore.php new file mode 100644 index 000000000..169785545 --- /dev/null +++ b/tests/integration/external/guzzle7/test_txn_ignore.php @@ -0,0 +1,98 @@ + +get($url); + echo $response->getBody(); + + $response = $client->get($url, [ + 'headers' => [ + 'zip' => 'zap']]); + echo $response->getBody(); + + $response = $client->get($url, [ + 'headers' => [ + 'zip' => 'zap', + CUSTOMER_HEADER => 'zap']]); + echo $response->getBody(); +} + +run_test(); + +newrelic_end_transaction(true); +newrelic_start_transaction(ini_get("newrelic.appname")); + +run_test(); + +newrelic_end_transaction(); diff --git a/tests/integration/pdo/test_txn_ignore.php b/tests/integration/pdo/test_txn_ignore.php new file mode 100644 index 000000000..7e5a16cfc --- /dev/null +++ b/tests/integration/pdo/test_txn_ignore.php @@ -0,0 +1,102 @@ +", + "?? SQL id", + "select * from information_schema.tables where table_name = ? limit ?;", + "Datastore/statement/MySQL/tables/select", + 1, + "?? total time", + "?? min time", + "?? max time", + { + "backtrace": [ + " in PDOStatement::execute called at __FILE__ (??)", + " in test_prepared_statement called at __FILE__ (??)" + ], + "explain_plan": [ + [ + "id", + "select_type", + "table", + "type", + "possible_keys", + "key", + "key_len", + "ref", + "rows", + "Extra" + ], + [ + [ + 1, + "SIMPLE", + "tables", + "ALL", + null, + "TABLE_NAME", + null, + null, + "??", + "??" + ] + ] + ] + } + ] + ] +] +*/ + +require_once(realpath (dirname ( __FILE__ )) . '/../../include/tap.php'); +require_once(realpath (dirname ( __FILE__ )) . '/pdo.inc'); + +function test_prepared_statement() { + global $PDO_MYSQL_DSN, $MYSQL_USER, $MYSQL_PASSWD; + + $conn = new PDO($PDO_MYSQL_DSN, $MYSQL_USER, $MYSQL_PASSWD); + $stmt = $conn->prepare('select * from information_schema.tables where table_name = ? limit 1;'); + $stmt->bindValue(1, "missing"); + tap_assert($stmt->execute(), 'execute prepared statement with a param'); +} + +test_prepared_statement(); + +newrelic_end_transaction(true); +newrelic_start_transaction(ini_get("newrelic.appname")); + +test_prepared_statement(); + +newrelic_end_transaction();