From: Christoph M. Becker Date: Mon, 1 Jul 2019 08:18:19 +0000 (+0200) Subject: Fix TsHashTable related race conditions X-Git-Tag: php-7.4.0alpha3~17 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=98b6330ab45732dcb16bb714d66ca1d987531406;p=php Fix TsHashTable related race conditions Although TsHashTable and the according API are supposed to easily make a HashTable thread-safe, they do not; for instance, there can be race conditions between finding and updating entries. We therefore avoid the usage of TsHashTable in favor of a HashTable with our own mutex management. The patch has been provided by krakjoe@php.net; I only did some minor fixes and tweaks. --- diff --git a/ext/com_dotnet/com_com.c b/ext/com_dotnet/com_com.c index fc8e84b3a1..58a22a9c62 100644 --- a/ext/com_dotnet/com_com.c +++ b/ext/com_dotnet/com_com.c @@ -267,7 +267,7 @@ PHP_FUNCTION(com_create_instance) if (SUCCEEDED(ITypeLib_GetDocumentation(TL, -1, &name, NULL, NULL, NULL))) { typelib_name = php_com_olestring_to_string(name, &typelib_name_len, obj->code_page); - if (NULL != zend_ts_hash_str_add_ptr(&php_com_typelibraries, typelib_name, typelib_name_len, TL)) { + if (NULL != php_com_cache_typelib(TL, typelib_name, typelib_name_len)) { php_com_import_typelib(TL, mode, obj->code_page); /* add a reference for the hash */ diff --git a/ext/com_dotnet/com_extension.c b/ext/com_dotnet/com_extension.c index 94b6b65ee6..7ff0fd0ebd 100644 --- a/ext/com_dotnet/com_extension.c +++ b/ext/com_dotnet/com_extension.c @@ -33,8 +33,6 @@ ZEND_DECLARE_MODULE_GLOBALS(com_dotnet) static PHP_GINIT_FUNCTION(com_dotnet); -TsHashTable php_com_typelibraries; - zend_class_entry *php_com_variant_class_entry, *php_com_exception_class_entry, @@ -330,8 +328,6 @@ PHP_MINIT_FUNCTION(com_dotnet) tmp->serialize = zend_class_serialize_deny; tmp->unserialize = zend_class_unserialize_deny; - zend_ts_hash_init(&php_com_typelibraries, 0, NULL, php_com_typelibrary_dtor, 1); - #if HAVE_MSCOREE_H INIT_CLASS_ENTRY(ce, "dotnet", NULL); ce.create_object = php_com_object_new; @@ -418,6 +414,9 @@ PHP_MINIT_FUNCTION(com_dotnet) COM_CONST(VT_UI8); COM_CONST(VT_I8); #endif + + PHP_MINIT(com_typeinfo)(INIT_FUNC_ARGS_PASSTHRU); + return SUCCESS; } /* }}} */ @@ -433,7 +432,8 @@ PHP_MSHUTDOWN_FUNCTION(com_dotnet) } #endif - zend_ts_hash_destroy(&php_com_typelibraries); + PHP_MSHUTDOWN(com_typeinfo)(INIT_FUNC_ARGS_PASSTHRU); + return SUCCESS; } /* }}} */ diff --git a/ext/com_dotnet/com_typeinfo.c b/ext/com_dotnet/com_typeinfo.c index d5afd57a89..44d0340af5 100644 --- a/ext/com_dotnet/com_typeinfo.c +++ b/ext/com_dotnet/com_typeinfo.c @@ -27,6 +27,33 @@ #include "php_com_dotnet.h" #include "php_com_dotnet_internal.h" +static HashTable php_com_typelibraries; + +#ifdef ZTS +static MUTEX_T php_com_typelibraries_mutex; +#endif + +PHP_MINIT_FUNCTION(com_typeinfo) +{ + zend_hash_init(&php_com_typelibraries, 0, NULL, php_com_typelibrary_dtor, 1); + +#ifdef ZTS + php_com_typelibraries_mutex = tsrm_mutex_alloc(); +#endif + + return SUCCESS; +} + +PHP_MSHUTDOWN_FUNCTION(com_typeinfo) +{ + zend_hash_destroy(&php_com_typelibraries); + +#ifdef ZTS + tsrm_mutex_free(php_com_typelibraries_mutex); +#endif + + return SUCCESS; +} /* The search string can be either: * a) a file name @@ -220,35 +247,58 @@ void php_com_typelibrary_dtor(zval *pDest) ITypeLib_Release(Lib); } +ITypeLib *php_com_cache_typelib(ITypeLib* TL, char *cache_key, zend_long cache_key_len) { + ITypeLib* result; +#ifdef ZTS + tsrm_mutex_lock(php_com_typelibraries_mutex); +#endif + + result = zend_hash_str_add_ptr(&php_com_typelibraries, cache_key, cache_key_len, TL); + +#ifdef ZTS + tsrm_mutex_unlock(php_com_typelibraries_mutex); +#endif + + return result; +} + PHP_COM_DOTNET_API ITypeLib *php_com_load_typelib_via_cache(char *search_string, int codepage, int *cached) { ITypeLib *TL; char *name_dup; - size_t l; + zend_string *key = zend_string_init(search_string, strlen(search_string), 1); - l = strlen(search_string); +#ifdef ZTS + tsrm_mutex_lock(php_com_typelibraries_mutex); +#endif - if ((TL = zend_ts_hash_str_find_ptr(&php_com_typelibraries, search_string, l)) != NULL) { + if ((TL = zend_hash_find_ptr(&php_com_typelibraries, key)) != NULL) { *cached = 1; /* add a reference for the caller */ ITypeLib_AddRef(TL); - return TL; + + goto php_com_load_typelib_via_cache_return; } *cached = 0; - name_dup = estrndup(search_string, l); + name_dup = estrndup(ZSTR_VAL(key), ZSTR_LEN(key)); TL = php_com_load_typelib(name_dup, codepage); efree(name_dup); if (TL) { - if (NULL != zend_ts_hash_str_update_ptr(&php_com_typelibraries, - search_string, l, TL)) { + if (NULL != zend_hash_add_ptr(&php_com_typelibraries, key, TL)) { /* add a reference for the hash table */ ITypeLib_AddRef(TL); } } +php_com_load_typelib_via_cache_return: +#ifdef ZTS + tsrm_mutex_unlock(php_com_typelibraries_mutex); +#endif + zend_string_release(key); + return TL; } diff --git a/ext/com_dotnet/php_com_dotnet_internal.h b/ext/com_dotnet/php_com_dotnet_internal.h index e41b7ffdd1..676d2f9330 100644 --- a/ext/com_dotnet/php_com_dotnet_internal.h +++ b/ext/com_dotnet/php_com_dotnet_internal.h @@ -27,8 +27,6 @@ #include #include "win32/winutil.h" -#include "zend_ts_hash.h" - typedef struct _php_com_dotnet_object { zend_object zo; @@ -70,7 +68,6 @@ static inline int php_com_is_valid_object(zval *zv) } while(0) /* com_extension.c */ -TsHashTable php_com_typelibraries; zend_class_entry *php_com_variant_class_entry, *php_com_exception_class_entry, *php_com_saproxy_class_entry; /* com_handlers.c */ @@ -175,6 +172,9 @@ PHP_COM_DOTNET_API int php_com_import_typelib(ITypeLib *TL, int mode, void php_com_typelibrary_dtor(zval *pDest); ITypeInfo *php_com_locate_typeinfo(char *typelibname, php_com_dotnet_object *obj, char *dispname, int sink); int php_com_process_typeinfo(ITypeInfo *typeinfo, HashTable *id_to_name, int printdef, GUID *guid, int codepage); +ITypeLib *php_com_cache_typelib(ITypeLib* TL, char *cache_key, zend_long cache_key_len); +PHP_MINIT_FUNCTION(com_typeinfo); +PHP_MSHUTDOWN_FUNCTION(com_typeinfo); /* com_iterator.c */ zend_object_iterator *php_com_iter_get(zend_class_entry *ce, zval *object, int by_ref);