Readonly properties

src/handlers.c

@@ -92,7 +92,14 @@ zval* pthreads_read_property(PTHREADS_READ_PROPERTY_PASSTHRU_D) {
 		} else {
 			//defined property, use mangled name
 			ZVAL_STR(&zmember, info->name);
+
+#if PHP_VERSION_ID >= 80100
+			if ((info->flags & ZEND_ACC_READONLY) == 0 || pthreads_store_read_local_property(object, member, type, rv) == FAILURE) {
+				pthreads_store_read(object, &zmember, type, rv);
+			}
+#else
 			pthreads_store_read(object, &zmember, type, rv);
+#endif
 
 			if (Z_ISUNDEF_P(rv)) {
 				if (type != BP_VAR_IS && !EG(exception)) {
@@ -139,28 +146,57 @@ zval* pthreads_write_property(PTHREADS_WRITE_PROPERTY_PASSTHRU_D) {
 	} 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 && (info->flags & ZEND_ACC_STATIC) == 0) {
-				ZVAL_STR(&zmember, info->name); //use mangled name to avoid private member shadowing issues
+			bool overwrite = true;
 
-				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));
+			if (info != NULL) {
+				if ((info->flags & ZEND_ACC_STATIC) == 0) {
+					ZVAL_STR(&zmember, info->name); //use mangled name to avoid private member shadowing issues
 
-				if (ZEND_TYPE_IS_SET(info->type) && !zend_verify_property_type(info, value, strict)) {
-					ok = false;
+					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));
+
+					if (ZEND_TYPE_IS_SET(info->type) && !zend_verify_property_type(info, value, strict)) {
+						ok = false;
+					}
 				}
+#if PHP_VERSION_ID >= 80100
+				overwrite = (info->flags & ZEND_ACC_READONLY) == 0;
+#endif
 			}
 
-			if (ok && pthreads_store_write(object, &zmember, value, PTHREADS_STORE_NO_COERCE_ARRAY) == FAILURE && !EG(exception)) {
-				zend_throw_error(
-					NULL,
-					"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 (ok) {
+				pthreads_store_write_result result = pthreads_store_write_ex(object, &zmember, value, PTHREADS_STORE_NO_COERCE_ARRAY, overwrite);
+				if (result != WRITE_SUCCESS && !EG(exception)) {
+					switch (result) {
+						case WRITE_FAIL_NOT_THREAD_SAFE: {
+							zend_throw_error(
+								NULL,
+								"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)
+							);
+							break;
+						}
+						case WRITE_FAIL_WOULD_OVERWRITE: {
+							zend_throw_error(
+								NULL,
+								"Cannot modify readonly property %s::$%s",
+								ZSTR_VAL(object->ce->name),
+								ZSTR_VAL(member)
+							);
+							break;
+						}
+						default: {
+							ZEND_ASSERT(0);
+							break;
+						}
+					}
+				}
 			}
 		}
 	}
@@ -231,10 +267,28 @@ void pthreads_unset_property(PTHREADS_UNSET_PROPERTY_PASSTHRU_D) {
 	} else {
 		zend_property_info* info = zend_get_property_info(object->ce, member, 0);
 		if (info != ZEND_WRONG_PROPERTY_INFO) {
-			if (info != NULL && (info->flags & ZEND_ACC_STATIC) == 0) {
-				ZVAL_STR(&zmember, info->name); //defined property, use mangled name
+			zend_bool ok = true;
+			if (info != NULL) {
+				if ((info->flags & ZEND_ACC_STATIC) == 0) {
+					ZVAL_STR(&zmember, info->name); //defined property, use mangled name
+				}
+
+#if PHP_VERSION_ID >= 80100
+				if ((info->flags & ZEND_ACC_READONLY) != 0) {
+					zend_throw_error(
+						NULL,
+						"Cannot unset readonly property %s::$%s",
+						ZSTR_VAL(object->ce->name),
+						ZSTR_VAL(member)
+					);
+					ok = false;
+				}
+#endif
+			}
+
+			if (ok) {
+				pthreads_store_delete(object, &zmember);
 			}
-			pthreads_store_delete(object, &zmember);
 		}
 	}
 }

src/store.c

@@ -406,6 +406,25 @@ static inline zend_bool pthreads_store_update_shared_property(pthreads_object_t*
 	return result;
 }
 
