Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit d74901a

Browse files
arnaud-lbTimWolla
andcommitted
Differenciate WeakMaps from bare HashTables used as weak maps for GC purposes
Since cbf67e4, the GC needs to find all WeakMaps referencing a weakly referenced object. Doing so, it treats all ZEND_WEAKREF_TAG_MAP as WeakMap instances. However, a ZEND_WEAKREF_TAG_MAP reference may be a bare HashTable when zend_weakrefs_hash_add() is used. Introduce a new tag, ZEND_WEAKREF_TAG_BARE_HT, and use this tag when weakly referencing an object from a bare HashTable. Ignore such references in GC. Fixes GH-19543 Closes GH-19544 Co-authored-by: Tim Düsterhus <tim@tideways-gmbh.com>
1 parent 0a12aaa commit d74901a

File tree

5 files changed

+72
-18
lines changed

5 files changed

+72
-18
lines changed

‎Zend/tests/gh19543-001.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
GH-19543 001: GC treats ZEND_WEAKREF_TAG_MAP references as WeakMap references
3+
--EXTENSIONS--
4+
zend_test
5+
--FILE--
6+
<?php
7+
8+
$e = new Exception();
9+
$a = new stdClass();
10+
zend_weakmap_attach($e, $a);
11+
unset($a);
12+
gc_collect_cycles();
13+
14+
?>
15+
==DONE==
16+
--EXPECT--
17+
==DONE==

‎Zend/tests/gh19543-002.phpt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
GH-19543 002: GC treats ZEND_WEAKREF_TAG_MAP references as WeakMap references
3+
--EXTENSIONS--
4+
zend_test
5+
--FILE--
6+
<?php
7+
8+
$e = new Exception();
9+
$a = new stdClass();
10+
zend_weakmap_attach($e, $a);
11+
unset($a);
12+
$e2 = $e;
13+
unset($e2); // add to roots
14+
gc_collect_cycles();
15+
16+
?>
17+
==DONE==
18+
--EXPECT--
19+
==DONE==

‎Zend/zend_weakrefs.c‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,21 @@ typedef struct _zend_weakmap_iterator {
3636
uint32_t ht_iter;
3737
} zend_weakmap_iterator;
3838

39-
/* EG(weakrefs) is a map from a key corresponding to a zend_object pointer to all the WeakReferenceand/or WeakMap entries relating to that pointer.
39+
/* EG(weakrefs) is a map from a key corresponding to a zend_object pointer to all the WeakReference, WeakMap, and/or bare HashTable entries relating to that pointer.
4040
*
4141
* 1. For a single WeakReference,
4242
* the HashTable's corresponding value's tag is a ZEND_WEAKREF_TAG_REF and the pointer is a singleton WeakReference instance (zend_weakref *) for that zend_object pointer (from WeakReference::create()).
4343
* 2. For a single WeakMap, the HashTable's corresponding value's tag is a ZEND_WEAKREF_TAG_MAP and the pointer is a WeakMap instance (zend_weakmap *).
44-
* 3. For multiple values associated with the same zend_object pointer, the HashTable entry's tag is a ZEND_WEAKREF_TAG_HT with a HashTable mapping
45-
* tagged pointers of at most 1 WeakReference and 1 or more WeakMaps to the same tagged pointer.
44+
* 3. For a single bare HashTable, the HashTable's corresponding value's tag is a ZEND_WEAKREF_TAG_BARE_HT and the pointer is a HashTable*.
45+
* 4. For multiple values associated with the same zend_object pointer, the HashTable entry's tag is a ZEND_WEAKREF_TAG_HT with a HashTable mapping
46+
* tagged pointers of at most 1 WeakReference and 1 or more WeakMap or bare HashTable to the same tagged pointer.
4647
*
4748
* ZEND_MM_ALIGNED_OFFSET_LOG2 is at least 2 on supported architectures (pointers to the objects in question are aligned to 4 bytes (1<<2) even on 32-bit systems),
4849
* i.e. the least two significant bits of the pointer can be used as a tag (ZEND_WEAKREF_TAG_*). */
49-
#define ZEND_WEAKREF_TAG_REF 0
50-
#define ZEND_WEAKREF_TAG_MAP 1
51-
#define ZEND_WEAKREF_TAG_HT 2
50+
#define ZEND_WEAKREF_TAG_REF 0
51+
#define ZEND_WEAKREF_TAG_MAP 1
52+
#define ZEND_WEAKREF_TAG_HT 2
53+
#define ZEND_WEAKREF_TAG_BARE_HT 3
5254
#define ZEND_WEAKREF_GET_TAG(p) (((uintptr_t) (p)) & 3)
5355
#define ZEND_WEAKREF_GET_PTR(p) ((void *) (((uintptr_t) (p)) & ~3))
5456
#define ZEND_WEAKREF_ENCODE(p, t) ((void *) (((uintptr_t) (p)) | (t)))
@@ -72,8 +74,8 @@ static inline void zend_weakref_unref_single(
7274
zend_weakref *wr = ptr;
7375
wr->referent = NULL;
7476
} else {
75-
/* unreferencing WeakMap entry (at ptr) with a key of object. */
76-
ZEND_ASSERT(tag == ZEND_WEAKREF_TAG_MAP);
77+
/* unreferencing WeakMap or bare HashTable entry (at ptr) with a key of object. */
78+
ZEND_ASSERT(tag == ZEND_WEAKREF_TAG_MAP||tag==ZEND_WEAKREF_TAG_BARE_HT);
7779
zend_hash_index_del((HashTable *) ptr, zend_object_to_weakref_key(object));
7880
}
7981
}
@@ -166,18 +168,20 @@ static void zend_weakref_unregister(zend_object *object, void *payload, bool wea
166168
}
167169
}
168170

