Skip to content

PHPC-1737: Use zend_hash_graceful_reverse_destroy for persistent client HashTable #1191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 20, 2021
Merged

PHPC-1737: Use zend_hash_graceful_reverse_destroy for persistent client HashTable #1191

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1e3f97
PHPC-1737: Use zend_hash_graceful_reverse_destroy for persistent clie…
jmikola Jan 20, 2021
1537ad0
Improve comments for request-scoped HashTables
jmikola Jan 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 24 additions & 22 deletions php_phongo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3722,23 +3722,29 @@ static void php_phongo_pclient_destroy_ptr(zval* ptr)
PHP_RINIT_FUNCTION(mongodb)
{
/* Initialize HashTable for non-persistent clients, which is initialized to
* NULL in GINIT and destroyed and reset to NULL in RSHUTDOWN. */
* NULL in GINIT and destroyed and reset to NULL in RSHUTDOWN. Although we
* specify an element destructor here, all request clients should be freed
* naturally via garbage collection (i.e. the HashTable should be empty at
* the time it is destroyed in RSHUTDOWN). */
if (MONGODB_G(request_clients) == NULL) {
ALLOC_HASHTABLE(MONGODB_G(request_clients));
zend_hash_init(MONGODB_G(request_clients), 0, NULL, php_phongo_pclient_destroy_ptr, 0);
}

/* Initialize HashTable for APM subscribers, which is initialized to NULL in
* GINIT and destroyed and reset to NULL in RSHUTDOWN. */
* GINIT and destroyed and reset to NULL in RSHUTDOWN. Since this HashTable
* will store subscriber object zvals, we specify ZVAL_PTR_DTOR as its
* element destructor so that any still-registered subscribers can be freed
* in RSHUTDOWN. */
if (MONGODB_G(subscribers) == NULL) {
ALLOC_HASHTABLE(MONGODB_G(subscribers));
zend_hash_init(MONGODB_G(subscribers), 0, NULL, ZVAL_PTR_DTOR, 0);
}

/* Initialize HashTable for registering Manager objects. This is initialized
* to NULL in GINIT and destroyed and reset to NULL in RSHUTDOWN. Since this
* HashTable stores pointers to existing php_phongo_manager_t objects, the
* element destructor is intentionally NULL. */
* HashTable stores pointers to existing php_phongo_manager_t objects (not
* counted references), the element destructor is intentionally NULL. */
if (MONGODB_G(managers) == NULL) {
ALLOC_HASHTABLE(MONGODB_G(managers));
zend_hash_init(MONGODB_G(managers), 0, NULL, NULL, 0);
Expand All @@ -3761,8 +3767,12 @@ PHP_GINIT_FUNCTION(mongodb)
/* Clear extension globals */
memset(mongodb_globals, 0, sizeof(zend_mongodb_globals));

/* Initialize HashTable for persistent clients */
zend_hash_init(&mongodb_globals->persistent_clients, 0, NULL, NULL, 1);
/* Initialize HashTable for persistent clients, which will be destroyed in
* GSHUTDOWN. We specify an element destructor so that persistent clients
* can be destroyed along with the HashTable. The HashTable's struct is
* nested within globals, so no allocation is needed (unlike the HashTables
* allocated in RINIT). */
zend_hash_init(&mongodb_globals->persistent_clients, 0, NULL, php_phongo_pclient_destroy_ptr, 1);
}
/* }}} */

Expand Down Expand Up @@ -3934,9 +3944,11 @@ PHP_RSHUTDOWN_FUNCTION(mongodb)

/* Destroy HashTable for non-persistent clients, which was initialized in
* RINIT. This is intentionally done after the APM subscribers to allow any
* non-persistent clients still referenced by a subscriber (not removed
* prior to RSHUTDOWN) to be naturally freed by the Manager's free_object
* handler rather than the HashTable's element destructor. */
* non-persistent clients still referenced by a subscriber (not freed prior
* to RSHUTDOWN) to be naturally garbage collected and freed by the Manager
* free_object handler rather than the HashTable's element destructor. There
* is no need to use zend_hash_graceful_reverse_destroy here like we do for
* persistent clients; moreover, the HashTable should already be empty. */
if (MONGODB_G(request_clients)) {
zend_hash_destroy(MONGODB_G(request_clients));
FREE_HASHTABLE(MONGODB_G(request_clients));
Expand All @@ -3957,20 +3969,10 @@ PHP_RSHUTDOWN_FUNCTION(mongodb)
/* {{{ PHP_GSHUTDOWN_FUNCTION */
PHP_GSHUTDOWN_FUNCTION(mongodb)
{
php_phongo_pclient_t* pclient;

/* Destroy mongoc_client_t objects in reverse order. This is necessary to
* prevent segmentation faults as clients may reference other clients in
/* Destroy persistent client HashTable in reverse order. This is necessary
* to prevent segmentation faults as clients may reference other clients in
* encryption settings. */
ZEND_HASH_REVERSE_FOREACH_PTR(&mongodb_globals->persistent_clients, pclient)
{
php_phongo_pclient_destroy(pclient);
}
ZEND_HASH_FOREACH_END();

/* Destroy HashTable for persistent clients. mongoc_client_t objects have
* already been destroyed. */
zend_hash_destroy(&mongodb_globals->persistent_clients);
zend_hash_graceful_reverse_destroy(&mongodb_globals->persistent_clients);

mongodb_globals->debug = NULL;
if (mongodb_globals->debug_fd) {
Expand Down