+/* {{{ */
+int pthreads_store_read_local_property(zend_object* object, zend_string* key, int type, zval* read) {
+	zval* property;
+	pthreads_zend_object_t* threaded = PTHREADS_FETCH_FROM(object);
+
+	if (threaded->std.properties) {
+		property = zend_hash_find(threaded->std.properties, key);
+
+		if (property && pthreads_store_valid_local_cache_item(property)) {
+			pthreads_monitor_unlock(&threaded->ts_obj->monitor);
+			ZVAL_DEINDIRECT(property);
+			ZVAL_COPY(read, property);
+			return SUCCESS;
+		}
+	}
+
+	return FAILURE;
+} /* }}} */
+
 /* {{{ */
 int pthreads_store_read(zend_object *object, zval *key, int type, zval *read) {
 	int result = FAILURE;
@@ -492,8 +511,8 @@ static zend_string* pthreads_store_restore_string(zend_string* string) {
 }
 
 /* {{{ */
-int pthreads_store_write(zend_object *object, zval *key, zval *write, zend_bool coerce_array_to_threaded) {
-	int result = FAILURE;
+pthreads_store_write_result pthreads_store_write_ex(zend_object *object, zval *key, zval *write, zend_bool coerce_array_to_threaded, zend_bool overwrite) {
+	pthreads_store_write_result result = WRITE_FAIL_UNKNOWN;
 	zval vol, member, zstorage;
 	pthreads_zend_object_t *threaded =
 		PTHREADS_FETCH_FROM(object);
@@ -511,7 +530,7 @@ int pthreads_store_write(zend_object *object, zval *key, zval *write, zend_bool
 	}
 
 	if (pthreads_store_save_zval(&threaded->owner, &zstorage, write) != SUCCESS) {
-		return FAILURE;
+		return WRITE_FAIL_NOT_THREAD_SAFE;
 	}
 
 	if (pthreads_monitor_lock(&ts_obj->monitor)) {
@@ -523,10 +542,26 @@ int pthreads_store_write(zend_object *object, zval *key, zval *write, zend_bool
 			coerced = pthreads_store_coerce(key, &member);
 		}
 
-		zend_bool was_pthreads_object = pthreads_store_member_is_cacheable(object, &member);
-		result = pthreads_store_update_shared_property(ts_obj, &member, &zstorage);
-		if (result == SUCCESS && was_pthreads_object) {
-			_pthreads_store_bump_modcount_nolock(threaded);
+		zend_bool ok = true;
+		if (!overwrite) {
+			if (Z_TYPE(member) == IS_LONG) {
+				ok = !zend_hash_index_exists(&ts_obj->props.hash, Z_LVAL(member));
+			} else {
+				ok = !zend_hash_exists(&ts_obj->props.hash, Z_STR(member));
+			}
+			if (!ok) {
+				result = WRITE_FAIL_WOULD_OVERWRITE;
+			}
+		}
+
+		if (ok) {
+			zend_bool was_pthreads_object = pthreads_store_member_is_cacheable(object, &member);
+			if (pthreads_store_update_shared_property(ts_obj, &member, &zstorage) == SUCCESS) {
+				result = WRITE_SUCCESS;
+				if (was_pthreads_object) {
+					_pthreads_store_bump_modcount_nolock(threaded);
+				}
+			}
 		}
 		//this isn't necessary for any specific property write, but since we don't have any other way to clean up local
 		//cached Threaded references that are dead, we have to take the opportunity
@@ -535,7 +570,7 @@ int pthreads_store_write(zend_object *object, zval *key, zval *write, zend_bool
 		pthreads_monitor_unlock(&ts_obj->monitor);
 	}
 
-	if (result != SUCCESS) {
+	if (result != WRITE_SUCCESS) {
 		pthreads_store_storage_dtor(&zstorage);
 	} else {
 		pthreads_store_update_local_property(&threaded->std, &member, write);
@@ -547,6 +582,10 @@ int pthreads_store_write(zend_object *object, zval *key, zval *write, zend_bool
 	return result;
 } /* }}} */
 
+int pthreads_store_write(zend_object* object, zval* key, zval* write, zend_bool coerce_array_to_threaded) {
+	return pthreads_store_write_ex(object, key, write, coerce_array_to_threaded, true) == WRITE_SUCCESS ? SUCCESS : FAILURE;
+}
+
 /* {{{ */
 int pthreads_store_count(zend_object *object, zend_long *count) {
 	pthreads_object_t* ts_obj = PTHREADS_FETCH_TS_FROM(object);

src/store.h

@@ -34,15 +34,24 @@ typedef struct _pthreads_store_t {
 	zend_long modcount;
 } pthreads_store_t;
 
+typedef enum {
+	WRITE_SUCCESS = 0,
+	WRITE_FAIL_UNKNOWN = -1,
+	WRITE_FAIL_NOT_THREAD_SAFE = -2,
+	WRITE_FAIL_WOULD_OVERWRITE = -3
+} pthreads_store_write_result;
+
 void pthreads_store_init(pthreads_store_t* store);
 void pthreads_store_destroy(pthreads_store_t* store);
 void pthreads_store_sync_local_properties(zend_object* object);
 void pthreads_store_full_sync_local_properties(zend_object *object);
 int pthreads_store_merge(zend_object *destination, zval *from, zend_bool overwrite, zend_bool coerce_array_to_threaded);
 int pthreads_store_delete(zend_object *object, zval *key);
 int pthreads_store_read(zend_object *object, zval *key, int type, zval *read);
+int pthreads_store_read_local_property(zend_object* object, zend_string* key, int type, zval* read);
 zend_bool pthreads_store_isset(zend_object *object, zval *key, int has_set_exists);
 int pthreads_store_write(zend_object *object, zval *key, zval *write, zend_bool coerce_array_to_threaded);
+pthreads_store_write_result pthreads_store_write_ex(zend_object *object, zval *key, zval *write, zend_bool coerce_array_to_threaded, zend_bool overwrite);
 void pthreads_store_tohash(zend_object *object, HashTable *hash);
 int pthreads_store_shift(zend_object *object, zval *member);
 int pthreads_store_chunk(zend_object *object, zend_long size, zend_bool preserve, zval *chunk);

tests/readonly-properties-thread-safe.phpt

@@ -0,0 +1,137 @@
+--TEST--
+Test that readonly properties behave as expected on thread-safe class descendents
+--DESCRIPTION--
+pthreads doesn't (yet) mirror the exact behaviour of readonly properties on standard objects
+for now, this test just ensures that the behaviour plays out as expected with lockless reads,
+since those use local cache without locks if possible.
+--SKIPIF--
+<?php if(PHP_VERSION_ID < 80100) die("skip readonly properties are only supported in 8.1 and up"); ?>
+--FILE--
+<?php
+
+class T extends \ThreadedBase{
+	public readonly int $a;
+
+	public readonly \ThreadedArray $array;
+
+	public readonly int $initiallyUninit;
+
+	public function __construct(){
+		$this->a = 1;
+		$this->array = new \ThreadedArray();
+	}
+
+	public function unsetProperties() : void{
+		try{
+			unset($this->a);
+		}catch(\Error $e){
+			echo $e->getMessage() . PHP_EOL;
+		}
+		try{
+			unset($this->array);
+		}catch(\Error $e){
+			echo $e->getMessage() . PHP_EOL;
+		}
+		try{
+			unset($this->initiallyUninit);
+		}catch(\Error $e){
+			echo $e->getMessage() . PHP_EOL;
+		}
+	}
+
+	public function writeProperties() : void{
+		try{
+			$this->a = 2;
+		}catch(\Error $e){
+			echo $e->getMessage() . PHP_EOL;
+		}
+		try{
+			$this->a++;
+		}catch(\Error $e){
+			echo $e->getMessage() . PHP_EOL;
+		}
+		try{
+			$this->array = new \ThreadedArray();
+		}catch(\Error $e){
+			echo $e->getMessage() . PHP_EOL;
+		}
+		try{
+			$this->initiallyUninit = 2;
+		}catch(\Error $e){
+			echo $e->getMessage() . PHP_EOL;
+		}
+	}
+
+	public function readProperties() : void{
+		var_dump($this->a);
+		var_dump($this->array);
+		var_dump($this->initiallyUninit);
+	}
+
+	public function issetProperties() : void{
+		var_dump(isset($this->a));
+		var_dump(isset($this->array));
+		var_dump(isset($this->initiallyUninit));
+	}
+}
+
+function test(T $t) : void{
+	//these must run first, to ensure they work on an unpopulated local cache
+	$t->unsetProperties();
+	$t->writeProperties();
+	$t->issetProperties();
+	$t->readProperties();
+}
+
+echo "--- main thread start ---\n";
+test(new T());
+echo "--- main thread end ---\n";
+
+//init properties from the main thread - ensure that the child thread receives an empty local cache
+$thread = new class(new T) extends \Thread{
+	public function __construct(
+		private T $t
+	){}
+
+	public function run() : void{
+		echo "--- child thread start ---\n";
+		test($this->t);
+		echo "--- child thread end ---\n";
+	}
+};
+$thread->start();
+$thread->join();
+echo "OK\n";
+?>
+--EXPECTF--
+--- main thread start ---
+Cannot unset readonly property T::$a
+Cannot unset readonly property T::$array
+Cannot unset readonly property T::$initiallyUninit
+Cannot modify readonly property T::$a
+Cannot modify readonly property T::$a
+Cannot modify readonly property T::$array
+bool(true)
+bool(true)
+bool(true)
+int(1)
+object(ThreadedArray)#%d (0) {
+}
+int(2)
+--- main thread end ---
+--- child thread start ---
+Cannot unset readonly property T::$a
+Cannot unset readonly property T::$array
+Cannot unset readonly property T::$initiallyUninit
+Cannot modify readonly property T::$a
+Cannot modify readonly property T::$a
+Cannot modify readonly property T::$array
+bool(true)
+bool(true)
+bool(true)
+int(1)
+object(ThreadedArray)#%d (0) {
+}
+int(2)
+--- child thread end ---
+OK