171+
/* Insert 'pData' into bare HashTable 'ht', with the given 'key'. 'key' is
172+
* weakly referenced. 'ht' is considered to be a bare HashTable, not a WeakMap. */
169173
ZEND_API zval *zend_weakrefs_hash_add(HashTable *ht, zend_object *key, zval *pData) {
170174
zval *zv = zend_hash_index_add(ht, zend_object_to_weakref_key(key), pData);
171175
if (zv) {
172-
zend_weakref_register(key, ZEND_WEAKREF_ENCODE(ht, ZEND_WEAKREF_TAG_MAP));
176+
zend_weakref_register(key, ZEND_WEAKREF_ENCODE(ht, ZEND_WEAKREF_TAG_BARE_HT));
173177
}
174178
return zv;
175179
}
176180

177181
ZEND_API zend_result zend_weakrefs_hash_del(HashTable *ht, zend_object *key) {
178182
zval *zv = zend_hash_index_find(ht, zend_object_to_weakref_key(key));
179183
if (zv) {
180-
zend_weakref_unregister(key, ZEND_WEAKREF_ENCODE(ht, ZEND_WEAKREF_TAG_MAP), 1);
184+
zend_weakref_unregister(key, ZEND_WEAKREF_ENCODE(ht, ZEND_WEAKREF_TAG_BARE_HT), 1);
181185
return SUCCESS;
182186
}
183187
return FAILURE;
@@ -520,6 +524,10 @@ HashTable *zend_weakmap_get_object_key_entry_gc(zend_object *object, zval **tabl
520524
ZEND_ASSERT(zv);
521525
zend_get_gc_buffer_add_ptr(gc_buffer, zv);
522526
zend_get_gc_buffer_add_obj(gc_buffer, &wm->std);
527+
} else if (ZEND_WEAKREF_GET_TAG(tagged_ptr) == ZEND_WEAKREF_TAG_BARE_HT) {
528+
/* Bare HashTables are intentionally ignored, since they are
529+
* intended for internal usage by extensions and might not be
530+
* collectable. */
523531
}
524532
} ZEND_HASH_FOREACH_END();
525533
} else if (tag == ZEND_WEAKREF_TAG_MAP) {
@@ -528,6 +536,8 @@ HashTable *zend_weakmap_get_object_key_entry_gc(zend_object *object, zval **tabl
528536
ZEND_ASSERT(zv);
529537
zend_get_gc_buffer_add_ptr(gc_buffer, zv);
530538
zend_get_gc_buffer_add_obj(gc_buffer, &wm->std);
539+
} else if (tag == ZEND_WEAKREF_TAG_BARE_HT) {
540+
/* Bare HashTables are intentionally ignored (see above) */
531541
}
532542

533543
zend_get_gc_buffer_use(gc_buffer, table, n);
@@ -554,13 +564,19 @@ HashTable *zend_weakmap_get_object_entry_gc(zend_object *object, zval **table, i
554564
zval *zv = zend_hash_index_find(&wm->ht, obj_key);
555565
ZEND_ASSERT(zv);
556566
zend_get_gc_buffer_add_ptr(gc_buffer, zv);
567+
} else if (ZEND_WEAKREF_GET_TAG(tagged_ptr) == ZEND_WEAKREF_TAG_BARE_HT) {
568+
/* Bare HashTables are intentionally ignored
569+
* (see zend_weakmap_get_object_key_entry_gc) */
557570
}
558571
} ZEND_HASH_FOREACH_END();
559572
} else if (tag == ZEND_WEAKREF_TAG_MAP) {
560573
zend_weakmap *wm = (zend_weakmap*) ptr;
561574
zval *zv = zend_hash_index_find(&wm->ht, obj_key);
562575
ZEND_ASSERT(zv);
563576
zend_get_gc_buffer_add_ptr(gc_buffer, zv);
577+
} else if (tag == ZEND_WEAKREF_TAG_BARE_HT) {
578+
/* Bare HashTables are intentionally ignored
579+
* (see zend_weakmap_get_object_key_entry_gc) */
564580
}
565581

