refactor: use module globals instead of persistent resources

Author: ericnorris

ext/curl/curl_private.h

@@ -40,9 +40,19 @@
 #define SAVE_CURL_ERROR(__handle, __err) \
     do { (__handle)->err.no = (int) __err; } while (0)
 
+ZEND_BEGIN_MODULE_GLOBALS(curl)
+	HashTable persistent_share_handles;
+ZEND_END_MODULE_GLOBALS(curl)
+
+ZEND_EXTERN_MODULE_GLOBALS(curl)
+
+#define CURL_G(v) ZEND_MODULE_GLOBALS_ACCESSOR(curl, v)
+
 PHP_MINIT_FUNCTION(curl);
 PHP_MSHUTDOWN_FUNCTION(curl);
 PHP_MINFO_FUNCTION(curl);
+PHP_GINIT_FUNCTION(curl);
+PHP_GSHUTDOWN_FUNCTION(curl);
 
 typedef struct {
 	zend_fcall_info_cache fcc;
@@ -126,7 +136,7 @@ typedef struct {
 
 typedef struct _php_curlsh {
 	CURLSH	  *share;
-	zend_bool is_persistent;
+	zend_bool persistent;
 
 	struct {
 		int no;
@@ -155,7 +165,8 @@ static inline php_curlsh *curl_share_from_obj(zend_object *obj) {
 #define Z_CURL_SHARE_P(zv) curl_share_from_obj(Z_OBJ_P(zv))
 
 void curl_multi_register_handlers(void);
-void curl_share_register_handlers(int);
+void curl_share_free_persistent(zval *data);
+void curl_share_register_handlers(void);
 void curl_share_free_obj(zend_object *object);
 void curlfile_register_class(void);
 zend_result curl_cast_object(zend_object *obj, zval *result, int type);

ext/curl/interface.c

@@ -59,6 +59,8 @@
 #include "ext/standard/url.h"
 #include "curl_private.h"
 
+ZEND_DECLARE_MODULE_GLOBALS(curl)
+
 #ifdef __GNUC__
 /* don't complain about deprecated CURLOPT_* we're exposing to PHP; we
    need to keep using those to avoid breaking PHP API compatibility */
@@ -215,7 +217,11 @@ zend_module_entry curl_module_entry = {
 	NULL,
 	PHP_MINFO(curl),
 	PHP_CURL_VERSION,
-	STANDARD_MODULE_PROPERTIES
+    PHP_MODULE_GLOBALS(curl),
+    PHP_GINIT(curl),
+	PHP_GSHUTDOWN(curl),
+    NULL,
+	STANDARD_MODULE_PROPERTIES_EX
 };
 /* }}} */
 
@@ -409,13 +415,26 @@ PHP_MINIT_FUNCTION(curl)
 	curl_multi_register_handlers();
 
 	curl_share_ce = register_class_CurlShareHandle();
-	curl_share_register_handlers(module_number);
+	curl_share_register_handlers();
 	curlfile_register_class();
 
 	return SUCCESS;
 }
 /* }}} */
 
+/* {{{ PHP_GINIT_FUNCTION */
+PHP_GINIT_FUNCTION(curl)
+{
+	zend_hash_init(&curl_globals->persistent_share_handles, 0, NULL, curl_share_free_persistent, true);
+	GC_MAKE_PERSISTENT_LOCAL(&curl_globals->persistent_share_handles);
+}
+/* }}} */
+
+PHP_GSHUTDOWN_FUNCTION(curl)
+{
+	zend_hash_destroy(&curl_globals->persistent_share_handles);
+}
+
 /* CurlHandle class */
 
 static zend_object *curl_create_object(zend_class_entry *class_type) {

ext/curl/share.c

@@ -28,64 +28,66 @@
 
 #define SAVE_CURLSH_ERROR(__handle, __err) (__handle)->err.no = (int) __err;
 
-static int le_pcurlsh;
+void curl_share_free_persistent(zval *data)
+{
+	CURLSH *handle = Z_PTR_P(data);
+
+	if (!handle) {
+		return;
+	}
+
+	curl_share_cleanup(handle);
+}
 
 /* {{{ Initialize a persistent curl share handle */
 PHP_FUNCTION(curl_share_init_persistent)
 {
-	php_curlsh *sh;
+	zend_string *id;
 
-	zend_string *persistent_id  = NULL;
-	zend_string *persistent_key = NULL;
+	zval *persisted;
 
-	zend_resource *persisted;
+	php_curlsh *sh;
 
 	zval *arr, *entry;
+	CURLSHcode error;
 
 	ZEND_PARSE_PARAMETERS_START(2, 2)
-		Z_PARAM_STR_EX(persistent_id, 1, 0)
+		Z_PARAM_STR_EX(id, 1, 0)
 		Z_PARAM_ARRAY(arr)
 	ZEND_PARSE_PARAMETERS_END();
 
-	persistent_key = strpprintf(0, "curl_share_init_persistent:id=%s", ZSTR_VAL(persistent_id));
-
 	object_init_ex(return_value, curl_share_ce);
-
 	sh = Z_CURL_SHARE_P(return_value);
 
-	if ((persisted = zend_hash_find_ptr(&EG(persistent_list), persistent_key)) != NULL) {
-		if (persisted->type == le_pcurlsh) {
-			sh->share         = persisted->ptr;
-			sh->is_persistent = 1;
+    if ((persisted = zend_hash_find(&CURL_G(persistent_share_handles), id)) != NULL) {
+		sh->share      = Z_PTR_P(persisted);
+		sh->persistent = 1;
+    } else {
+		sh->share = curl_share_init();
 
-			goto cleanup;
-		}
-	}
-
-	sh->share = curl_share_init();
+		ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(arr), entry) {
+			ZVAL_DEREF(entry);
 
-	ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(arr), entry) {
-		ZVAL_DEREF(entry);
+			error = curl_share_setopt(sh->share, CURLSHOPT_SHARE, zval_get_long(entry));
 
-		CURLSHcode error = curl_share_setopt(sh->share, CURLSHOPT_SHARE, zval_get_long(entry));
+			if (error != CURLSHE_OK) {
+				php_error_docref(NULL, E_WARNING, "could not construct persistent curl share: %s", curl_share_strerror(error));
 
-		if (error != CURLSHE_OK) {
-			php_error_docref(NULL, E_WARNING, "could not construct persistent curl share: %s", curl_share_strerror(error));
-			goto error;
-		}
-	} ZEND_HASH_FOREACH_END();
+				goto error;
+			}
+		} ZEND_HASH_FOREACH_END();
 
-	zend_register_persistent_resource(ZSTR_VAL(persistent_key), ZSTR_LEN(persistent_key), sh->share, le_pcurlsh);
+		// It's important not to mark this as persistent until *after* we've successfully set all of the share options,
+		// as otherwise the curl_share_free_obj in the error handler won't free the CURLSH.
+		sh->persistent = 1;
 
-	sh->is_persistent = 1;
+		zend_hash_str_add_new_ptr(&CURL_G(persistent_share_handles), ZSTR_VAL(id), ZSTR_LEN(id), sh->share);
+	}
 
- cleanup:
-	zend_string_release(persistent_key);
 	return;
 
  error:
-	zend_string_release(persistent_key);
-	curl_share_free_obj(Z_OBJ_P(return_value));
+	zval_ptr_dtor(return_value);
 
 	RETURN_FALSE;
 }
@@ -105,14 +107,6 @@ PHP_FUNCTION(curl_share_init)
 }
 /* }}} */
 
-ZEND_RSRC_DTOR_FUNC(php_pcurlsh_dtor)
-{
-	if (res->ptr) {
-		curl_share_cleanup(res->ptr);
-		res->ptr = NULL;
-	}
-}
-
 /* {{{ Close a set of cURL handles */
 PHP_FUNCTION(curl_share_close)
 {
@@ -225,7 +219,7 @@ void curl_share_free_obj(zend_object *object)
 {
 	php_curlsh *sh = curl_share_from_obj(object);
 
-	if (!sh->is_persistent) {
+	if (!sh->persistent) {
 		curl_share_cleanup(sh->share);
 	}
 
@@ -234,7 +228,7 @@ void curl_share_free_obj(zend_object *object)
 
 static zend_object_handlers curl_share_handlers;
 
-void curl_share_register_handlers(int module_number) {
+void curl_share_register_handlers(void) {
 	curl_share_ce->create_object = curl_share_create_object;
 	curl_share_ce->default_object_handlers = &curl_share_handlers;
 
@@ -244,6 +238,4 @@ void curl_share_register_handlers(int module_number) {
 	curl_share_handlers.get_constructor = curl_share_get_constructor;
 	curl_share_handlers.clone_obj = NULL;
 	curl_share_handlers.compare = zend_objects_not_comparable;
-
-	le_pcurlsh = zend_register_list_destructors_ex(NULL, php_pcurlsh_dtor, "Curl persistent shared handle", module_number);
 }