Skip to content

Commit 234af00

Browse files
committed
Destroy constant values before object store
Now that constants can contain objects (currently only enums), we should destroy them before we free the object store, otherwise there will be false positive leak reports. This doesn't affect the fast_shutdown sequence.
1 parent 7e9f6d2 commit 234af00

File tree

2 files changed

+30
-19
lines changed

2 files changed

+30
-19
lines changed

Zend/zend_execute_API.c

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,24 @@ void shutdown_executor(void) /* {{{ */
278278
if (!fast_shutdown) {
279279
zend_hash_graceful_reverse_destroy(&EG(symbol_table));
280280

281+
/* Constants may contain objects, destroy them before the object store. */
282+
if (EG(full_tables_cleanup)) {
283+
zend_hash_reverse_apply(EG(zend_constants), clean_non_persistent_constant_full);
284+
} else {
285+
ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(EG(zend_constants), key, zv) {
286+
zend_constant *c = Z_PTR_P(zv);
287+
if (_idx == EG(persistent_constants_count)) {
288+
break;
289+
}
290+
zval_ptr_dtor_nogc(&c->value);
291+
if (c->name) {
292+
zend_string_release_ex(c->name, 0);
293+
}
294+
efree(c);
295+
zend_string_release_ex(key, 0);
296+
} ZEND_HASH_FOREACH_END_DEL();
297+
}
298+
281299
/* Release static properties and static variables prior to the final GC run,
282300
* as they may hold GC roots. */
283301
ZEND_HASH_REVERSE_FOREACH_VAL(EG(function_table), zv) {
@@ -302,6 +320,15 @@ void shutdown_executor(void) /* {{{ */
302320

303321
if (ZEND_MAP_PTR(ce->mutable_data) && ZEND_MAP_PTR_GET_IMM(ce->mutable_data)) {
304322
zend_cleanup_mutable_class_data(ce);
323+
} else if (ce->type == ZEND_USER_CLASS && !(ce->ce_flags & ZEND_ACC_IMMUTABLE)) {
324+
/* Constants may contain objects, destroy the values before the object store. */
325+
zend_class_constant *c;
326+
ZEND_HASH_FOREACH_PTR(&ce->constants_table, c) {
327+
if (c->ce == ce) {
328+
zval_ptr_dtor_nogc(&c->value);
329+
ZVAL_UNDEF(&c->value);
330+
}
331+
} ZEND_HASH_FOREACH_END();
305332
}
306333

307334
if (ce->ce_flags & ZEND_HAS_STATIC_IN_METHODS) {
@@ -340,6 +367,8 @@ void shutdown_executor(void) /* {{{ */
340367
gc_collect_cycles();
341368
}
342369
#endif
370+
} else {
371+
zend_hash_discard(EG(zend_constants), EG(persistent_constants_count));
343372
}
344373

345374
zend_objects_store_free_object_storage(&EG(objects_store), fast_shutdown);
@@ -356,31 +385,16 @@ void shutdown_executor(void) /* {{{ */
356385
* Zend Memory Manager frees memory by its own. We don't have to free
357386
* each allocated block separately.
358387
*/
359-
zend_hash_discard(EG(zend_constants), EG(persistent_constants_count));
360388
zend_hash_discard(EG(function_table), EG(persistent_functions_count));
361389
zend_hash_discard(EG(class_table), EG(persistent_classes_count));
362390
zend_cleanup_internal_classes();
363391
} else {
364392
zend_vm_stack_destroy();
365393

366394
if (EG(full_tables_cleanup)) {
367-
zend_hash_reverse_apply(EG(zend_constants), clean_non_persistent_constant_full);
368395
zend_hash_reverse_apply(EG(function_table), clean_non_persistent_function_full);
369396
zend_hash_reverse_apply(EG(class_table), clean_non_persistent_class_full);
370397
} else {
371-
ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(EG(zend_constants), key, zv) {
372-
zend_constant *c = Z_PTR_P(zv);
373-
if (_idx == EG(persistent_constants_count)) {
374-
break;
375-
}
376-
zval_ptr_dtor_nogc(&c->value);
377-
if (c->name) {
378-
zend_string_release_ex(c->name, 0);
379-
}
380-
efree(c);
381-
zend_string_release_ex(key, 0);
382-
} ZEND_HASH_FOREACH_END_DEL();
383-
384398
ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(EG(function_table), key, zv) {
385399
zend_function *func = Z_PTR_P(zv);
386400
if (_idx == EG(persistent_functions_count)) {

Zend/zend_objects_API.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,7 @@ ZEND_API void ZEND_FASTCALL zend_objects_store_free_object_storage(zend_objects_
113113
if (IS_OBJ_VALID(obj)) {
114114
if (!(OBJ_FLAGS(obj) & IS_OBJ_FREE_CALLED)) {
115115
GC_ADD_FLAGS(obj, IS_OBJ_FREE_CALLED);
116-
// FIXME: This causes constant objects to leak
117-
if (!(obj->ce->ce_flags & ZEND_ACC_ENUM)) {
118-
GC_ADDREF(obj);
119-
}
116+
GC_ADDREF(obj);
120117
obj->handlers->free_obj(obj);
121118
}
122119
}

0 commit comments

Comments
 (0)