Shim standard property handlers, in an attempt to move ThreadSafe behaviour closer to regular objects

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 */

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
 		}
 

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

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);
 		}
 	}
 }

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;
 	}
 } /* }}} */
 

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 {