diff --git a/classes/thread_safe.c b/classes/thread_safe.c index 15fa82f5..77089409 100644 --- a/classes/thread_safe.c +++ b/classes/thread_safe.c @@ -83,7 +83,7 @@ ThreadSafe_method(synchronized) if (pmmpthread_monitor_lock(&threaded->monitor)) { /* synchronize property tables */ - pmmpthread_store_sync_local_properties(Z_OBJ_P(getThis())); + pmmpthread_store_clean_stale_cache(Z_OBJ_P(getThis())); zend_try { /* call the closure */ diff --git a/src/globals.c b/src/globals.c index b9eb1ef8..aa4b993d 100644 --- a/src/globals.c +++ b/src/globals.c @@ -114,7 +114,11 @@ zend_bool pmmpthread_globals_init(){ (dtor_func_t)pmmpthread_globals_string_dtor_func, 1 ); - ZVAL_UNDEF(&PMMPTHREAD_G(undef_zval)); + + ZVAL_UNDEF(&PMMPTHREAD_G(uninitialized_property)); + Z_PROP_FLAG_P(&PMMPTHREAD_G(uninitialized_property)) |= IS_PROP_UNINIT; + ZVAL_UNDEF(&PMMPTHREAD_G(unset_property)); + PMMPTHREAD_G(thread_count) = 0; //only counting threads explicitly created by pmmpthread } diff --git a/src/globals.h b/src/globals.h index f331e420..d956db7f 100644 --- a/src/globals.h +++ b/src/globals.h @@ -59,7 +59,8 @@ struct _pmmpthread_globals { */ HashTable interned_strings; - zval undef_zval; + zval uninitialized_property; + zval unset_property; /* * High Frequency Strings diff --git a/src/handlers.c b/src/handlers.c index 2d06bc47..391fb840 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -66,7 +66,7 @@ zval *pmmpthread_get_property_ptr_ptr_stub(zend_object *object, zend_string *mem /* {{{ */ zval* pmmpthread_read_dimension(PMMPTHREAD_READ_DIMENSION_PASSTHRU_D) { - if (pmmpthread_store_read(object, member, type, rv) == FAILURE) { + if (pmmpthread_store_read(object, member, NULL, type, rv) == FAILURE) { //TODO: this ought to generate warnings, but this is a pain right now due to key type juggling //for now this maintains the v4 behaviour of silently generating NULL, which is better than segfaulting if (!EG(exception)) { @@ -81,39 +81,48 @@ zval* pmmpthread_read_dimension(PMMPTHREAD_READ_DIMENSION_PASSTHRU_D) { zval* pmmpthread_read_property(PMMPTHREAD_READ_PROPERTY_PASSTHRU_D) { zval zmember; + zval *result; zend_guard* guard; - ZVAL_STR(&zmember, member); - if (object->ce->__get && (guard = zend_get_property_guard(object, member)) && !((*guard) & IN_GET)) { + ZVAL_STR(&zmember, member); (*guard) |= IN_GET; zend_call_known_instance_method_with_1_params(object->ce->__get, object, rv, &zmember); (*guard) &= ~IN_GET; } else { - zend_property_info* info = zend_get_property_info(object->ce, member, 0); - if (info == ZEND_WRONG_PROPERTY_INFO) { - rv = &EG(uninitialized_zval); - } else if (info == NULL || !PMMPTHREAD_OBJECT_PROPERTY(info)) { //dynamic property - if (pmmpthread_store_read(object, &zmember, type, rv) == FAILURE) { - if (type != BP_VAR_IS) { - zend_error(E_WARNING, "Undefined property: %s::$%s", ZSTR_VAL(object->ce->name), ZSTR_VAL(member)); - } - rv = &EG(uninitialized_zval); + zend_property_info* info = zend_get_property_info(object->ce, member, 1); + ZVAL_STR(&zmember, member); + if (info != NULL) { + if (info->flags & ZEND_ACC_VIRTUAL) { + //virtual properties are similar to magic methods - don't touch store or cache + return zend_std_read_property(object, member, type, NULL, rv); } - } else { - //defined property, use mangled name + if (!PMMPTHREAD_OBJECT_PROPERTY(info)) { + info = NULL; //don't send invalid infos into store + } + } + + if (info != NULL) { ZVAL_STR(&zmember, info->name); + } else { + ZVAL_STR(&zmember, member); + } - if (pmmpthread_store_read(object, &zmember, type, rv) == FAILURE) { - if (type != BP_VAR_IS && !EG(exception)) { - zend_throw_error(NULL, "Typed property %s::$%s must not be accessed before initialization", - ZSTR_VAL(info->ce->name), - ZSTR_VAL(member)); - } - rv = &EG(uninitialized_zval); + //this moves the value to cache for zend_std_read_property() to work on + pmmpthread_store_read_ex(object, &zmember, info, type, rv, 1); + if (EG(exception)) { + rv = &EG(uninitialized_zval); + } else { + //no cache for now - we don't want the VM bypassing this handler + result = zend_std_read_property(object, member, type, NULL, rv); + ZVAL_COPY_VALUE(rv, result); + //tidy property cache so we don't read wrong values later + if (!pmmpthread_store_retain_in_local_cache(rv)) { + pmmpthread_store_clean_local_property(object, &zmember, info); } } } + return rv; } /* }}} */ @@ -129,7 +138,7 @@ zval* pmmpthread_read_property_deny(PMMPTHREAD_READ_PROPERTY_PASSTHRU_D) { /* {{{ */ void pmmpthread_write_dimension(PMMPTHREAD_WRITE_DIMENSION_PASSTHRU_D) { - if (pmmpthread_store_write(object, member, value, PMMPTHREAD_STORE_NO_COERCE_ARRAY) == FAILURE && !EG(exception)){ + if (pmmpthread_store_write(object, member, NULL, value, PMMPTHREAD_STORE_NO_COERCE_ARRAY) == FAILURE && !EG(exception)){ zend_throw_error( pmmpthread_ce_nts_value_error, "Cannot assign non-thread-safe value of type %s to %s", @@ -141,15 +150,12 @@ void pmmpthread_write_dimension(PMMPTHREAD_WRITE_DIMENSION_PASSTHRU_D) { zval* pmmpthread_write_property(PMMPTHREAD_WRITE_PROPERTY_PASSTHRU_D) { zval zmember; - zval tmp; zend_guard* guard; - ZVAL_STR(&zmember, member); - ZVAL_UNDEF(&tmp); - if (object->ce->__set && (guard = zend_get_property_guard(object, member)) && !((*guard) & IN_SET)) { zval rv; ZVAL_UNDEF(&rv); + ZVAL_STR(&zmember, member); (*guard) |= IN_SET; zend_call_known_instance_method_with_2_params(object->ce->__set, object, &rv, &zmember, value); @@ -158,42 +164,51 @@ zval* pmmpthread_write_property(PMMPTHREAD_WRITE_PROPERTY_PASSTHRU_D) { if (Z_TYPE(rv) != IS_UNDEF) zval_dtor(&rv); } else { - bool ok = true; - zend_property_info* info = zend_get_property_info(object->ce, member, 0); - if (info != ZEND_WRONG_PROPERTY_INFO) { - if (info != NULL && PMMPTHREAD_OBJECT_PROPERTY(info)) { - ZVAL_STR(&zmember, info->name); //use mangled name to avoid private member shadowing issues - - zend_execute_data* execute_data = EG(current_execute_data); - bool strict = execute_data - && execute_data->func - && ZEND_CALL_USES_STRICT_TYPES(EG(current_execute_data)); - - //zend_verify_property_type() might modify the value - //value is not copied before we receive it, so it might be - //from opcache protected memory which we can't modify - ZVAL_COPY(&tmp, value); - value = &tmp; - - if (ZEND_TYPE_IS_SET(info->type) && !zend_verify_property_type(info, value, strict)) { - ok = false; + //no cache for now - cache would allow the VM to bypass this handler + //std_write may coerce the var to a different type, so we need to use the result + value = zend_std_write_property(object, member, value, NULL); + + if (value != &EG(error_zval)) { + zval* real_value = NULL; + zval zmember; + + ZVAL_STR(&zmember, member); + zend_property_info* info = zend_get_property_info(object->ce, member, 1); + if (info != NULL) { + if (info->flags & ZEND_ACC_VIRTUAL) { + return value; + } + if (!PMMPTHREAD_OBJECT_PROPERTY(info)) { + info = NULL; //don't send invalid property infos into store } } - if (ok && pmmpthread_store_write(object, &zmember, value, PMMPTHREAD_STORE_NO_COERCE_ARRAY) == FAILURE && !EG(exception)) { - zend_throw_error( - pmmpthread_ce_nts_value_error, - "Cannot assign non-thread-safe value of type %s to thread-safe class property %s::$%s", - zend_zval_type_name(value), - ZSTR_VAL(object->ce->name), - ZSTR_VAL(member) - ); + if (info != NULL) { + ZVAL_STR(&zmember, info->name); + real_value = OBJ_PROP(object, info->offset); + } else if (object->properties != NULL) { + ZVAL_STR(&zmember, member); + real_value = zend_hash_find(object->properties, member); + } + if (real_value != NULL && real_value == value) { //if real_value doesn't match, a hook or magic method was probably involved + zend_bool cached = 0; + if (pmmpthread_store_write_ex(object, &zmember, info, real_value, PMMPTHREAD_STORE_NO_COERCE_ARRAY, &cached) == FAILURE && !EG(exception)) { + zend_throw_error( + pmmpthread_ce_nts_value_error, + "Cannot assign non-thread-safe value of type %s to thread-safe class property %s::$%s", + zend_zval_type_name(real_value), + ZSTR_VAL(object->ce->name), + ZSTR_VAL(member) + ); + value = &EG(error_zval); + } + if (!cached) { + pmmpthread_store_clean_local_property(object, &zmember, info); + } } } } - zval_ptr_dtor(&tmp); - return EG(exception) ? &EG(error_zval) : value; } /* }}} */ @@ -249,18 +264,17 @@ int pmmpthread_has_property_deny(PMMPTHREAD_HAS_PROPERTY_PASSTHRU_D) { /* {{{ */ void pmmpthread_unset_dimension(PMMPTHREAD_UNSET_DIMENSION_PASSTHRU_D) { - pmmpthread_store_delete(object, member); + pmmpthread_store_delete(object, member, NULL); } void pmmpthread_unset_property(PMMPTHREAD_UNSET_PROPERTY_PASSTHRU_D) { zval zmember; zend_guard* guard; - ZVAL_STR(&zmember, member); - if (object->ce->__unset && (guard = zend_get_property_guard(object, member)) && !((*guard) & IN_UNSET)) { zval rv; ZVAL_UNDEF(&rv); + ZVAL_STR(&zmember, member); (*guard) |= IN_UNSET; zend_call_known_instance_method_with_1_params(object->ce->__unset, object, &rv, &zmember); @@ -270,12 +284,18 @@ void pmmpthread_unset_property(PMMPTHREAD_UNSET_PROPERTY_PASSTHRU_D) { zval_dtor(&rv); } } else { - zend_property_info* info = zend_get_property_info(object->ce, member, 0); - if (info != ZEND_WRONG_PROPERTY_INFO) { - if (info != NULL && PMMPTHREAD_OBJECT_PROPERTY(info)) { - ZVAL_STR(&zmember, info->name); //defined property, use mangled name + zend_std_unset_property(object, member, NULL); + if (!EG(exception)) { + zend_property_info* info = zend_get_property_info(object->ce, member, 1); + if (info != NULL && !PMMPTHREAD_OBJECT_PROPERTY(info)) { + info = NULL; //don't send invalid infos into store + } + if (info != NULL) { + ZVAL_STR(&zmember, info->name); + } else { + ZVAL_STR(&zmember, member); } - pmmpthread_store_delete(object, &zmember); + pmmpthread_store_delete(object, &zmember, info); } } } diff --git a/src/object.c b/src/object.c index 0b9dac98..c4762db9 100644 --- a/src/object.c +++ b/src/object.c @@ -179,14 +179,31 @@ void pmmpthread_current_thread(zval *return_value) { } /* }}} */ /* {{{ */ -static inline int _pmmpthread_connect_nolock(pmmpthread_zend_object_t* source, pmmpthread_zend_object_t* destination) { - if (source && destination) { +static inline int _pmmpthread_connect_no_global_lock(pmmpthread_zend_object_t* source, pmmpthread_zend_object_t* destination) { + if (source && destination && pmmpthread_monitor_lock(&source->ts_obj->monitor)) { destination->ts_obj = source->ts_obj; ++destination->ts_obj->refcount; if (destination->std.properties) zend_hash_clean(destination->std.properties); + for (int i = 0; i < destination->std.ce->default_properties_count; i++) { + zend_property_info* prop_info = destination->std.ce->properties_info_table[i]; + if (!prop_info || !PMMPTHREAD_OBJECT_PROPERTY(prop_info)) { + continue; + } + + zval* local = OBJ_PROP(destination, prop_info->offset); + zval* shared = zend_hash_find(&source->ts_obj->props.hash, prop_info->name); + + //ensure connected objects reflect unset/uninit property state correctly + //indirections will point to PMMPTHREAD_G if they exist + Z_PROP_FLAG_P(local) = Z_TYPE_P(shared) == IS_INDIRECT ? + Z_PROP_FLAG_P(Z_INDIRECT_P(shared)) : + 0; + } + + pmmpthread_monitor_unlock(&source->ts_obj->monitor); return SUCCESS; } else return FAILURE; } /* }}} */ @@ -195,7 +212,7 @@ static inline int _pmmpthread_connect_nolock(pmmpthread_zend_object_t* source, p static int pmmpthread_connect(pmmpthread_zend_object_t* source, pmmpthread_zend_object_t* destination) { int result = FAILURE; if(pmmpthread_globals_lock()){ - result = _pmmpthread_connect_nolock(source, destination); + result = _pmmpthread_connect_no_global_lock(source, destination); pmmpthread_globals_unlock(); } return result; @@ -243,41 +260,43 @@ static inline void pmmpthread_base_write_property_defaults(pmmpthread_zend_objec zend_class_entry* ce = base->std.ce; - while (ce != NULL) { - ZEND_HASH_FOREACH_PTR(&ce->properties_info, info) { - zval* value; - int result; + for (int i = 0; i < ce->default_properties_count; i++) { + info = ce->properties_info_table[i]; + if (info == NULL || !PMMPTHREAD_OBJECT_PROPERTY(info)) { + continue; + } - if (!PMMPTHREAD_OBJECT_PROPERTY(info)) { - continue; - } + zval* value; + int result; - zend_string* interned_name = pmmpthread_globals_add_interned_string(info->name); - ZVAL_INTERNED_STR(&key, interned_name); - - value = OBJ_PROP(&base->std, info->offset); - if (!Z_ISUNDEF_P(value)) { - result = pmmpthread_store_write( - &base->std, &key, - value, - PMMPTHREAD_STORE_NO_COERCE_ARRAY - ); - if (result == FAILURE) { - zend_throw_error( - NULL, - "Cannot use non-thread-safe default of type %s for thread-safe class property %s::$%s", - zend_zval_type_name(value), - ZSTR_VAL(ce->name), - ZSTR_VAL(Z_STR(key)) - ); - break; - } - zval_ptr_dtor(value); - ZVAL_UNDEF(value); - } - } ZEND_HASH_FOREACH_END(); + zend_string* interned_name = pmmpthread_globals_add_interned_string(info->name); + ZVAL_INTERNED_STR(&key, interned_name); + + value = OBJ_PROP(&base->std, info->offset); + if (Z_ISUNDEF_P(value)) { + ZEND_ASSERT(Z_PROP_FLAG_P(value) & IS_PROP_UNINIT); + value = &PMMPTHREAD_G(uninitialized_property); + } + result = pmmpthread_store_write( + &base->std, + &key, + info, + value, + PMMPTHREAD_STORE_NO_COERCE_ARRAY + ); + if (result == FAILURE) { + zend_throw_error( + NULL, + "Cannot use non-thread-safe default of type %s for thread-safe class property %s::$%s", + zend_zval_type_name(value), + ZSTR_VAL(ce->name), + ZSTR_VAL(Z_STR(key)) + ); + break; + } + zval_ptr_dtor(value); + ZVAL_UNDEF(value); - ce = ce->parent; } } /* }}} */ diff --git a/src/pmmpthread.h b/src/pmmpthread.h index 840238f9..79b1abbb 100644 --- a/src/pmmpthread.h +++ b/src/pmmpthread.h @@ -163,11 +163,10 @@ typedef struct _pmmpthread_call_t { #define PMMPTHREAD_CALL_EMPTY {empty_fcall_info, empty_fcall_info_cache} -#if PHP_VERSION_ID >= 80400 -#define PMMPTHREAD_OBJECT_PROPERTY(prop_info) ((prop_info->flags & (ZEND_ACC_STATIC | ZEND_ACC_VIRTUAL)) == 0) -#else -#define PMMPTHREAD_OBJECT_PROPERTY(prop_info) ((prop_info->flags & ZEND_ACC_STATIC) == 0) +#if PHP_VERSION_ID < 80400 +#define ZEND_ACC_VIRTUAL 0 #endif +#define PMMPTHREAD_OBJECT_PROPERTY(prop_info) ((prop_info->flags & (ZEND_ACC_STATIC | ZEND_ACC_VIRTUAL)) == 0) /* this is a copy of the same struct in zend_closures.c, which unfortunately isn't exported */ typedef struct _zend_closure { diff --git a/src/routine.c b/src/routine.c index 8cca1f9c..2c485532 100644 --- a/src/routine.c +++ b/src/routine.c @@ -242,7 +242,7 @@ zend_bool pmmpthread_join(pmmpthread_zend_object_t* thread) { //now, synchronize all object properties that may have been assigned by the thread if (pmmpthread_monitor_lock(&thread->ts_obj->monitor)) { - pmmpthread_store_full_sync_local_properties(&thread->std); + pmmpthread_store_cache_all(&thread->std); pmmpthread_monitor_unlock(&thread->ts_obj->monitor); } if (thread->worker_data != NULL) { @@ -250,7 +250,7 @@ zend_bool pmmpthread_join(pmmpthread_zend_object_t* thread) { } pmmpthread_zend_object_t* user_globals = PMMPTHREAD_ZG(thread_shared_globals); if (pmmpthread_monitor_lock(&user_globals->ts_obj->monitor)) { - pmmpthread_store_full_sync_local_properties(&user_globals->std); + pmmpthread_store_cache_all(&user_globals->std); pmmpthread_monitor_unlock(&user_globals->ts_obj->monitor); } diff --git a/src/store.c b/src/store.c index 002501c2..62e876f5 100644 --- a/src/store.c +++ b/src/store.c @@ -68,90 +68,101 @@ static void pmmpthread_store_init_local_properties(zend_object* object) { } } /* }}} */ -void pmmpthread_store_sync_local_properties(zend_object* object) { /* {{{ */ +static bool cache_item_stale(const zval* val, const pmmpthread_storage* ts_val) { + zend_bool remove = 1; + if (ts_val) { + ZVAL_DEINDIRECT(val); + if (ts_val->type == STORE_TYPE_THREADSAFE_OBJECT && IS_THREADSAFE_CLASS_INSTANCE(val)) { + pmmpthread_zend_object_t* shared = ((pmmpthread_zend_object_storage_t*)ts_val)->object; + pmmpthread_zend_object_t* local = PMMPTHREAD_FETCH_FROM(Z_OBJ_P(val)); + + if ( + shared == local || //same object + (pmmpthread_globals_object_valid(shared) && shared->ts_obj == local->ts_obj) //connection to a valid foreign object + ) { + remove = 0; + } + } else if (ts_val->type == STORE_TYPE_CLOSURE && IS_CLOSURE_OBJECT(val)) { + zend_closure* shared = ((pmmpthread_closure_storage_t*)ts_val)->closure; + zend_closure* local = (zend_closure*)Z_OBJ_P(val); + if (shared == local) { + remove = 0; + } +#if HAVE_PMMPTHREAD_EXT_SOCKETS_SUPPORT + } else if (ts_val->type == STORE_TYPE_SOCKET && IS_EXT_SOCKETS_OBJECT(val)) { + pmmpthread_socket_storage_t* shared = (pmmpthread_socket_storage_t*)ts_val; + php_socket* local = Z_SOCKET_P(val); + if (shared->bsd_socket == local->bsd_socket) { + remove = 0; + } +#endif + } else if (ts_val->type == STORE_TYPE_STRING_PTR && Z_TYPE_P(val) == IS_STRING) { + pmmpthread_string_storage_t* string = (pmmpthread_string_storage_t*)ts_val; + if (string->owner.ls == TSRMLS_CACHE && string->string == Z_STR_P(val)) { + //local caching of this by other threads is probably fine too, but fully caching it would probably + //require bytewise comparison, which isn't gonna be very performant + //it should be sufficient to only persist the owner thread's ref, since that's where copies will be + //made from anyway. + remove = 0; + } + } + } + + return remove; +} + +void pmmpthread_store_clean_stale_cache(zend_object* object) { /* {{{ */ pmmpthread_zend_object_t *threaded = PMMPTHREAD_FETCH_FROM(object); pmmpthread_object_t *ts_obj = threaded->ts_obj; zend_ulong idx; zend_string *name; zval *val; pmmpthread_storage *ts_val; - zend_bool remove; if (threaded->local_props_modcount == ts_obj->props.modcount) { return; } + for (int prop_index = 0; prop_index < object->ce->default_properties_count; prop_index++) { + zend_property_info* prop_info = object->ce->properties_info_table[prop_index]; + if (!prop_info) { + continue; + } + + ts_val = TRY_PMMPTHREAD_STORAGE_PTR_P(zend_hash_find(&ts_obj->props.hash, prop_info->name)); + val = OBJ_PROP(object, prop_info->offset); + + if (cache_item_stale(val, ts_val)) { + zval_ptr_dtor(val); + ZVAL_UNDEF(val); + } + } + if (threaded->std.properties) { ZEND_HASH_FOREACH_KEY_VAL(threaded->std.properties, idx, name, val) { + if (Z_TYPE_P(val) == IS_INDIRECT) { + //ptr to known property, don't mess with it + continue; + } if (!name) { ts_val = TRY_PMMPTHREAD_STORAGE_PTR_P(zend_hash_index_find(&ts_obj->props.hash, idx)); } else { ts_val = TRY_PMMPTHREAD_STORAGE_PTR_P(zend_hash_find(&ts_obj->props.hash, name)); } - remove = 1; - if (ts_val) { - ZVAL_DEINDIRECT(val); - if (ts_val->type == STORE_TYPE_THREADSAFE_OBJECT && IS_THREADSAFE_CLASS_INSTANCE(val)) { - pmmpthread_zend_object_t* shared = ((pmmpthread_zend_object_storage_t*)ts_val)->object; - pmmpthread_zend_object_t* local = PMMPTHREAD_FETCH_FROM(Z_OBJ_P(val)); - - if ( - shared == local || //same object - (pmmpthread_globals_object_valid(shared) && shared->ts_obj == local->ts_obj) //connection to a valid foreign object - ) { - remove = 0; - } - } else if (ts_val->type == STORE_TYPE_CLOSURE && IS_CLOSURE_OBJECT(val)) { - zend_closure* shared = ((pmmpthread_closure_storage_t*)ts_val)->closure; - zend_closure* local = (zend_closure*)Z_OBJ_P(val); - if (shared == local) { - remove = 0; - } -#if HAVE_PMMPTHREAD_EXT_SOCKETS_SUPPORT - } else if (ts_val->type == STORE_TYPE_SOCKET && IS_EXT_SOCKETS_OBJECT(val)) { - pmmpthread_socket_storage_t* shared = (pmmpthread_socket_storage_t*)ts_val; - php_socket* local = Z_SOCKET_P(val); - if (shared->bsd_socket == local->bsd_socket) { - remove = 0; - } -#endif - } else if (ts_val->type == STORE_TYPE_STRING_PTR && Z_TYPE_P(val) == IS_STRING) { - pmmpthread_string_storage_t* string = (pmmpthread_string_storage_t*)ts_val; - if (string->owner.ls == TSRMLS_CACHE && string->string == Z_STR_P(val)) { - //local caching of this by other threads is probably fine too, but fully caching it would probably - //require bytewise comparison, which isn't gonna be very performant - //it should be sufficient to only persist the owner thread's ref, since that's where copies will be - //made from anyway. - remove = 0; - } - } - } - - if (remove) { + if (cache_item_stale(val, ts_val)) { if (!name) { zend_hash_index_del(threaded->std.properties, idx); - } - else { + } else { zend_hash_del(threaded->std.properties, name); } } } ZEND_HASH_FOREACH_END(); - HT_FLAGS(threaded->std.properties) &= ~HASH_FLAG_HAS_EMPTY_IND; } + threaded->local_props_modcount = ts_obj->props.modcount; } /* }}} */ -static inline zend_bool pmmpthread_store_retain_in_local_cache(zval* val) { - return IS_THREADSAFE_CLASS_INSTANCE(val) || IS_CLOSURE_OBJECT(val) || IS_EXT_SOCKETS_OBJECT(val) || Z_TYPE_P(val) == IS_STRING; -} - -static inline zend_bool pmmpthread_store_valid_local_cache_item(zval* val) { - //zend_std_get_properties_ex() may add IS_INDIRECT zvals to point to the linear property table - //we don't want that, because they aren't used by pmmpthread and are always uninitialized - return Z_TYPE_P(val) != IS_INDIRECT; -} - /* {{{ */ static inline zend_bool pmmpthread_store_storage_is_cacheable(zval* zstorage) { if (zstorage == NULL) { @@ -172,7 +183,7 @@ static inline zend_bool pmmpthread_store_storage_is_pmmpthread_obj(zval* zstorag } /* }}} */ /* {{{ Syncs all the cacheable properties from TS storage into local cache */ -void pmmpthread_store_full_sync_local_properties(zend_object *object) { +void pmmpthread_store_cache_all(zend_object *object) { pmmpthread_zend_object_t* threaded = PMMPTHREAD_FETCH_FROM(object); if (GC_IS_RECURSIVE(&threaded->std)) { @@ -184,47 +195,70 @@ void pmmpthread_store_full_sync_local_properties(zend_object *object) { } GC_PROTECT_RECURSION(&threaded->std); - pmmpthread_store_sync_local_properties(object); //remove any outdated cache elements + pmmpthread_store_clean_stale_cache(object); //remove any unwanted cache elements + + //TODO: we might be able to avoid initializing this if there are no dynamic properties + pmmpthread_store_init_local_properties(object); pmmpthread_object_t* ts_obj = threaded->ts_obj; zend_long idx; zend_string* name; zval* zstorage; + zval* cached; - pmmpthread_store_init_local_properties(&threaded->std); + for (int prop_index = 0; prop_index < object->ce->default_properties_count; prop_index++) { + zend_property_info* prop_info = object->ce->properties_info_table[prop_index]; + if (!prop_info || !PMMPTHREAD_OBJECT_PROPERTY(prop_info)) { + continue; + } + + zstorage = zend_hash_find(&ts_obj->props.hash, prop_info->name); + cached = OBJ_PROP(object, prop_info->offset); + if (Z_ISUNDEF_P(cached) && pmmpthread_store_storage_is_cacheable(zstorage)) { + pmmpthread_store_restore_zval(cached, zstorage); + } + + if (pmmpthread_store_storage_is_pmmpthread_obj(zstorage)) { + pmmpthread_store_cache_all(Z_OBJ_P(cached)); + } + } ZEND_HASH_FOREACH_KEY_VAL(&ts_obj->props.hash, idx, name, zstorage) { - zval* cached; zval pzval; + ZVAL_UNDEF(&pzval); //we just synced local cache, so if something is already here, it doesn't need to be modified - if (!name) { - cached = zend_hash_index_find(threaded->std.properties, idx); - } else { - cached = zend_hash_find(threaded->std.properties, name); - } - if (cached && pmmpthread_store_valid_local_cache_item(cached)) { - if (pmmpthread_store_storage_is_pmmpthread_obj(zstorage)) { - pmmpthread_store_full_sync_local_properties(Z_OBJ_P(cached)); + cached = name ? + zend_hash_find(threaded->std.properties, name) : + zend_hash_index_find(threaded->std.properties, idx); + + if (cached) { + if (Z_TYPE_P(cached) == IS_INDIRECT) { + //ptr to known property which we've already processed + continue; } + //we still need to call store_cache_all() on thread-safe objects even if cached + } else if (pmmpthread_store_storage_is_cacheable(zstorage)) { + pmmpthread_store_restore_zval(&pzval, zstorage); + cached = &pzval; + } else { + //not cached or cacheable continue; } - if (pmmpthread_store_storage_is_cacheable(zstorage)) { - pmmpthread_store_restore_zval(&pzval, zstorage); - if (pmmpthread_store_storage_is_pmmpthread_obj(zstorage)) { - pmmpthread_store_full_sync_local_properties(Z_OBJ(pzval)); - } + if (pmmpthread_store_storage_is_pmmpthread_obj(zstorage)) { + pmmpthread_store_cache_all(Z_OBJ_P(cached)); + } - if (!name) { - if (!zend_hash_index_update(threaded->std.properties, idx, &pzval)) { - zval_ptr_dtor(&pzval); - } - } else { + if (!Z_ISUNDEF(pzval)) { + zval* update = name ? /* we can't use zend_hash_update() here - the string from store.props must not be returned to user code */ - if (!zend_hash_str_update(threaded->std.properties, ZSTR_VAL(name), ZSTR_LEN(name), &pzval)) - zval_ptr_dtor(&pzval); + zend_hash_str_update(threaded->std.properties, ZSTR_VAL(name), ZSTR_LEN(name), &pzval) : + zend_hash_index_update(threaded->std.properties, idx, &pzval); + + if (!update) { + zval_ptr_dtor(&pzval); } } } ZEND_HASH_FOREACH_END(); @@ -280,8 +314,90 @@ static inline zend_bool pmmpthread_store_member_is_cacheable(zend_object *object return pmmpthread_store_storage_is_cacheable(zstorage); } /* }}} */ + +static inline void pmmpthread_store_update_local_property(zend_object* object, zval* key, zend_property_info* prop_info, zval* value) { + zval* property; + if (prop_info != NULL && prop_info != ZEND_WRONG_PROPERTY_INFO) { + Z_TRY_ADDREF_P(value); + property = OBJ_PROP(object, prop_info->offset); + zval_ptr_dtor(property); + ZVAL_COPY_VALUE(property, value); + } else { + pmmpthread_store_init_local_properties(object); + property = Z_TYPE_P(key) == IS_LONG ? + zend_hash_index_find(object->properties, Z_LVAL_P(key)) : + zend_hash_find(object->properties, Z_STR_P(key)); + if (property != value) { + Z_TRY_ADDREF_P(value); + if (Z_TYPE_P(key) == IS_LONG) { + zend_hash_index_update(object->properties, Z_LVAL_P(key), value); + } else { + zend_string* str_key = Z_STR_P(key); + //update_ind() ensures defined properties get properly updated even if we didn't have prop_info + if ((GC_FLAGS(str_key) & (IS_STR_PERSISTENT | IS_STR_INTERNED)) == IS_STR_PERSISTENT) { + //refcounted persistent string from pmmpthread_store - we can't use it directly + //if a bucket with this key already exists, it'll be reused + zend_hash_str_update_ind(object->properties, Z_STRVAL_P(key), Z_STRLEN_P(key), value); + } else { + //any other interned or emalloc'd strings should be safe to use directly here + zend_hash_update_ind(object->properties, str_key, value); + } + } + } + } +} + +static inline zend_bool pmmpthread_store_update_shared_property(pmmpthread_object_t* ts_obj, zval* key, zval* zstorage) { + zend_bool result = FAILURE; + if (Z_TYPE_P(key) == IS_LONG) { + if (zend_hash_index_update(&ts_obj->props.hash, Z_LVAL_P(key), zstorage)) + result = SUCCESS; + } else { + zend_string* str_key = Z_STR_P(key); + if (GC_FLAGS(str_key) & IS_STR_PERMANENT) { + //only permanent strings can be used directly + if (zend_hash_update(&ts_obj->props.hash, str_key, zstorage)) { + result = SUCCESS; + } + } else { + //refcounted or request-local interned string - this must be hard-copied, regardless of where it came from + if (zend_hash_str_update(&ts_obj->props.hash, ZSTR_VAL(str_key), ZSTR_LEN(str_key), zstorage)) { + result = SUCCESS; + } + } + } + + return result; +} + +void pmmpthread_store_clean_local_property(zend_object* object, zval* key, zend_property_info* prop_info) { + zval* property; + if (prop_info != NULL) { + property = OBJ_PROP(object, prop_info->offset); + zval_ptr_dtor(property); + ZVAL_UNDEF(property); + } else if (object->properties != NULL) { + if (Z_TYPE_P(key) == IS_LONG) { + zend_hash_index_del(object->properties, Z_LVAL_P(key)); + } else { + zval* property = zend_hash_find(object->properties, Z_STR_P(key)); + if (property) { + if (Z_TYPE_P(property) == IS_INDIRECT) { + //known property, but we don't have prop_info + property = Z_INDIRECT_P(property); + zval_ptr_dtor(property); + ZVAL_UNDEF(property); + } else { + //dynamic property + zend_hash_del(object->properties, Z_STR_P(key)); + } + } + } + } +} + /* {{{ */ -int pmmpthread_store_delete(zend_object *object, zval *key) { +int pmmpthread_store_delete(zend_object *object, zval *key, zend_property_info* prop_info) { int result = FAILURE; zval member; pmmpthread_zend_object_t *threaded = PMMPTHREAD_FETCH_FROM(object); @@ -290,9 +406,15 @@ int pmmpthread_store_delete(zend_object *object, zval *key) { if (pmmpthread_monitor_lock(&ts_obj->monitor)) { zend_bool was_pmmpthread_object = pmmpthread_store_member_is_cacheable(object, &member); - if (Z_TYPE(member) == IS_LONG) { - result = zend_hash_index_del(&ts_obj->props.hash, Z_LVAL(member)); - } else result = zend_hash_del(&ts_obj->props.hash, Z_STR(member)); + if (prop_info != NULL) { + zval indirect; + ZVAL_INDIRECT(&indirect, &PMMPTHREAD_G(unset_property)); + result = pmmpthread_store_update_shared_property(ts_obj, &member, &indirect); + } else { + if (Z_TYPE(member) == IS_LONG) { + result = zend_hash_index_del(&ts_obj->props.hash, Z_LVAL(member)); + } else result = zend_hash_del(&ts_obj->props.hash, Z_STR(member)); + } if (result == SUCCESS && was_pmmpthread_object) { _pmmpthread_store_bump_modcount_nolock(threaded); @@ -304,10 +426,8 @@ int pmmpthread_store_delete(zend_object *object, zval *key) { pmmpthread_monitor_unlock(&ts_obj->monitor); } else result = FAILURE; - if (result == SUCCESS && threaded->std.properties) { - if (Z_TYPE(member) == IS_LONG) { - zend_hash_index_del(threaded->std.properties, Z_LVAL(member)); - } else zend_hash_del(threaded->std.properties, Z_STR(member)); + if (result == SUCCESS) { + pmmpthread_store_clean_local_property(object, &member, prop_info); } if (coerced) @@ -336,6 +456,7 @@ zend_bool pmmpthread_store_isset(zend_object *object, zval *key, int has_set_exi isset = 1; if (has_set_exists == ZEND_PROPERTY_NOT_EMPTY) { switch (Z_TYPE_P(zstorage)) { + case IS_INDIRECT: //we only use IS_INDIRECT for uninit/unset known properties case IS_NULL: case IS_FALSE: isset = 0; @@ -373,7 +494,7 @@ zend_bool pmmpthread_store_isset(zend_object *object, zval *key, int has_set_exi break; } } else if (has_set_exists == ZEND_PROPERTY_ISSET) { - if (Z_TYPE_P(zstorage) == IS_NULL) { + if (Z_TYPE_P(zstorage) == IS_NULL || Z_TYPE_P(zstorage) == IS_INDIRECT) { isset = 0; } } else if (has_set_exists != ZEND_PROPERTY_EXISTS) { @@ -389,51 +510,8 @@ zend_bool pmmpthread_store_isset(zend_object *object, zval *key, int has_set_exi return isset; } /* }}} */ -static inline void pmmpthread_store_update_local_property(zend_object* object, zval* key, zval* value) { - if (pmmpthread_store_retain_in_local_cache(value)) { - pmmpthread_store_init_local_properties(object); - if (Z_TYPE_P(key) == IS_LONG) { - zend_hash_index_update(object->properties, Z_LVAL_P(key), value); - } else { - zend_string* str_key = Z_STR_P(key); - if ((GC_FLAGS(str_key) & (IS_STR_PERSISTENT|IS_STR_INTERNED)) == IS_STR_PERSISTENT) { - //refcounted persistent string from pmmpthread_store - we can't use it directly - //if a bucket with this key already exists, it'll be reused - zend_hash_str_update(object->properties, Z_STRVAL_P(key), Z_STRLEN_P(key), value); - } else { - //any other interned or emalloc'd strings should be safe to use directly here - zend_hash_update(object->properties, str_key, value); - } - } - Z_TRY_ADDREF_P(value); - } -} - -static inline zend_bool pmmpthread_store_update_shared_property(pmmpthread_object_t* ts_obj, zval* key, zval* zstorage) { - zend_bool result = FAILURE; - if (Z_TYPE_P(key) == IS_LONG) { - if (zend_hash_index_update(&ts_obj->props.hash, Z_LVAL_P(key), zstorage)) - result = SUCCESS; - } else { - zend_string* str_key = Z_STR_P(key); - if (GC_FLAGS(str_key) & IS_STR_PERMANENT) { - //only permanent strings can be used directly - if (zend_hash_update(&ts_obj->props.hash, str_key, zstorage)) { - result = SUCCESS; - } - } else { - //refcounted or request-local interned string - this must be hard-copied, regardless of where it came from - if (zend_hash_str_update(&ts_obj->props.hash, ZSTR_VAL(str_key), ZSTR_LEN(str_key), zstorage)) { - result = SUCCESS; - } - } - } - - return result; -} - /* {{{ */ -int pmmpthread_store_read(zend_object *object, zval *key, int type, zval *read) { +int pmmpthread_store_read_ex(zend_object *object, zval *key, zend_property_info *prop_info, int type, zval *read, zend_bool force_cache) { int result = FAILURE; zval member, *property = NULL; pmmpthread_zend_object_t *threaded = PMMPTHREAD_FETCH_FROM(object); @@ -441,17 +519,22 @@ int pmmpthread_store_read(zend_object *object, zval *key, int type, zval *read) zend_bool coerced = pmmpthread_store_coerce(key, &member); if (pmmpthread_monitor_lock(&ts_obj->monitor)) { - if (threaded->std.properties) { - pmmpthread_store_sync_local_properties(object); + if (prop_info != NULL || threaded->std.properties) { + pmmpthread_store_clean_stale_cache(object); /* check if there's still a ref in local cache after sync - this ensures ref reuse for ThreadSafe and Closure objects */ - if (Z_TYPE(member) == IS_LONG) { - property = zend_hash_index_find(threaded->std.properties, Z_LVAL(member)); - } else property = zend_hash_find(threaded->std.properties, Z_STR(member)); + if (prop_info) { + property = OBJ_PROP(object, prop_info->offset); + } else { + if (Z_TYPE(member) == IS_LONG) { + property = zend_hash_index_find(threaded->std.properties, Z_LVAL(member)); + } else property = zend_hash_find(threaded->std.properties, Z_STR(member)); + } - if (property && pmmpthread_store_valid_local_cache_item(property)) { + if (property && !Z_ISUNDEF_P(property)) { pmmpthread_monitor_unlock(&ts_obj->monitor); + //even if we don't have prop_info, this might be a known property ZVAL_DEINDIRECT(property); ZVAL_COPY(read, property); if (coerced) { @@ -481,8 +564,9 @@ int pmmpthread_store_read(zend_object *object, zval *key, int type, zval *read) if (result != SUCCESS) { ZVAL_UNDEF(read); - } else { - pmmpthread_store_update_local_property(&threaded->std, &member, read); + } else if (force_cache || pmmpthread_store_retain_in_local_cache(read)) { + //always cache this - we'll need it for std_read handler + pmmpthread_store_update_local_property(&threaded->std, &member, prop_info, read); } if (coerced) @@ -491,6 +575,10 @@ int pmmpthread_store_read(zend_object *object, zval *key, int type, zval *read) return result; } /* }}} */ +int pmmpthread_store_read(zend_object* object, zval* key, zend_property_info* prop_info, int type, zval* read) { + return pmmpthread_store_read_ex(object, key, prop_info, type, read, 0); +} + /* {{{ Copies strings (as needed) for use in thread-safe object tables */ static zend_string* pmmpthread_store_save_string(zend_string* string) { zend_string* result; @@ -516,7 +604,7 @@ static zend_string* pmmpthread_store_restore_string(zend_string* string) { } /* {{{ */ -int pmmpthread_store_write(zend_object *object, zval *key, zval *write, zend_bool coerce_array_to_threaded) { +int pmmpthread_store_write_ex(zend_object *object, zval *key, zend_property_info* prop_info, zval *write, zend_bool coerce_array_to_threaded, zend_bool *cached) { int result = FAILURE; zval vol, member, zstorage; pmmpthread_zend_object_t *threaded = @@ -524,6 +612,8 @@ int pmmpthread_store_write(zend_object *object, zval *key, zval *write, zend_boo pmmpthread_object_t *ts_obj = threaded->ts_obj; zend_bool coerced = 0; + *cached = 0; + if (Z_TYPE_P(write) == IS_ARRAY && coerce_array_to_threaded == PMMPTHREAD_STORE_COERCE_ARRAY) { /* coerce arrays into threaded objects */ object_init_ex( @@ -572,15 +662,16 @@ int pmmpthread_store_write(zend_object *object, zval *key, zval *write, zend_boo } //this isn't necessary for any specific property write, but since we don't have any other way to clean up local //cached ThreadSafe references that are dead, we have to take the opportunity - pmmpthread_store_sync_local_properties(object); + pmmpthread_store_clean_stale_cache(object); pmmpthread_monitor_unlock(&ts_obj->monitor); } if (result != SUCCESS) { pmmpthread_store_storage_dtor(&zstorage); - } else { - pmmpthread_store_update_local_property(&threaded->std, &member, write); + } else if (pmmpthread_store_retain_in_local_cache(write)) { + *cached = 1; + pmmpthread_store_update_local_property(&threaded->std, &member, prop_info, write); } if (coerced) @@ -589,6 +680,11 @@ int pmmpthread_store_write(zend_object *object, zval *key, zval *write, zend_boo return result; } /* }}} */ +int pmmpthread_store_write(zend_object* object, zval* key, zend_property_info* prop_info, zval* write, zend_bool coerce_array_to_threaded) { + zend_bool dummy = 0; + return pmmpthread_store_write_ex(object, key, prop_info, write, coerce_array_to_threaded, &dummy); +} + /* {{{ */ int pmmpthread_store_count(zend_object *object, zend_long *count) { pmmpthread_object_t* ts_obj = PMMPTHREAD_FETCH_TS_FROM(object); @@ -637,6 +733,8 @@ int pmmpthread_store_shift(zend_object *object, zval *member) { if (threaded->std.properties) { zend_hash_del(threaded->std.properties, Z_STR(key)); } + //TODO: this should also clear defined properties + //it's probably fine not to do this for now since shift() is exclusive to arrays? zend_string_release(Z_STR(key)); } @@ -701,6 +799,8 @@ int pmmpthread_store_chunk(zend_object *object, zend_long size, zend_bool preser if (threaded->std.properties) { zend_hash_del(threaded->std.properties, Z_STR(key)); } + //TODO: this should also clear defined properties + //it's probably fine not to do this for now since shift() is exclusive to arrays? zend_string_release(Z_STR(key)); } } @@ -754,6 +854,8 @@ int pmmpthread_store_pop(zend_object *object, zval *member) { if (threaded->std.properties) { zend_hash_del(threaded->std.properties, Z_STR(key)); } + //TODO: this should also clear defined properties + //it's probably fine not to do this for now since shift() is exclusive to arrays? zend_string_release(Z_STR(key)); } if (may_be_locally_cached) { @@ -780,78 +882,65 @@ void pmmpthread_store_tohash(zend_object *object, HashTable *hash) { if (pmmpthread_monitor_lock(&ts_obj->monitor)) { zend_string *name = NULL; zend_ulong idx; - zval *zstorage; - zend_bool changed = 0; - pmmpthread_store_sync_local_properties(object); + pmmpthread_store_clean_stale_cache(object); + for (int i = 0; i < object->ce->default_properties_count; i++) { + zend_property_info* info = object->ce->properties_info_table[i]; + if (info == NULL || !PMMPTHREAD_OBJECT_PROPERTY(info)) { + continue; + } - ZEND_HASH_FOREACH_KEY_VAL(&ts_obj->props.hash, idx, name, zstorage) { - zval *cached; zval pzval; - - //we just synced local cache, so if something is already here, it doesn't need to be modified - if (hash == threaded->std.properties) { - if (!name) { - cached = zend_hash_index_find(threaded->std.properties, idx); + zval zname; + ZVAL_STR(&zname, info->name); + pmmpthread_store_read_ex(object, &zname, info, BP_VAR_R, &pzval, hash == object->properties); + + //indirections should already exist if this is the normal property table + if (hash != object->properties) { + if (Z_ISUNDEF(pzval)) { + ZVAL_INDIRECT(&pzval, OBJ_PROP(object, info->offset)); + zend_hash_update(hash, info->name, &pzval); + HT_FLAGS(hash) |= HASH_FLAG_HAS_EMPTY_IND; } else { - cached = zend_hash_find(threaded->std.properties, name); - } - if (cached && pmmpthread_store_valid_local_cache_item(cached)) { - continue; - } - } else { - if (!name) { - cached = zend_hash_index_find(threaded->std.properties, idx); - if (cached && pmmpthread_store_valid_local_cache_item(cached)) { - zend_hash_index_update(hash, idx, cached); - Z_TRY_ADDREF_P(cached); - continue; - } - } else { - cached = zend_hash_find(threaded->std.properties, name); - if (cached && pmmpthread_store_valid_local_cache_item(cached)) { - /* we can't use zend_hash_update() here - the string from store.props must not be returned to user code */ - zend_hash_str_update(hash, ZSTR_VAL(name), ZSTR_LEN(name), cached); - Z_TRY_ADDREF_P(cached); - continue; - } + zend_hash_update(hash, info->name, &pzval); } } + } - pmmpthread_store_restore_zval(&pzval, zstorage); + ZEND_HASH_FOREACH_KEY(&ts_obj->props.hash, idx, name) { + zval pzval; + zval* existing; - if (!name) { - if (!zend_hash_index_update(hash, idx, &pzval)) { - zval_ptr_dtor(&pzval); - } + zval key; + if (name) { + ZVAL_STR(&key, name); + existing = zend_hash_find(hash, name); } else { - /* we can't use zend_hash_update() here - the string from store.props must not be returned to user code */ - if (!zend_hash_str_update(hash, ZSTR_VAL(name), ZSTR_LEN(name), &pzval)) - zval_ptr_dtor(&pzval); + ZVAL_LONG(&key, idx); + existing = zend_hash_index_find(hash, idx); } - changed = 1; - } ZEND_HASH_FOREACH_END(); - - for (int i = 0; i < object->ce->default_properties_count; i++) { - zend_property_info* info = object->ce->properties_info_table[i]; - if (info == NULL || !PMMPTHREAD_OBJECT_PROPERTY(info)) { + if (existing) { + //either this is a new hash, or clean cache, so if an element already exists, + //don't mess with it continue; } - zval pzval; - if (zend_hash_find(hash, info->name) == NULL) { - //uninitialized typed property - ZVAL_INDIRECT(&pzval, &object->properties_table[OBJ_PROP_TO_NUM(info->offset)]); - zend_hash_update(hash, info->name, &pzval); - HT_FLAGS(hash) |= HASH_FLAG_HAS_EMPTY_IND; - changed = 1; + pmmpthread_store_read_ex(object, &key, NULL, BP_VAR_R, &pzval, hash == object->properties); + if (hash != object->properties) { + if (name) { + /* we can't use zend_hash_update() here - the string from store.props must not be returned to user code */ + zend_hash_str_update(hash, ZSTR_VAL(name), ZSTR_LEN(name), &pzval); + } else { + zend_hash_index_update(hash, idx, &pzval); + } } - } + } ZEND_HASH_FOREACH_END(); - if (changed && hash == threaded->std.properties) { + if (hash == threaded->std.properties) { //if this is the object's own properties table, we need to ensure that junk added here //doesn't get incorrectly treated as gospel + //TODO: we should use get_properties_for() so we don't have to deal with this bullshit threaded->local_props_modcount = ts_obj->props.modcount - 1; } @@ -1024,6 +1113,14 @@ static pmmpthread_storage* pmmpthread_store_create(pmmpthread_ident_t* source, z static zend_result pmmpthread_store_save_zval(pmmpthread_ident_t* source, zval *zstorage, zval *write) { zend_result result = FAILURE; switch (Z_TYPE_P(write)) { + case IS_UNDEF: + if (write == &PMMPTHREAD_G(uninitialized_property)) { + ZVAL_INDIRECT(zstorage, &PMMPTHREAD_G(uninitialized_property)); + } else { + ZVAL_INDIRECT(zstorage, &PMMPTHREAD_G(unset_property)); + } + result = SUCCESS; + break; case IS_NULL: case IS_FALSE: case IS_TRUE: @@ -1128,6 +1225,11 @@ static int pmmpthread_store_convert(pmmpthread_storage *storage, zval *pzval){ static void pmmpthread_store_restore_zval_ex(zval *unstore, zval *zstorage, zend_bool *may_be_locally_cached) { *may_be_locally_cached = pmmpthread_store_storage_is_cacheable(zstorage); switch (Z_TYPE_P(zstorage)) { + case IS_INDIRECT: + //we only use IS_INDIRECT for uninit/unset known properties + //ZVAL_COPY_PROP ensures retention of Z_PROP_FLAG + ZVAL_COPY_PROP(unstore, Z_INDIRECT_P(zstorage)); + break; case IS_NULL: case IS_FALSE: case IS_TRUE: @@ -1248,6 +1350,8 @@ int pmmpthread_store_merge(zend_object *destination, zval *from, zend_bool overw zend_hash_index_del(destination->properties, Z_LVAL(key)); } else { zend_hash_del(destination->properties, Z_STR(key)); + //TODO: this should also clear defined properties + //it's probably fine not to do this for now since this operation is exclusive to arrays? } } } @@ -1297,7 +1401,7 @@ int pmmpthread_store_merge(zend_object *destination, zval *from, zend_bool overw if (!overwrite && zend_hash_index_exists(&ts_obj->props.hash, Z_LVAL(key))) { goto next; } - if (pmmpthread_store_write(destination, &key, pzval, coerce_array_to_threaded) == FAILURE) { + if (pmmpthread_store_write(destination, &key, NULL, pzval, coerce_array_to_threaded) == FAILURE) { zend_throw_error( pmmpthread_ce_nts_value_error, "Cannot merge non-thread-safe value of type %s (input key %zd) into %s", @@ -1312,7 +1416,7 @@ int pmmpthread_store_merge(zend_object *destination, zval *from, zend_bool overw if (!overwrite && zend_hash_exists(&ts_obj->props.hash, Z_STR(key))) { goto next; } - if (pmmpthread_store_write(destination, &key, pzval, coerce_array_to_threaded) == FAILURE) { + if (pmmpthread_store_write(destination, &key, NULL, pzval, coerce_array_to_threaded) == FAILURE) { zend_throw_error( pmmpthread_ce_nts_value_error, "Cannot merge non-thread-safe value of type %s (input key \"%s\") into %s", @@ -1369,6 +1473,24 @@ static void pmmpthread_store_storage_dtor (zval *zstorage){ } } /* }}} */ +static void iterator_skip_uninit(pmmpthread_object_t* ts_obj, HashPosition* position) { + while (true) { + if (zend_hash_has_more_elements_ex(&ts_obj->props.hash, position) == FAILURE) { + *position = HT_INVALID_IDX; + break; + } + zval* current = zend_hash_get_current_data_ex(&ts_obj->props.hash, position); + + if (Z_TYPE_P(current) == IS_INDIRECT) { + //we currently only use IS_INDIRECT for uninit and unset properties + ZEND_ASSERT(Z_TYPE_P(Z_INDIRECT_P(current)) == IS_UNDEF); + zend_hash_move_forward_ex(&ts_obj->props.hash, position); + } else { + break; + } + } +} + /* {{{ iteration helpers */ void pmmpthread_store_reset(zend_object *object, HashPosition *position) { pmmpthread_object_t *ts_obj = PMMPTHREAD_FETCH_TS_FROM(object); @@ -1376,11 +1498,8 @@ void pmmpthread_store_reset(zend_object *object, HashPosition *position) { if (pmmpthread_monitor_lock(&ts_obj->monitor)) { if (ts_obj->props.first == HT_INVALID_IDX) { zend_hash_internal_pointer_reset_ex(&ts_obj->props.hash, position); - if (zend_hash_has_more_elements_ex(&ts_obj->props.hash, position) == FAILURE) { //empty - *position = HT_INVALID_IDX; - } else { - ts_obj->props.first = *position; - } + iterator_skip_uninit(ts_obj, position); + ts_obj->props.first = *position; } else { *position = ts_obj->props.first; } @@ -1416,7 +1535,7 @@ void pmmpthread_store_data(zend_object *object, zval *value, HashPosition *posit zval key; zend_hash_get_current_key_zval_ex(&ts_obj->props.hash, &key, position); - if (pmmpthread_store_read(object, &key, BP_VAR_R, value) == FAILURE) { + if (pmmpthread_store_read(object, &key, NULL, BP_VAR_R, value) == FAILURE) { ZVAL_UNDEF(value); } if (Z_TYPE(key) == IS_STRING) { @@ -1433,9 +1552,7 @@ void pmmpthread_store_forward(zend_object *object, HashPosition *position) { if (pmmpthread_monitor_lock(&ts_obj->monitor)) { zend_hash_move_forward_ex( &ts_obj->props.hash, position); - if (zend_hash_has_more_elements_ex(&ts_obj->props.hash, position) == FAILURE) { - *position = HT_INVALID_IDX; - } + iterator_skip_uninit(ts_obj, position); pmmpthread_monitor_unlock(&ts_obj->monitor); } } /* }}} */ diff --git a/src/store.h b/src/store.h index 9eb12ead..0f2acd36 100644 --- a/src/store.h +++ b/src/store.h @@ -38,13 +38,16 @@ typedef struct _pmmpthread_store_t { void pmmpthread_store_init(pmmpthread_store_t* store); void pmmpthread_store_destroy(pmmpthread_store_t* store); -void pmmpthread_store_sync_local_properties(zend_object* object); -void pmmpthread_store_full_sync_local_properties(zend_object *object); +void pmmpthread_store_clean_stale_cache(zend_object* object); +void pmmpthread_store_cache_all(zend_object *object); int pmmpthread_store_merge(zend_object *destination, zval *from, zend_bool overwrite, zend_bool coerce_array_to_threaded); -int pmmpthread_store_delete(zend_object *object, zval *key); -int pmmpthread_store_read(zend_object *object, zval *key, int type, zval *read); +void pmmpthread_store_clean_local_property(zend_object* object, zval* key, zend_property_info* prop_info); +int pmmpthread_store_delete(zend_object *object, zval *key, zend_property_info* prop_info); +int pmmpthread_store_read_ex(zend_object *object, zval *key, zend_property_info* prop_info, int type, zval *read, zend_bool force_cache); +int pmmpthread_store_read(zend_object *object, zval *key, zend_property_info* prop_info, int type, zval *read); zend_bool pmmpthread_store_isset(zend_object *object, zval *key, int has_set_exists); -int pmmpthread_store_write(zend_object *object, zval *key, zval *write, zend_bool coerce_array_to_threaded); +int pmmpthread_store_write_ex(zend_object *object, zval *key, zend_property_info* prop_info, zval *write, zend_bool coerce_array_to_threaded, zend_bool *cached); +int pmmpthread_store_write(zend_object *object, zval *key, zend_property_info* prop_info, zval *write, zend_bool coerce_array_to_threaded); void pmmpthread_store_tohash(zend_object *object, HashTable *hash); int pmmpthread_store_shift(zend_object *object, zval *member); int pmmpthread_store_chunk(zend_object *object, zend_long size, zend_bool preserve, zval *chunk); @@ -59,4 +62,7 @@ void pmmpthread_store_key(zend_object *object, zval *key, HashPosition *position void pmmpthread_store_data(zend_object *object, zval *value, HashPosition *position); void pmmpthread_store_forward(zend_object *object, HashPosition *position); /* }}} */ +static inline zend_bool pmmpthread_store_retain_in_local_cache(zval* val) { + return IS_THREADSAFE_CLASS_INSTANCE(val) || IS_CLOSURE_OBJECT(val) || IS_EXT_SOCKETS_OBJECT(val) || Z_TYPE_P(val) == IS_STRING; +} #endif diff --git a/src/worker.c b/src/worker.c index 00e0da60..b5da5767 100644 --- a/src/worker.c +++ b/src/worker.c @@ -118,7 +118,7 @@ zend_long pmmpthread_worker_collect_tasks(pmmpthread_worker_data_t *worker_data, item = worker_data->gc.head; zend_long tasks_collected = 0; while (item) { - pmmpthread_store_full_sync_local_properties(Z_OBJ(item->value)); + pmmpthread_store_cache_all(Z_OBJ(item->value)); if (collect(call, &item->value)) { item = item->next; @@ -149,7 +149,7 @@ zend_result pmmpthread_worker_sync_collectable_tasks(pmmpthread_worker_data_t* w while (item) { pmmpthread_zend_object_t* threaded = PMMPTHREAD_FETCH_FROM(Z_OBJ(item->value)); if (pmmpthread_monitor_lock(&threaded->ts_obj->monitor)) { - pmmpthread_store_full_sync_local_properties(Z_OBJ(item->value)); + pmmpthread_store_cache_all(Z_OBJ(item->value)); pmmpthread_monitor_unlock(&threaded->ts_obj->monitor); } item = item->next; diff --git a/tests/closure-assign-to-threaded.phpt b/tests/closure-assign-to-threaded.phpt index 392130c4..bba75f1e 100644 --- a/tests/closure-assign-to-threaded.phpt +++ b/tests/closure-assign-to-threaded.phpt @@ -3,7 +3,9 @@ Test assigning Closure to ThreadSafe doesn't break use()d variables --FILE-- func = function() use($name, $type) : void{ diff --git a/tests/foreign-private-members.phpt b/tests/foreign-private-members.phpt index d0282a3c..1235a34f 100644 --- a/tests/foreign-private-members.phpt +++ b/tests/foreign-private-members.phpt @@ -13,6 +13,7 @@ class MY { } class TEST extends \pmmp\thread\Thread { + private string $my; public function __construct($my) { $this->my = serialize($my); } diff --git a/tests/inherited-anon-class-outside-context.phpt b/tests/inherited-anon-class-outside-context.phpt index ceb51b6c..cbdae018 100644 --- a/tests/inherited-anon-class-outside-context.phpt +++ b/tests/inherited-anon-class-outside-context.phpt @@ -8,6 +8,8 @@ Unbound anon class causing segfaults, we delay copy but still cannot serialize t interface TestInterface{} class Test extends \pmmp\thread\Thread { + public bool $alive = false; + public \pmmp\thread\Thread $anonymous; /** * doccomment run */ @@ -25,6 +27,7 @@ class Test extends \pmmp\thread\Thread { protected $protProp; private $privProp; public static $staticProp; + public bool $ready; public function run() : void{ var_dump('anonymous run'); $this->ready = true; @@ -71,6 +74,8 @@ object(pmmp\thread\Thread@anonymous)#3 (3) { NULL ["privProp":"pmmp\thread\Thread@anonymous":private]=> NULL + ["ready"]=> + uninitialized(bool) } string(13) "anonymous run" object(pmmp\thread\Thread@anonymous)#4 (4) { diff --git a/tests/inherited-anon-class.phpt b/tests/inherited-anon-class.phpt index fcb90c7f..534be0af 100644 --- a/tests/inherited-anon-class.phpt +++ b/tests/inherited-anon-class.phpt @@ -5,6 +5,7 @@ Unbound anon class causing segfaults, we delay copy but still cannot serialize t --FILE-- prop = new class extends \pmmp\thread\ThreadSafe {}; var_dump($this->prop); diff --git a/tests/interface.phpt b/tests/interface.phpt index 1d3eb34a..7c77a4fd 100644 --- a/tests/interface.phpt +++ b/tests/interface.phpt @@ -9,7 +9,8 @@ interface INamedThread { function getName(); } -class TestThread extends \pmmp\thread\Thread implements INamedThread { +class TestThread extends \pmmp\thread\Thread implements INamedThread { + public string $name; public function setName($name) { $this->name = $name; } diff --git a/tests/magic-methods-get-set.phpt b/tests/magic-methods-get-set.phpt index 1ff012a4..ae6d7350 100644 --- a/tests/magic-methods-get-set.phpt +++ b/tests/magic-methods-get-set.phpt @@ -4,6 +4,8 @@ Test magic __get and __set This test verifies that __set and __get work as expected --FILE-- t = new \pmmp\thread\ThreadSafe(); + $this->t = new class extends \pmmp\thread\ThreadSafe{ + public bool $set; + }; $this->t->set = true; } @@ -27,18 +30,18 @@ $t->join(); --EXPECTF-- object(T)#%d (%d) { ["t"]=> - object(pmmp\thread\ThreadSafe)#%d (%d) { + object(%s@anonymous)#%d (%d) { ["set"]=> bool(true) } } -object(pmmp\thread\ThreadSafe)#%d (%d) { +object(%s@anonymous)#%d (%d) { ["set"]=> bool(true) } object(T)#%d (%d) { ["t"]=> - object(pmmp\thread\ThreadSafe)#%d (%d) { + object(%s@anonymous)#%d (%d) { ["set"]=> bool(true) } diff --git a/tests/normal-reads.phpt b/tests/normal-reads.phpt index 0ffbb8ed..b8af5547 100644 --- a/tests/normal-reads.phpt +++ b/tests/normal-reads.phpt @@ -5,6 +5,7 @@ This test verifies that reading properties from the object without var_dump/prin --FILE-- name = sprintf("%s", __CLASS__); } diff --git a/tests/object-comparison.phpt b/tests/object-comparison.phpt index 7f231b00..b9c073d0 100644 --- a/tests/object-comparison.phpt +++ b/tests/object-comparison.phpt @@ -15,9 +15,10 @@ comparison which would return true if the property tables of two distinct object --FILE-- arg1 = $arg1; - $this->arg2 = $arg2; + public function __construct( + private \pmmp\thread\ThreadSafeArray $arg1, + private \pmmp\thread\ThreadSafeArray $arg2 + ) { } public function run() : void{ diff --git a/tests/oomethods.phpt b/tests/oomethods.phpt index e732ecf1..97d7b59b 100644 --- a/tests/oomethods.phpt +++ b/tests/oomethods.phpt @@ -4,7 +4,8 @@ Test access to user defined methods in the object context User methods are now imported from your declared class into the thread --FILE-- value; } diff --git a/tests/php8-socket-copy-overwrite-basic.phpt b/tests/php8-socket-copy-overwrite-basic.phpt index a8d02972..0b77391f 100644 --- a/tests/php8-socket-copy-overwrite-basic.phpt +++ b/tests/php8-socket-copy-overwrite-basic.phpt @@ -5,7 +5,9 @@ Test that standard Socket objects get copied and overwritten properly --FILE-- socket = socket_create(AF_INET, SOCK_DGRAM, SOL_UDP); socket_connect($threaded->socket, "127.0.0.1", 19132); diff --git a/tests/property-exists.phpt b/tests/property-exists.phpt index d8b57522..264ab31b 100644 --- a/tests/property-exists.phpt +++ b/tests/property-exists.phpt @@ -12,7 +12,9 @@ function test(object $t) : void{ } test(new \stdClass); -test(new \pmmp\thread\ThreadSafe); +test(new class extends \pmmp\thread\ThreadSafe{ + public $prop; +}); ?> --EXPECT-- bool(true) diff --git a/tests/staticmethods.phpt b/tests/staticmethods.phpt index 1ee946e7..60c301bd 100644 --- a/tests/staticmethods.phpt +++ b/tests/staticmethods.phpt @@ -4,7 +4,8 @@ Test access to static methods from within user threads Static methods as declared in the users implementation of Thread should now be available for calling in the thread scope --FILE-- a = function(){}; diff --git a/tests/uninitialized-typed-properties.phpt b/tests/uninitialized-typed-properties.phpt index 37c3d873..ada50e47 100644 --- a/tests/uninitialized-typed-properties.phpt +++ b/tests/uninitialized-typed-properties.phpt @@ -32,6 +32,12 @@ function test(object $t1) : void{ var_dump($t1->c); var_dump(isset($t1->c)); var_dump(property_exists($t1, "c")); + + unset($t1->b); + var_dump($t1); + foreach($t1 as $name => $value){ + var_dump($name, $value); + } } echo "--- Normal object ---\n"; @@ -69,6 +75,16 @@ NULL NULL bool(false) bool(true) +object(NTS)#2 (1) { + ["a"]=> + uninitialized(int) + ["b"]=> + uninitialized(string) + ["c"]=> + NULL +} +string(1) "c" +NULL --- pthreads object --- --- properties are currently expected to be in an unstable order --- object(TS)#3 (3) { @@ -80,12 +96,12 @@ object(TS)#3 (3) { NULL } object(TS)#3 (2) { + ["a"]=> + uninitialized(int) ["b"]=> string(5) "hello" ["c"]=> NULL - ["a"]=> - uninitialized(int) } bool(false) bool(true) @@ -97,4 +113,14 @@ NULL NULL bool(false) bool(true) +object(TS)#3 (1) { + ["a"]=> + uninitialized(int) + ["b"]=> + uninitialized(string) + ["c"]=> + NULL +} +string(1) "c" +NULL --- Done --- diff --git a/tests/workers-no-collect.phpt b/tests/workers-no-collect.phpt index df9ca98d..b191f2aa 100644 --- a/tests/workers-no-collect.phpt +++ b/tests/workers-no-collect.phpt @@ -5,8 +5,9 @@ This test verifies that workers cannot be misused (collect) --FILE-- worker = $worker; + public function __construct( + private \pmmp\thread\Worker $worker + ) { } public function run() : void{ diff --git a/tests/workers-no-join.phpt b/tests/workers-no-join.phpt index fec52762..5cde2935 100644 --- a/tests/workers-no-join.phpt +++ b/tests/workers-no-join.phpt @@ -5,8 +5,9 @@ This test verifies that workers cannot be misused (join) --FILE-- worker = $worker; + public function __construct( + private \pmmp\thread\Worker $worker + ) { } public function run() : void{ @@ -21,11 +22,11 @@ $test->start(\pmmp\thread\Thread::INHERIT_ALL); $test->join(); ?> --EXPECTF-- -Fatal error: Uncaught RuntimeException: only the creator of this pmmp\thread\Worker may join with it in %s:8 +Fatal error: Uncaught RuntimeException: only the creator of this pmmp\thread\Worker may join with it in %s:%d Stack trace: -#0 %s(8): pmmp\thread\Worker->shutdown() +#0 %s(%d): pmmp\thread\Worker->shutdown() #1 [internal function]: Test->run() #2 {main} - thrown in %s on line 8 + thrown in %s on line %d diff --git a/tests/workers-no-stack.phpt b/tests/workers-no-stack.phpt index f22185f6..c37dd9ef 100644 --- a/tests/workers-no-stack.phpt +++ b/tests/workers-no-stack.phpt @@ -9,8 +9,9 @@ class Work extends \pmmp\thread\Runnable { } class Test extends \pmmp\thread\Thread { - public function __construct(\pmmp\thread\Worker $worker) { - $this->worker = $worker; + public function __construct( + private \pmmp\thread\Worker $worker + ) { } public function run() : void{ diff --git a/tests/workers-no-start.phpt b/tests/workers-no-start.phpt index b23f9e0b..5bdd8b3d 100644 --- a/tests/workers-no-start.phpt +++ b/tests/workers-no-start.phpt @@ -5,8 +5,9 @@ This test verifies that workers cannot be misused (start) --FILE-- worker = $worker; + public function __construct( + private \pmmp\thread\Worker $worker + ) { } public function run() : void{ @@ -20,11 +21,11 @@ $test->start(\pmmp\thread\Thread::INHERIT_ALL); $test->join(); ?> --EXPECTF-- -Fatal error: Uncaught RuntimeException: only the creator of this pmmp\thread\Worker may start it in %s:8 +Fatal error: Uncaught RuntimeException: only the creator of this pmmp\thread\Worker may start it in %s:%d Stack trace: -#0 %s(8): pmmp\thread\Thread->start(%d) +#0 %s(%d): pmmp\thread\Thread->start(%d) #1 [internal function]: Test->run() #2 {main} - thrown in %s on line 8 + thrown in %s on line %d diff --git a/tests/workers-no-unstack.phpt b/tests/workers-no-unstack.phpt index d7968df1..42351e94 100644 --- a/tests/workers-no-unstack.phpt +++ b/tests/workers-no-unstack.phpt @@ -5,8 +5,9 @@ This test verifies that workers cannot be misused (unstack) --FILE-- worker = $worker; + public function __construct( + private \pmmp\thread\Worker $worker + ) { } public function run() : void{ @@ -22,11 +23,11 @@ $test->start(\pmmp\thread\Thread::INHERIT_ALL); $test->join(); ?> --EXPECTF-- -Fatal error: Uncaught RuntimeException: only the creator of this pmmp\thread\Worker may call unstack in %s:8 +Fatal error: Uncaught RuntimeException: only the creator of this pmmp\thread\Worker may call unstack in %s:%d Stack trace: -#0 %s(8): pmmp\thread\Worker->unstack() +#0 %s(%d): pmmp\thread\Worker->unstack() #1 [internal function]: Test->run() #2 {main} - thrown in %s on line 8 + thrown in %s on line %d