566582
zend_get_gc_buffer_use(gc_buffer, table, n);

‎ext/zend_test/php_test.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test)
5050
int observer_fiber_switch;
5151
int observer_fiber_destroy;
5252
int observer_execute_internal;
53-
HashTable global_weakmap;
53+
HashTable *global_weakmap;
5454
int replace_zend_execute_ex;
5555
int register_passes;
5656
bool print_stderr_mshutdown;

‎ext/zend_test/test.c‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ static ZEND_FUNCTION(zend_weakmap_attach)
340340
Z_PARAM_ZVAL(value)
341341
ZEND_PARSE_PARAMETERS_END();
342342

343-
if (zend_weakrefs_hash_add(&ZT_G(global_weakmap), obj, value)) {
343+
if (zend_weakrefs_hash_add(ZT_G(global_weakmap), obj, value)) {
344344
Z_TRY_ADDREF_P(value);
345345
RETURN_TRUE;
346346
}
@@ -355,13 +355,13 @@ static ZEND_FUNCTION(zend_weakmap_remove)
355355
Z_PARAM_OBJ(obj)
356356
ZEND_PARSE_PARAMETERS_END();
357357

358-
RETURN_BOOL(zend_weakrefs_hash_del(&ZT_G(global_weakmap), obj) == SUCCESS);
358+
RETURN_BOOL(zend_weakrefs_hash_del(ZT_G(global_weakmap), obj) == SUCCESS);
359359
}
360360

361361
static ZEND_FUNCTION(zend_weakmap_dump)
362362
{
363363
ZEND_PARSE_PARAMETERS_NONE();
364-
RETURN_ARR(zend_array_dup(&ZT_G(global_weakmap)));
364+
RETURN_ARR(zend_array_dup(ZT_G(global_weakmap)));
365365
}
366366

367367
static ZEND_FUNCTION(zend_get_current_func_name)
@@ -1311,18 +1311,20 @@ PHP_MSHUTDOWN_FUNCTION(zend_test)
13111311

13121312
PHP_RINIT_FUNCTION(zend_test)
13131313
{
1314-
zend_hash_init(&ZT_G(global_weakmap), 8, NULL, ZVAL_PTR_DTOR, 0);
1314+
ALLOC_HASHTABLE(ZT_G(global_weakmap));
1315+
zend_hash_init(ZT_G(global_weakmap), 8, NULL, ZVAL_PTR_DTOR, 0);
13151316
ZT_G(observer_nesting_depth) = 0;
13161317
return SUCCESS;
13171318
}
13181319

13191320
PHP_RSHUTDOWN_FUNCTION(zend_test)
13201321
{
13211322
zend_ulong obj_key;
1322-
ZEND_HASH_FOREACH_NUM_KEY(&ZT_G(global_weakmap), obj_key) {
1323-
zend_weakrefs_hash_del(&ZT_G(global_weakmap), zend_weakref_key_to_object(obj_key));
1323+
ZEND_HASH_FOREACH_NUM_KEY(ZT_G(global_weakmap), obj_key) {
1324+
zend_weakrefs_hash_del(ZT_G(global_weakmap), zend_weakref_key_to_object(obj_key));
13241325
} ZEND_HASH_FOREACH_END();
1325-
zend_hash_destroy(&ZT_G(global_weakmap));
1326+
zend_hash_destroy(ZT_G(global_weakmap));
1327+
FREE_HASHTABLE(ZT_G(global_weakmap));
13261328

13271329
if (ZT_G(zend_test_heap)) {
13281330
free(ZT_G(zend_test_heap));

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /