Skip to content

Commit 4a19a98

Browse files
ericnorrisericnorris
committed
refactor: use module globals instead of persistent resources
1 parent cdf66ab commit 4a19a98

File tree

3 files changed

+70
-48
lines changed

3 files changed

+70
-48
lines changed

‎ext/curl/curl_private.h

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,19 @@
4040
#define SAVE_CURL_ERROR(__handle, __err) \
4141
do { (__handle)->err.no = (int) __err; } while (0)
4242

43+
ZEND_BEGIN_MODULE_GLOBALS(curl)
44+
HashTable persistent_share_handles;
45+
ZEND_END_MODULE_GLOBALS(curl)
46+
47+
ZEND_EXTERN_MODULE_GLOBALS(curl)
48+
49+
#define CURL_G(v) ZEND_MODULE_GLOBALS_ACCESSOR(curl, v)
50+
4351
PHP_MINIT_FUNCTION(curl);
4452
PHP_MSHUTDOWN_FUNCTION(curl);
4553
PHP_MINFO_FUNCTION(curl);
54+
PHP_GINIT_FUNCTION(curl);
55+
PHP_GSHUTDOWN_FUNCTION(curl);
4656

4757
typedef struct {
4858
zend_fcall_info_cache fcc;
@@ -126,7 +136,7 @@ typedef struct {
126136

127137
typedef struct _php_curlsh {
128138
CURLSH *share;
129-
zend_bool is_persistent;
139+
zend_bool persistent;
130140

131141
struct {
132142
int no;
@@ -155,7 +165,8 @@ static inline php_curlsh *curl_share_from_obj(zend_object *obj) {
155165
#define Z_CURL_SHARE_P(zv) curl_share_from_obj(Z_OBJ_P(zv))
156166

157167
void curl_multi_register_handlers(void);
158-
void curl_share_register_handlers(int);
168+
void curl_share_free_persistent(zval *data);
169+
void curl_share_register_handlers(void);
159170
void curl_share_free_obj(zend_object *object);
160171
void curlfile_register_class(void);
161172
zend_result curl_cast_object(zend_object *obj, zval *result, int type);

‎ext/curl/interface.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@
5959
#include "ext/standard/url.h"
6060
#include "curl_private.h"
6161

62+
ZEND_DECLARE_MODULE_GLOBALS(curl)
63+
6264
#ifdef __GNUC__
6365
/* don't complain about deprecated CURLOPT_* we're exposing to PHP; we
6466
need to keep using those to avoid breaking PHP API compatibility */
@@ -215,7 +217,11 @@ zend_module_entry curl_module_entry = {
215217
NULL,
216218
PHP_MINFO(curl),
217219
PHP_CURL_VERSION,
218-
STANDARD_MODULE_PROPERTIES
220+
PHP_MODULE_GLOBALS(curl),
221+
PHP_GINIT(curl),
222+
PHP_GSHUTDOWN(curl),
223+
NULL,
224+
STANDARD_MODULE_PROPERTIES_EX
219225
};
220226
/* }}} */
221227

@@ -409,13 +415,26 @@ PHP_MINIT_FUNCTION(curl)
409415
curl_multi_register_handlers();
410416

411417
curl_share_ce = register_class_CurlShareHandle();
412-
curl_share_register_handlers(module_number);
418+
curl_share_register_handlers();
413419
curlfile_register_class();
414420

415421
return SUCCESS;
416422
}
417423
/* }}} */
418424

425+
/* {{{ PHP_GINIT_FUNCTION */
426+
PHP_GINIT_FUNCTION(curl)
427+
{
428+
zend_hash_init(&curl_globals->persistent_share_handles, 0, NULL, curl_share_free_persistent, true);
429+
GC_MAKE_PERSISTENT_LOCAL(&curl_globals->persistent_share_handles);
430+
}
431+
/* }}} */
432+
433+
PHP_GSHUTDOWN_FUNCTION(curl)
434+
{
435+
zend_hash_destroy(&curl_globals->persistent_share_handles);
436+
}
437+
419438
/* CurlHandle class */
420439

421440
static zend_object *curl_create_object(zend_class_entry *class_type) {

‎ext/curl/share.c

Lines changed: 36 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -28,64 +28,66 @@
2828

2929
#define SAVE_CURLSH_ERROR(__handle, __err) (__handle)->err.no = (int) __err;
3030

31-
static int le_pcurlsh;
31+
void curl_share_free_persistent(zval *data)
32+
{
33+
CURLSH *handle = Z_PTR_P(data);
34+
35+
if (!handle) {
36+
return;
37+
}
38+
39+
curl_share_cleanup(handle);
40+
}
3241

3342
/* {{{ Initialize a persistent curl share handle */
3443
PHP_FUNCTION(curl_share_init_persistent)
3544
{
36-
php_curlsh *sh;
45+
zend_string *id;
3746

38-
zend_string *persistent_id = NULL;
39-
zend_string *persistent_key = NULL;
47+
zval *persisted;
4048

41-
zend_resource *persisted;
49+
php_curlsh *sh;
4250

4351
zval *arr, *entry;
52+
CURLSHcode error;
4453

4554
ZEND_PARSE_PARAMETERS_START(2, 2)
46-
Z_PARAM_STR_EX(persistent_id, 1, 0)
55+
Z_PARAM_STR_EX(id, 1, 0)
4756
Z_PARAM_ARRAY(arr)
4857
ZEND_PARSE_PARAMETERS_END();
4958

50-
persistent_key = strpprintf(0, "curl_share_init_persistent:id=%s", ZSTR_VAL(persistent_id));
51-
5259
object_init_ex(return_value, curl_share_ce);
53-
5460
sh = Z_CURL_SHARE_P(return_value);
5561

56-
if ((persisted = zend_hash_find_ptr(&EG(persistent_list), persistent_key)) != NULL) {
57-
if (persisted->type == le_pcurlsh) {
58-
sh->share = persisted->ptr;
59-
sh->is_persistent = 1;
62+
if ((persisted = zend_hash_find(&CURL_G(persistent_share_handles), id)) != NULL) {
63+
sh->share = Z_PTR_P(persisted);
64+
sh->persistent = 1;
65+
} else {
66+
sh->share = curl_share_init();
6067

61-
goto cleanup;
62-
}
63-
}
64-
65-
sh->share = curl_share_init();
68+
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(arr), entry) {
69+
ZVAL_DEREF(entry);
6670

67-
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(arr), entry) {
68-
ZVAL_DEREF(entry);
71+
error = curl_share_setopt(sh->share, CURLSHOPT_SHARE, zval_get_long(entry));
6972

70-
CURLSHcode error = curl_share_setopt(sh->share, CURLSHOPT_SHARE, zval_get_long(entry));
73+
if (error != CURLSHE_OK) {
74+
php_error_docref(NULL, E_WARNING, "could not construct persistent curl share: %s", curl_share_strerror(error));
7175

72-
if (error != CURLSHE_OK) {
73-
php_error_docref(NULL, E_WARNING, "could not construct persistent curl share: %s", curl_share_strerror(error));
74-
goto error;
75-
}
76-
} ZEND_HASH_FOREACH_END();
76+
goto error;
77+
}
78+
} ZEND_HASH_FOREACH_END();
7779

78-
zend_register_persistent_resource(ZSTR_VAL(persistent_key), ZSTR_LEN(persistent_key), sh->share, le_pcurlsh);
80+
// It's important not to mark this as persistent until *after* we've successfully set all of the share options,
81+
// as otherwise the curl_share_free_obj in the error handler won't free the CURLSH.
82+
sh->persistent = 1;
7983

80-
sh->is_persistent = 1;
84+
zend_hash_str_add_new_ptr(&CURL_G(persistent_share_handles), ZSTR_VAL(id), ZSTR_LEN(id), sh->share);
85+
}
8186

82-
cleanup:
83-
zend_string_release(persistent_key);
8487
return;
8588

8689
error:
87-
zend_string_release(persistent_key);
88-
curl_share_free_obj(Z_OBJ_P(return_value));
90+
zval_ptr_dtor(return_value);
8991

9092
RETURN_FALSE;
9193
}
@@ -105,14 +107,6 @@ PHP_FUNCTION(curl_share_init)
105107
}
106108
/* }}} */
107109

108-
ZEND_RSRC_DTOR_FUNC(php_pcurlsh_dtor)
109-
{
110-
if (res->ptr) {
111-
curl_share_cleanup(res->ptr);
112-
res->ptr = NULL;
113-
}
114-
}
115-
116110
/* {{{ Close a set of cURL handles */
117111
PHP_FUNCTION(curl_share_close)
118112
{
@@ -225,7 +219,7 @@ void curl_share_free_obj(zend_object *object)
225219
{
226220
php_curlsh *sh = curl_share_from_obj(object);
227221

228-
if (!sh->is_persistent) {
222+
if (!sh->persistent) {
229223
curl_share_cleanup(sh->share);
230224
}
231225

@@ -234,7 +228,7 @@ void curl_share_free_obj(zend_object *object)
234228

235229
static zend_object_handlers curl_share_handlers;
236230

237-
void curl_share_register_handlers(int module_number) {
231+
void curl_share_register_handlers(void) {
238232
curl_share_ce->create_object = curl_share_create_object;
239233
curl_share_ce->default_object_handlers = &curl_share_handlers;
240234

@@ -244,6 +238,4 @@ void curl_share_register_handlers(int module_number) {
244238
curl_share_handlers.get_constructor = curl_share_get_constructor;
245239
curl_share_handlers.clone_obj = NULL;
246240
curl_share_handlers.compare = zend_objects_not_comparable;
247-
248-
le_pcurlsh = zend_register_list_destructors_ex(NULL, php_pcurlsh_dtor, "Curl persistent shared handle", module_number);
249241
}

0 commit comments

Comments
 (0)