Skip to content

Commit

Permalink
Added GC checks and improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
@dstogov
dstogov committed Apr 17, 2015
1 parent 10d4fdb commit 276080e
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 34 deletions.
61 changes: 30 additions & 31 deletions Zend/zend_gc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,7 @@ ZEND_API void gc_init(void)

ZEND_API void ZEND_FASTCALL gc_possible_root(zend_refcounted *ref)
{
if (UNEXPECTED(GC_TYPE(ref) == IS_NULL) ||
UNEXPECTED(CG(unclean_shutdown)) ||
UNEXPECTED(GC_G(gc_active))) {
if (UNEXPECTED(CG(unclean_shutdown)) || UNEXPECTED(GC_G(gc_active))) {
return;
}

Expand Down Expand Up @@ -242,14 +240,14 @@ ZEND_API void ZEND_FASTCALL gc_possible_root(zend_refcounted *ref)
GC_G(unused) = newRoot->prev;
}

GC_REF_SET_PURPLE(ref);
GC_TRACE_SET_COLOR(ref, GC_PURPLE);
GC_INFO(ref) = (newRoot - GC_G(buf)) | GC_PURPLE;

newRoot->next = GC_G(roots).next;
newRoot->prev = &GC_G(roots);
GC_G(roots).next->prev = newRoot;
GC_G(roots).next = newRoot;

GC_REF_SET_ADDRESS(ref, newRoot - GC_G(buf));

newRoot->ref = ref;

GC_BENCH_INC(zval_buffered);
Expand Down Expand Up @@ -283,7 +281,7 @@ static void gc_scan_black(zend_refcounted *ref)
ht = NULL;
GC_REF_SET_BLACK(ref);

if (GC_TYPE(ref) == IS_OBJECT) {
if (GC_TYPE(ref) == IS_OBJECT && !(GC_FLAGS(ref) & IS_OBJ_FREE_CALLED)) {
zend_object_get_gc_t get_gc;
zend_object *obj = (zend_object*)ref;

Expand Down Expand Up @@ -376,7 +374,7 @@ static void gc_mark_grey(zend_refcounted *ref)
GC_BENCH_INC(zval_marked_grey);
GC_REF_SET_COLOR(ref, GC_GREY);

if (GC_TYPE(ref) == IS_OBJECT) {
if (GC_TYPE(ref) == IS_OBJECT && !(GC_FLAGS(ref) & IS_OBJ_FREE_CALLED)) {
zend_object_get_gc_t get_gc;
zend_object *obj = (zend_object*)ref;

Expand Down Expand Up @@ -487,7 +485,7 @@ static void gc_scan(zend_refcounted *ref)
gc_scan_black(ref);
} else {
GC_REF_SET_COLOR(ref, GC_WHITE);
if (GC_TYPE(ref) == IS_OBJECT) {
if (GC_TYPE(ref) == IS_OBJECT && !(GC_FLAGS(ref) & IS_OBJ_FREE_CALLED)) {
zend_object_get_gc_t get_gc;
zend_object *obj = (zend_object*)ref;

Expand Down Expand Up @@ -576,13 +574,16 @@ static int gc_collect_white(zend_refcounted *ref, uint32_t *flags)
if (GC_REF_GET_COLOR(ref) == GC_WHITE) {
ht = NULL;
GC_REF_SET_BLACK(ref);
if (!GC_ADDRESS(GC_INFO(ref))) {
GC_FLAGS(ref) |= IS_GC_INNER_GARBAGE;
}

/* don't count references for compatibility ??? */
if (GC_TYPE(ref) != IS_REFERENCE) {
count++;
}

if (GC_TYPE(ref) == IS_OBJECT) {
if (GC_TYPE(ref) == IS_OBJECT && !(GC_FLAGS(ref) & IS_OBJ_FREE_CALLED)) {
zend_object_get_gc_t get_gc;
zend_object *obj = (zend_object*)ref;

Expand All @@ -608,20 +609,17 @@ static int gc_collect_white(zend_refcounted *ref, uint32_t *flags)
*flags |= GC_HAS_INNER_CYCLES;
}
if (buf) {
GC_REFCOUNT(ref)++;
GC_FLAGS(ref) &= ~IS_GC_INNER_GARBAGE;
buf->ref = ref;
buf->next = GC_G(roots).next;
buf->prev = &GC_G(roots);
GC_G(roots).next->prev = buf;
GC_G(roots).next = buf;
GC_REF_SET_ADDRESS(ref, buf - GC_G(buf));
*flags |= GC_HAS_DESTRUCTORS;
}
} else {
*flags |= GC_HAS_DESTRUCTORS;
}
} else if (!GC_ADDRESS(GC_INFO(ref))) {
GC_FLAGS(ref) |= IS_GC_INNER_CYCLE;
*flags |= GC_HAS_INNER_CYCLES;
*flags |= GC_HAS_DESTRUCTORS;
}
ZVAL_OBJ(&tmp, obj);
ht = get_gc(&tmp, &zv, &n);
Expand Down Expand Up @@ -657,20 +655,17 @@ static int gc_collect_white(zend_refcounted *ref, uint32_t *flags)
}
} else if (GC_TYPE(ref) == IS_ARRAY) {
if (!GC_ADDRESS(GC_INFO(ref))) {
GC_FLAGS(ref) |= IS_GC_INNER_CYCLE;
*flags |= GC_HAS_INNER_CYCLES;
}
ht = (zend_array*)ref;
} else if (GC_TYPE(ref) == IS_REFERENCE) {
GC_FLAGS(ref) |= IS_GC_INNER_CYCLE;
if (Z_REFCOUNTED(((zend_reference*)ref)->val)) {
ref = Z_COUNTED(((zend_reference*)ref)->val);
GC_REFCOUNT(ref)++;
goto tail_call;
}
return count;
} else {
GC_FLAGS(ref) |= IS_GC_INNER_CYCLE;
return count;
}

Expand Down Expand Up @@ -719,6 +714,7 @@ static int gc_collect_roots(uint32_t *flags)

current = GC_G(roots).next;
while (current != &GC_G(roots)) {
GC_REFCOUNT(current->ref)++;
if (GC_REF_GET_COLOR(current->ref) == GC_WHITE) {
count += gc_collect_white(current->ref, flags);
}
Expand Down Expand Up @@ -753,14 +749,14 @@ static void gc_break_cycles(zend_refcounted *ref, zval *zv)
Bucket *p, *end;

tail_call:
if (zv && !(GC_FLAGS(ref) & IS_GC_INNER_CYCLE)) {
if (zv && !(GC_FLAGS(ref) & IS_GC_INNER_GARBAGE)) {
GC_REFCOUNT(ref)--;
ZVAL_NULL(zv);
return;
}
GC_FLAGS(ref) &= ~IS_GC_INNER_CYCLE;
GC_FLAGS(ref) &= ~IS_GC_INNER_GARBAGE;

if (GC_TYPE(ref) == IS_OBJECT) {
if (GC_TYPE(ref) == IS_OBJECT && !(GC_FLAGS(ref) & IS_OBJ_FREE_CALLED)) {
zend_object_get_gc_t get_gc;
zend_object *obj = (zend_object*)ref;

Expand Down Expand Up @@ -833,9 +829,10 @@ static void gc_remove_nested_data_from_buffer(zend_refcounted *ref)
tail_call:
if (GC_ADDRESS(GC_INFO(ref)) != 0 && GC_REF_GET_COLOR(ref) == GC_BLACK) {
GC_TRACE_REF(ref, "removing from buffer");
GC_REFCOUNT(ref)--;
GC_REMOVE_FROM_BUFFER(ref);

if (GC_TYPE(ref) == IS_OBJECT) {
if (GC_TYPE(ref) == IS_OBJECT && !(GC_FLAGS(ref) & IS_OBJ_FREE_CALLED)) {
zend_object_get_gc_t get_gc;
zend_object *obj = (zend_object*)ref;

Expand Down Expand Up @@ -960,16 +957,16 @@ ZEND_API int zend_gc_collect_cycles(void)
#endif

if (gc_flags & GC_HAS_DESTRUCTORS) {
/* Remember reference counters before calling destructors */
current = to_free.next;
while (current != &to_free) {
current->refcount = GC_REFCOUNT(current->ref);
current = current->next;
}

/* Call destructors */
GC_TRACE("Calling destructors");
if (EG(objects_store).object_buckets) {
/* Remember reference counters before calling destructors */
current = to_free.next;
while (current != &to_free) {
current->refcount = GC_REFCOUNT(current->ref);
current = current->next;
}

/* Call destructors */
current = to_free.next;
while (current != &to_free) {
p = current->ref;
Expand Down Expand Up @@ -1026,6 +1023,7 @@ ZEND_API int zend_gc_collect_cycles(void)
if (EG(objects_store).object_buckets &&
IS_OBJ_VALID(EG(objects_store).object_buckets[obj->handle])) {
GC_TYPE(obj) = IS_NULL;
GC_INFO_SET_COLOR(GC_INFO(obj), GC_WHITE);
if (!(GC_FLAGS(obj) & IS_OBJ_FREE_CALLED)) {
GC_FLAGS(obj) |= IS_OBJ_FREE_CALLED;
if (obj->handlers->free_obj) {
Expand All @@ -1042,6 +1040,7 @@ ZEND_API int zend_gc_collect_cycles(void)
zend_array *arr = (zend_array*)p;

GC_TYPE(arr) = IS_NULL;
GC_INFO_SET_COLOR(GC_INFO(arr), GC_WHITE);
zend_hash_destroy(arr);
}
current = GC_G(next_to_free);
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ static zend_always_inline zend_uchar zval_get_type(const zval* pz) {
#define IS_OBJ_HAS_GUARDS (1<<6)

/* GC flags (common for all referenced) */
#define IS_GC_INNER_CYCLE (1<<7)
#define IS_GC_INNER_GARBAGE (1<<7)

#define Z_OBJ_APPLY_COUNT(zval) \
(Z_GC_FLAGS(zval) & IS_OBJ_APPLY_COUNT)
Expand Down
4 changes: 2 additions & 2 deletions Zend/zend_variables.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ ZEND_API void ZEND_FASTCALL _zval_dtor_func(zend_refcounted *p ZEND_FILE_LINE_DC
ZEND_ASSERT(GC_REFCOUNT(arr) <= 1);

/* break possible cycles */
GC_TYPE(arr) = IS_NULL;
GC_REMOVE_FROM_BUFFER(arr);
GC_TYPE_INFO(arr) = IS_NULL | (GC_WHITE << 16);
zend_array_destroy(arr);
break;
}
Expand Down Expand Up @@ -98,8 +98,8 @@ ZEND_API void ZEND_FASTCALL _zval_dtor_func_for_ptr(zend_refcounted *p ZEND_FILE
zend_array *arr = (zend_array*)p;

/* break possible cycles */
GC_TYPE(arr) = IS_NULL;
GC_REMOVE_FROM_BUFFER(arr);
GC_TYPE_INFO(arr) = IS_NULL | (GC_WHITE << 16);
zend_array_destroy(arr);
break;
}
Expand Down

3 comments on commit 276080e

@nikic
Copy link
Member

@nikic nikic commented on 276080e Apr 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes an assertion failure in Zend/tests/bug63635.phpt

php: /home/nikic/php-src/Zend/zend_gc.c:211: gc_possible_root: Assertion `((zend_refcounted*)(ref))->u.v.type == 7 || ((zend_refcounted*)(ref))->u.v.type == 8' failed.

#3  0x00007ffff4f77c32 in __GI___assert_fail (
    assertion=0xfdc360 "((zend_refcounted*)(ref))->u.v.type == 7 || ((zend_refcounted*)(ref))->u.v.type == 8", 
    file=0xfdc338 "/home/nikic/php-src/Zend/zend_gc.c", line=211, 
    function=0xfdc3c0 <__PRETTY_FUNCTION__.8993> "gc_possible_root")
    at assert.c:101
#4  0x0000000000a297a4 in gc_possible_root (ref=0x7fffedf1ef50)
    at /home/nikic/php-src/Zend/zend_gc.c:211
#5  0x0000000000a0a089 in gc_check_possible_root (z=0x7fffedba7d08)
    at /home/nikic/php-src/Zend/zend_gc.h:136
#6  0x0000000000a0a0e7 in i_zval_ptr_dtor (zval_ptr=0x7fffedba7d08, 
    __zend_filename=0xfda108 "/home/nikic/php-src/Zend/zend_hash.c", 
    __zend_lineno=1179) at /home/nikic/php-src/Zend/zend_variables.h:59
#7  0x0000000000a0db2c in zend_array_destroy (ht=0x7fffedbc8420)
    at /home/nikic/php-src/Zend/zend_hash.c:1179
#8  0x00000000009f54d8 in _zval_dtor_func_for_ptr (p=0x7fffedbc8420, 
    __zend_filename=0xfdef80 "/home/nikic/php-src/Zend/zend_execute.h", 
    __zend_lineno=102) at /home/nikic/php-src/Zend/zend_variables.c:103
#9  0x0000000000a49a4e in zend_assign_to_variable (
    variable_ptr=0x7fffedbc9188, value=0x7fffeee781a0, value_type=1 '\001')
    at /home/nikic/php-src/Zend/zend_execute.h:102
#10 0x0000000000a4ca61 in zend_assign_to_object (retval=0x0, 
    object=0x7fffeee14050, object_op_type=8, property_name=0x7fffeee78190, 
    property_op_type=1, value_type=1, value_op=..., 
    execute_data=0x7fffeee14030, cache_slot=0x7fffeee03410)
    at /home/nikic/php-src/Zend/zend_execute.c:1032
#11 0x0000000000a86f25 in ZEND_ASSIGN_OBJ_SPEC_UNUSED_CONST_HANDLER ()
    at /home/nikic/php-src/Zend/zend_vm_execute.h:22998
#12 0x0000000000a4ffd9 in execute_ex (ex=0x7fffeee14030)
    at /home/nikic/php-src/Zend/zend_vm_execute.h:394
#13 0x00000000009e17e5 in zend_call_function (fci=0x7fffffffc3f0, 
    fci_cache=0x7fffffffc3c0)
    at /home/nikic/php-src/Zend/zend_execute_API.c:840
#14 0x0000000000a1e7a9 in zend_call_method (object=0x7fffffffc4a0, 
    obj_ce=0x7fffeee03018, fn_proxy=0x7fffffffc478, 
    function_name=0xfdd9ae "__destruct", function_name_len=10, retval_ptr=0x0, 
    param_count=0, arg1=0x0, arg2=0x0)
    at /home/nikic/php-src/Zend/zend_interfaces.c:101
#15 0x0000000000a3b9e3 in zend_objects_destroy_object (object=0x7fffedbc9150)
    at /home/nikic/php-src/Zend/zend_objects.c:126
#16 0x0000000000a41523 in zend_objects_store_call_destructors (
    objects=0x1366050 <executor_globals+816>)
    at /home/nikic/php-src/Zend/zend_objects_API.c:54
#17 0x00000000009df5b6 in shutdown_destructors ()
    at /home/nikic/php-src/Zend/zend_execute_API.c:226
#18 0x00000000009f7b7b in zend_call_destructors ()
    at /home/nikic/php-src/Zend/zend.c:949
#19 0x0000000000966b50 in php_request_shutdown (dummy=0x0)
    at /home/nikic/php-src/main/main.c:1750
#20 0x0000000000ab9249 in do_cli (argc=2, argv=0x136b7e0)
    at /home/nikic/php-src/sapi/cli/php_cli.c:1135
#21 0x0000000000ab9abe in main (argc=2, argv=0x136b7e0)
    at /home/nikic/php-src/sapi/cli/php_cli.c:1334

@laruence
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's knew... :)

@dstogov
Copy link
Member Author

@dstogov dstogov commented on 276080e Apr 17, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.