Skip to content

Commit 338e0d1

Browse files
dktappsdktapps
committed
First look at readonly property support
Before this, readonly was silently ignored, so properties would be treated the same as normal properties. This change enforces write-once and no-unset policies for readonly properties. It also permits lockless reads of locally cached property values of readonly properties. Currently this only covers stuff that is kept in the local cache, such as thread-safe objects, closures, sockets and strings. The cache could be extended to more stuff to permit this optimisation to apply to other types of properties, but that's a task for another time.
1 parent 4a2d434 commit 338e0d1

File tree

3 files changed

+129
-27
lines changed

3 files changed

+129
-27
lines changed

‎src/handlers.c‎

Lines changed: 73 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,14 @@ zval* pthreads_read_property(PTHREADS_READ_PROPERTY_PASSTHRU_D) {
9292
} else {
9393
//defined property, use mangled name
9494
ZVAL_STR(&zmember, info->name);
95+
96+
#if PHP_VERSION_ID >= 80100
97+
if ((info->flags & ZEND_ACC_READONLY) == 0 || pthreads_store_read_local_property(object, member, type, rv) == FAILURE) {
98+
pthreads_store_read(object, &zmember, type, rv);
99+
}
100+
#else
95101
pthreads_store_read(object, &zmember, type, rv);
102+
#endif
96103

97104
if (Z_ISUNDEF_P(rv)) {
98105
if (type != BP_VAR_IS && !EG(exception)) {
@@ -139,28 +146,57 @@ zval* pthreads_write_property(PTHREADS_WRITE_PROPERTY_PASSTHRU_D) {
139146
} else {
140147
bool ok = true;
141148
zend_property_info* info = zend_get_property_info(object->ce, member, 0);
149+
142150
if (info != ZEND_WRONG_PROPERTY_INFO) {
143-
if (info != NULL && (info->flags & ZEND_ACC_STATIC) == 0) {
144-
ZVAL_STR(&zmember, info->name); //use mangled name to avoid private member shadowing issues
151+
bool overwrite = true;
145152

146-
zend_execute_data* execute_data = EG(current_execute_data);
147-
bool strict = execute_data
148-
&& execute_data->func
149-
&& ZEND_CALL_USES_STRICT_TYPES(EG(current_execute_data));
153+
if (info != NULL) {
154+
if ((info->flags & ZEND_ACC_STATIC) == 0) {
155+
ZVAL_STR(&zmember, info->name); //use mangled name to avoid private member shadowing issues
150156

151-
if (ZEND_TYPE_IS_SET(info->type) && !zend_verify_property_type(info, value, strict)) {
152-
ok = false;
157+
zend_execute_data* execute_data = EG(current_execute_data);
158+
bool strict = execute_data
159+
&& execute_data->func
160+
&& ZEND_CALL_USES_STRICT_TYPES(EG(current_execute_data));
161+
162+
if (ZEND_TYPE_IS_SET(info->type) && !zend_verify_property_type(info, value, strict)) {
163+
ok = false;
164+
}
153165
}
166+
#if PHP_VERSION_ID >= 80100
167+
overwrite = (info->flags & ZEND_ACC_READONLY) == 0;
168+
#endif
154169
}
155170

156-
if (ok && pthreads_store_write(object, &zmember, value, PTHREADS_STORE_NO_COERCE_ARRAY) == FAILURE && !EG(exception)) {
157-
zend_throw_error(
158-
NULL,
159-
"Cannot assign non-thread-safe value of type %s to thread-safe class property %s::$%s",
160-
zend_zval_type_name(value),
161-
ZSTR_VAL(object->ce->name),
162-
ZSTR_VAL(member)
163-
);
171+
if (ok) {
172+
pthreads_store_write_result result = pthreads_store_write_ex(object, &zmember, value, PTHREADS_STORE_NO_COERCE_ARRAY, overwrite);
173+
if (result != WRITE_SUCCESS && !EG(exception)) {
174+
switch (result) {
175+
case WRITE_FAIL_NOT_THREAD_SAFE: {
176+
zend_throw_error(
177+
NULL,
178+
"Cannot assign non-thread-safe value of type %s to thread-safe class property %s::$%s",
179+
zend_zval_type_name(value),
180+
ZSTR_VAL(object->ce->name),
181+
ZSTR_VAL(member)
182+
);
183+
break;
184+
}
185+
case WRITE_FAIL_WOULD_OVERWRITE: {
186+
zend_throw_error(
187+
NULL,
188+
"Cannot modify readonly property %s::$%s",
189+
ZSTR_VAL(object->ce->name),
190+
ZSTR_VAL(member)
191+
);
192+
break;
193+
}
194+
default: {
195+
ZEND_ASSERT(0);
196+
break;
197+
}
198+
}
199+
}
164200
}
165201
}
166202
}
@@ -231,10 +267,28 @@ void pthreads_unset_property(PTHREADS_UNSET_PROPERTY_PASSTHRU_D) {
231267
} else {
232268
zend_property_info* info = zend_get_property_info(object->ce, member, 0);
233269
if (info != ZEND_WRONG_PROPERTY_INFO) {
234-
if (info != NULL && (info->flags & ZEND_ACC_STATIC) == 0) {
235-
ZVAL_STR(&zmember, info->name); //defined property, use mangled name
270+
zend_bool ok = true;
271+
if (info != NULL) {
272+
if ((info->flags & ZEND_ACC_STATIC) == 0) {
273+
ZVAL_STR(&zmember, info->name); //defined property, use mangled name
274+
}
275+
276+
#if PHP_VERSION_ID >= 80100
277+
if ((info->flags & ZEND_ACC_READONLY) != 0) {
278+
zend_throw_error(
279+
NULL,
280+
"Cannot unset readonly property %s::$%s",
281+
ZSTR_VAL(object->ce->name),
282+
ZSTR_VAL(member)
283+
);
284+
ok = false;
285+
}
286+
#endif
287+
}
288+
289+
if (ok) {
290+
pthreads_store_delete(object, &zmember);
236291
}
237-
pthreads_store_delete(object, &zmember);
238292
}
239293
}
240294
}

‎src/store.c‎

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,25 @@ static inline zend_bool pthreads_store_update_shared_property(pthreads_object_t*
406406
return result;
407407
}
408408

409+
/* {{{ */
410+
int pthreads_store_read_local_property(zend_object* object, zend_string* key, int type, zval* read) {
411+
zval* property;
412+
pthreads_zend_object_t* threaded = PTHREADS_FETCH_FROM(object);
413+
414+
if (threaded->std.properties) {
415+
property = zend_hash_find(threaded->std.properties, key);
416+
417+
if (property && pthreads_store_valid_local_cache_item(property)) {
418+
pthreads_monitor_unlock(&threaded->ts_obj->monitor);
419+
ZVAL_DEINDIRECT(property);
420+
ZVAL_COPY(read, property);
421+
return SUCCESS;
422+
}
423+
}
424+
425+
return FAILURE;
426+
} /* }}} */
427+
409428
/* {{{ */
410429
int pthreads_store_read(zend_object *object, zval *key, int type, zval *read) {
411430
int result = FAILURE;
@@ -492,8 +511,8 @@ static zend_string* pthreads_store_restore_string(zend_string* string) {
492511
}
493512

494513
/* {{{ */
495-
int pthreads_store_write(zend_object *object, zval *key, zval *write, zend_bool coerce_array_to_threaded) {
496-
int result = FAILURE;
514+
pthreads_store_write_result pthreads_store_write_ex(zend_object *object, zval *key, zval *write, zend_bool coerce_array_to_threaded, zend_bool overwrite) {
515+
pthreads_store_write_result result = WRITE_FAIL_UNKNOWN;
497516
zval vol, member, zstorage;
498517
pthreads_zend_object_t *threaded =
499518
PTHREADS_FETCH_FROM(object);
@@ -511,7 +530,7 @@ int pthreads_store_write(zend_object *object, zval *key, zval *write, zend_bool
511530
}
512531

513532
if (pthreads_store_save_zval(&threaded->owner, &zstorage, write) != SUCCESS) {
514-
return FAILURE;
533+
return WRITE_FAIL_NOT_THREAD_SAFE;
515534
}
516535

517536
if (pthreads_monitor_lock(&ts_obj->monitor)) {
@@ -523,10 +542,26 @@ int pthreads_store_write(zend_object *object, zval *key, zval *write, zend_bool
523542
coerced = pthreads_store_coerce(key, &member);
524543
}
525544

526-
zend_bool was_pthreads_object = pthreads_store_member_is_cacheable(object, &member);
527-
result = pthreads_store_update_shared_property(ts_obj, &member, &zstorage);
528-
if (result == SUCCESS && was_pthreads_object) {
529-
_pthreads_store_bump_modcount_nolock(threaded);
545+
zend_bool ok = true;
546+
if (!overwrite) {
547+
if (Z_TYPE(member) == IS_LONG) {
548+
ok = !zend_hash_index_exists(&ts_obj->props.hash, Z_LVAL(member));
549+
} else {
550+
ok = !zend_hash_exists(&ts_obj->props.hash, Z_STR(member));
551+
}
552+
if (!ok) {
553+
result = WRITE_FAIL_WOULD_OVERWRITE;
554+
}
555+
}
556+
557+
if (ok) {
558+
zend_bool was_pthreads_object = pthreads_store_member_is_cacheable(object, &member);
559+
if (pthreads_store_update_shared_property(ts_obj, &member, &zstorage) == SUCCESS) {
560+
result = WRITE_SUCCESS;
561+
if (was_pthreads_object) {
562+
_pthreads_store_bump_modcount_nolock(threaded);
563+
}
564+
}
530565
}
531566
//this isn't necessary for any specific property write, but since we don't have any other way to clean up local
532567
//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
535570
pthreads_monitor_unlock(&ts_obj->monitor);
536571
}
537572

538-
if (result != SUCCESS) {
573+
if (result != WRITE_SUCCESS) {
539574
pthreads_store_storage_dtor(&zstorage);
540575
} else {
541576
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
547582
return result;
548583
} /* }}} */
549584

585+
int pthreads_store_write(zend_object* object, zval* key, zval* write, zend_bool coerce_array_to_threaded) {
586+
return pthreads_store_write_ex(object, key, write, coerce_array_to_threaded, true) == WRITE_SUCCESS ? SUCCESS : FAILURE;
587+
}
588+
550589
/* {{{ */
551590
int pthreads_store_count(zend_object *object, zend_long *count) {
552591
pthreads_object_t* ts_obj = PTHREADS_FETCH_TS_FROM(object);

‎src/store.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,24 @@ typedef struct _pthreads_store_t {
3434
zend_long modcount;
3535
} pthreads_store_t;
3636

37+
typedef enum {
38+
WRITE_SUCCESS = 0,
39+
WRITE_FAIL_UNKNOWN = -1,
40+
WRITE_FAIL_NOT_THREAD_SAFE = -2,
41+
WRITE_FAIL_WOULD_OVERWRITE = -3
42+
} pthreads_store_write_result;
43+
3744
void pthreads_store_init(pthreads_store_t* store);
3845
void pthreads_store_destroy(pthreads_store_t* store);
3946
void pthreads_store_sync_local_properties(zend_object* object);
4047
void pthreads_store_full_sync_local_properties(zend_object *object);
4148
int pthreads_store_merge(zend_object *destination, zval *from, zend_bool overwrite, zend_bool coerce_array_to_threaded);
4249
int pthreads_store_delete(zend_object *object, zval *key);
4350
int pthreads_store_read(zend_object *object, zval *key, int type, zval *read);
51+
int pthreads_store_read_local_property(zend_object* object, zend_string* key, int type, zval* read);
4452
zend_bool pthreads_store_isset(zend_object *object, zval *key, int has_set_exists);
4553
int pthreads_store_write(zend_object *object, zval *key, zval *write, zend_bool coerce_array_to_threaded);
54+
pthreads_store_write_result pthreads_store_write_ex(zend_object *object, zval *key, zval *write, zend_bool coerce_array_to_threaded, zend_bool overwrite);
4655
void pthreads_store_tohash(zend_object *object, HashTable *hash);
4756
int pthreads_store_shift(zend_object *object, zval *member);
4857
int pthreads_store_chunk(zend_object *object, zend_long size, zend_bool preserve, zval *chunk);

0 commit comments

Comments
 (0)