Fixed destruction of objects and iterators on unclean request shutdown and GC (few cases are still unfixed).

Now we destroy objects it two steps. At first - object properties of all objects and only then the objects their selves.

15 files changed, 114 insertions, 99 deletions

diff --git a/Zend/zend_API.c b/Zend/zend_API.c
index 104c5b3ed4..d9e926b685 100644
--- a/Zend/zend_API.c
+++ b/Zend/zend_API.c
@@ -3158,7 +3158,7 @@ ZEND_API zend_bool zend_is_callable_ex(zval *callable, zend_object *object, uint
 
 	if (object &&
 	    (!EG(objects_store).object_buckets ||
-	     !IS_VALID(EG(objects_store).object_buckets[object->handle]))) {
+	     !IS_OBJ_VALID(EG(objects_store).object_buckets[object->handle]))) {
 		return 0;
 	}
 
@@ -3246,7 +3246,7 @@ ZEND_API zend_bool zend_is_callable_ex(zval *callable, zend_object *object, uint
 
 					} else if (Z_TYPE_P(obj) == IS_OBJECT) {
 						if (!EG(objects_store).object_buckets ||
-						    !IS_VALID(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(obj)])) {
+						    !IS_OBJ_VALID(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(obj)])) {
 							return 0;
 						}
 
diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c
index 0fa0a6aec3..238718b0ce 100644
--- a/Zend/zend_closures.c
+++ b/Zend/zend_closures.c
@@ -241,9 +241,6 @@ static void zend_closure_free_storage(zend_object *object TSRMLS_DC) /* {{{ */
 	if (Z_TYPE(closure->this_ptr) != IS_UNDEF) {
 		zval_ptr_dtor(&closure->this_ptr);
 	}
-
-	GC_REMOVE_FROM_BUFFER(closure);
-	efree(closure);
 }
 /* }}} */
 
diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c
index 018e473995..cf321398f8 100644
--- a/Zend/zend_execute_API.c
+++ b/Zend/zend_execute_API.c
@@ -843,7 +843,7 @@ int zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_cache TS
 	EX(object) = fci->object = fci_cache->object;
 	if (fci->object &&
 	    (!EG(objects_store).object_buckets ||
-	     !IS_VALID(EG(objects_store).object_buckets[fci->object->handle]))) {
+	     !IS_OBJ_VALID(EG(objects_store).object_buckets[fci->object->handle]))) {
 		return FAILURE;
 	}
 
diff --git a/Zend/zend_gc.c b/Zend/zend_gc.c
index 1b3e347ed0..ec72d1e5ce 100644
--- a/Zend/zend_gc.c
+++ b/Zend/zend_gc.c
@@ -218,7 +218,7 @@ tail_call:
 		zend_object_get_gc_t get_gc;
 		zend_object *obj = (zend_object*)ref;
 
-		if (EXPECTED(IS_VALID(EG(objects_store).object_buckets[obj->handle]) &&
+		if (EXPECTED(IS_OBJ_VALID(EG(objects_store).object_buckets[obj->handle]) &&
 		             (get_gc = obj->handlers->get_gc) != NULL)) {
 			int i, n;
 			zval *table;
@@ -302,7 +302,7 @@ tail_call:
 			zend_object_get_gc_t get_gc;
 			zend_object *obj = (zend_object*)ref;
 
-			if (EXPECTED(IS_VALID(EG(objects_store).object_buckets[obj->handle]) &&
+			if (EXPECTED(IS_OBJ_VALID(EG(objects_store).object_buckets[obj->handle]) &&
 		             (get_gc = obj->handlers->get_gc) != NULL)) {
 				int i, n;
 				zval *table;
@@ -399,7 +399,7 @@ tail_call:
 				zend_object_get_gc_t get_gc;
 				zend_object *obj = (zend_object*)ref;
 
-				if (EXPECTED(IS_VALID(EG(objects_store).object_buckets[obj->handle]) &&
+				if (EXPECTED(IS_OBJ_VALID(EG(objects_store).object_buckets[obj->handle]) &&
 				             (get_gc = obj->handlers->get_gc) != NULL)) {
 					int i, n;
 					zval *table;
@@ -490,7 +490,7 @@ tail_call:
 			/* PURPLE instead of BLACK to prevent buffering in nested gc calls */
 //???			GC_SET_PURPLE(GC_INFO(obj));
 
-			if (EXPECTED(IS_VALID(EG(objects_store).object_buckets[obj->handle]) &&
+			if (EXPECTED(IS_OBJ_VALID(EG(objects_store).object_buckets[obj->handle]) &&
 			             (get_gc = obj->handlers->get_gc) != NULL)) {
 				int i, n;
 				zval *table;
@@ -634,14 +634,15 @@ ZEND_API int gc_collect_cycles(TSRMLS_D)
 				zend_object *obj = (zend_object*)p;
 
 				if (EG(objects_store).object_buckets &&
-				    obj->handlers->dtor_obj &&
-				    IS_VALID(EG(objects_store).object_buckets[obj->handle]) &&
+				    IS_OBJ_VALID(EG(objects_store).object_buckets[obj->handle]) &&
 					!(GC_FLAGS(obj) & IS_OBJ_DESTRUCTOR_CALLED)) {
 
 					GC_FLAGS(obj) |= IS_OBJ_DESTRUCTOR_CALLED;
-					GC_REFCOUNT(obj)++;
-					obj->handlers->dtor_obj(obj TSRMLS_CC);
-					GC_REFCOUNT(obj)--;
+					if (obj->handlers->dtor_obj) {
+						GC_REFCOUNT(obj)++;
+						obj->handlers->dtor_obj(obj TSRMLS_CC);
+						GC_REFCOUNT(obj)--;
+					}
 				}
 			}
 			current = GC_G(next_to_free);
@@ -656,17 +657,41 @@ ZEND_API int gc_collect_cycles(TSRMLS_D)
 				zend_object *obj = (zend_object*)p;
 
 				if (EG(objects_store).object_buckets &&
-					IS_VALID(EG(objects_store).object_buckets[obj->handle])) {
-					GC_TYPE(obj) = IS_NULL;
-					zend_objects_store_free(obj TSRMLS_CC);
+				    IS_OBJ_VALID(EG(objects_store).object_buckets[obj->handle]) &&
+					!(GC_FLAGS(obj) & IS_OBJ_FREE_CALLED)) {
+
+					GC_FLAGS(obj) |= IS_OBJ_FREE_CALLED;
+					if (obj->handlers->free_obj) {
+						GC_REFCOUNT(obj)++;
+						obj->handlers->free_obj(obj TSRMLS_CC);
+						GC_REFCOUNT(obj)--;
+					}
 				}
 			} else if (GC_TYPE(p) == IS_ARRAY) {
 				zend_array *arr = (zend_array*)p;
 
 				GC_TYPE(arr) = IS_NULL;
 				zend_hash_destroy(&arr->ht);
-				GC_REMOVE_FROM_BUFFER(arr);
-				efree(arr);
+			}
+			current = GC_G(next_to_free);
+		}
+
+		/* Free objects */
+		current = GC_G(to_free).next;
+		while (current != &GC_G(to_free)) {
+			p = current->ref;
+			GC_G(next_to_free) = current->next;
+			if (GC_TYPE(p) == IS_OBJECT) {
+				zend_object *obj = (zend_object*)p;
+
+				if (EG(objects_store).object_buckets &&
+				    IS_OBJ_VALID(EG(objects_store).object_buckets[obj->handle])) {
+
+					zend_objects_store_free(obj TSRMLS_CC);
+				}
+			} else {
+				GC_REMOVE_FROM_BUFFER(p);
+				efree(p);
 			}
 			current = GC_G(next_to_free);
 		}
diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c
index 13808a0eb4..559e9f2e20 100644
--- a/Zend/zend_generators.c
+++ b/Zend/zend_generators.c
@@ -212,12 +212,9 @@ static void zend_generator_free_storage(zend_object *object TSRMLS_DC) /* {{{ */
 
 	zend_object_std_dtor(&generator->std TSRMLS_CC);
 
-	if (generator->iterator.std.handle) {
-		zend_iterator_dtor(&generator->iterator TSRMLS_CC);
+	if (generator->iterator) {
+		zend_iterator_dtor(generator->iterator TSRMLS_CC);
 	}
-
-	GC_REMOVE_FROM_BUFFER(generator);
-	efree(generator);
 }
 /* }}} */
 
@@ -599,7 +596,9 @@ ZEND_METHOD(Generator, __wakeup)
 
 static void zend_generator_iterator_dtor(zend_object_iterator *iterator TSRMLS_DC) /* {{{ */
 {
+	zend_generator *generator = (zend_generator*)Z_OBJ(iterator->data);
 	zval_ptr_dtor(&iterator->data);
+	generator->iterator = NULL;
 }
 /* }}} */
 
@@ -679,7 +678,7 @@ zend_object_iterator *zend_generator_get_iterator(zend_class_entry *ce, zval *ob
 		return NULL;
 	}
 
-	iterator = &generator->iterator;
+	iterator = generator->iterator = emalloc(sizeof(zend_object_iterator));
 	
 	zend_iterator_init(iterator TSRMLS_CC);
 
diff --git a/Zend/zend_generators.h b/Zend/zend_generators.h
index 1209bc2d62..999ead4b6e 100644
--- a/Zend/zend_generators.h
+++ b/Zend/zend_generators.h
@@ -28,7 +28,7 @@ extern ZEND_API zend_class_entry *zend_ce_generator;
 typedef struct _zend_generator {
 	zend_object std;
 
-	zend_object_iterator iterator;
+	zend_object_iterator *iterator;
 
 	/* The suspended execution context. */
 	zend_execute_data *execute_data;
diff --git a/Zend/zend_interfaces.c b/Zend/zend_interfaces.c
index ce659ba32a..3e9d9378b6 100644
--- a/Zend/zend_interfaces.c
+++ b/Zend/zend_interfaces.c
@@ -153,7 +153,6 @@ static void zend_user_it_dtor(zend_object_iterator *_iter TSRMLS_DC)
 
 	zend_user_it_invalidate_current(_iter TSRMLS_CC);
 	zval_ptr_dtor(object);
-	efree(iter);
 }
 /* }}} */
 
diff --git a/Zend/zend_iterators.c b/Zend/zend_iterators.c
index 89e0d19c66..9fa4084d63 100644
--- a/Zend/zend_iterators.c
+++ b/Zend/zend_iterators.c
@@ -27,6 +27,7 @@ static zend_class_entry zend_iterator_class_entry;
 static void iter_wrapper_dtor(zend_object *object TSRMLS_DC);
 
 static zend_object_handlers iterator_object_handlers = {
+	0,
 	iter_wrapper_dtor,
 	NULL,
 	NULL,
diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c
index 03fd7b76c2..2150ad5773 100644
--- a/Zend/zend_object_handlers.c
+++ b/Zend/zend_object_handlers.c
@@ -1620,7 +1620,9 @@ int zend_std_get_closure(zval *obj, zend_class_entry **ce_ptr, zend_function **f
 /* }}} */
 
 ZEND_API zend_object_handlers std_object_handlers = {
-	zend_object_free,						/* free_obj */
+	0,										/* offset */
+
+	zend_object_std_dtor,					/* free_obj */
 	zend_objects_destroy_object,			/* dtor_obj */
 	zend_objects_clone_obj,					/* clone_obj */
 
diff --git a/Zend/zend_object_handlers.h b/Zend/zend_object_handlers.h
index be099aee81..249effa10e 100644
--- a/Zend/zend_object_handlers.h
+++ b/Zend/zend_object_handlers.h
@@ -116,6 +116,8 @@ typedef HashTable *(*zend_object_get_gc_t)(zval *object, zval **table, int *n TS
 typedef int (*zend_object_do_operation_t)(zend_uchar opcode, zval *result, zval *op1, zval *op2 TSRMLS_DC);
 
 struct _zend_object_handlers {
+	/* offset of real object header (usually zero) */
+	int										offset;
 	/* general object functions */
 	zend_object_free_obj_t					free_obj;
 	zend_object_dtor_obj_t					dtor_obj;
diff --git a/Zend/zend_objects.c b/Zend/zend_objects.c
index 35b237e132..34a4ddf4a7 100644
--- a/Zend/zend_objects.c
+++ b/Zend/zend_objects.c
@@ -128,13 +128,6 @@ ZEND_API void zend_objects_destroy_object(zend_object *object TSRMLS_DC)
 	}
 }
 
-ZEND_API void zend_object_free(zend_object *object TSRMLS_DC)
-{
-	zend_object_std_dtor(object TSRMLS_CC);
-	GC_REMOVE_FROM_BUFFER(object);
-	efree(object);
-}
-
 ZEND_API zend_object *zend_objects_new(zend_class_entry *ce TSRMLS_DC)
 {
 	zend_object *object = emalloc(sizeof(zend_object) + sizeof(zval) * (ce->default_properties_count - 1));
diff --git a/Zend/zend_objects.h b/Zend/zend_objects.h
index ac0adf8634..1521d9756b 100644
--- a/Zend/zend_objects.h
+++ b/Zend/zend_objects.h
@@ -31,7 +31,6 @@ ZEND_API zend_object *zend_objects_new(zend_class_entry *ce TSRMLS_DC);
 ZEND_API void zend_objects_destroy_object(zend_object *object TSRMLS_DC);
 ZEND_API void zend_objects_clone_members(zend_object *new_object, zend_object *old_object TSRMLS_DC);
 ZEND_API zend_object *zend_objects_clone_obj(zval *object TSRMLS_DC);
-ZEND_API void zend_object_free(zend_object *object TSRMLS_DC);
 END_EXTERN_C()
 
 #endif /* ZEND_OBJECTS_H */
diff --git a/Zend/zend_objects_API.c b/Zend/zend_objects_API.c
index a65b4a993e..541b2a465f 100644
--- a/Zend/zend_objects_API.c
+++ b/Zend/zend_objects_API.c
@@ -42,12 +42,12 @@ ZEND_API void zend_objects_store_destroy(zend_objects_store *objects)
 
 ZEND_API void zend_objects_store_call_destructors(zend_objects_store *objects TSRMLS_DC)
 {
-	zend_uint i = 1;
+	zend_uint i;
 
 	for (i = 1; i < objects->top ; i++) {
 		zend_object *obj = objects->object_buckets[i];
 
-		if (IS_VALID(obj)) {
+		if (IS_OBJ_VALID(obj)) {
 			if (!(GC_FLAGS(obj) & IS_OBJ_DESTRUCTOR_CALLED)) {
 				GC_FLAGS(obj) |= IS_OBJ_DESTRUCTOR_CALLED;
 				GC_REFCOUNT(obj)++;
@@ -68,7 +68,7 @@ ZEND_API void zend_objects_store_mark_destructed(zend_objects_store *objects TSR
 	for (i = 1; i < objects->top ; i++) {
 		zend_object *obj = objects->object_buckets[i];
 
-		if (IS_VALID(obj)) {
+		if (IS_OBJ_VALID(obj)) {
 			GC_FLAGS(obj) |= IS_OBJ_DESTRUCTOR_CALLED;
 		}
 	}
@@ -76,17 +76,33 @@ ZEND_API void zend_objects_store_mark_destructed(zend_objects_store *objects TSR
 
 ZEND_API void zend_objects_store_free_object_storage(zend_objects_store *objects TSRMLS_DC)
 {
-	zend_uint i = 1;
+	zend_uint i;
 
+	/* Free object properties but don't free object their selves */
 	for (i = 1; i < objects->top ; i++) {
 		zend_object *obj = objects->object_buckets[i];
 
-		if (IS_VALID(obj)) {
-			objects->object_buckets[i] = SET_INVALID(obj);
-			if (obj->handlers->free_obj) {
-				obj->handlers->free_obj(obj TSRMLS_CC);
+		if (IS_OBJ_VALID(obj)) {
+			if (!(GC_FLAGS(obj) & IS_OBJ_FREE_CALLED)) {
+				GC_FLAGS(obj) |= IS_OBJ_FREE_CALLED;
+				if (obj->handlers->free_obj) {
+					GC_REFCOUNT(obj)++;
+					obj->handlers->free_obj(obj TSRMLS_CC);
+					GC_REFCOUNT(obj)--;
+				}
 			}
+		}
+	}
+
+	/* Now free objects theirselves */
+	for (i = 1; i < objects->top ; i++) {
+		zend_object *obj = objects->object_buckets[i];
+
+		if (IS_OBJ_VALID(obj)) {
 			/* Not adding to free list as we are shutting down anyway */
+			void *ptr = ((char*)obj) - obj->handlers->offset;
+			GC_REMOVE_FROM_BUFFER(obj);
+			efree(ptr);
 		}
 	}
 }
@@ -100,7 +116,7 @@ ZEND_API void zend_objects_store_put(zend_object *object TSRMLS_DC)
 
 	if (EG(objects_store).free_list_head != -1) {
 		handle = EG(objects_store).free_list_head;
-		EG(objects_store).free_list_head = GET_BUCKET_NUMBER(EG(objects_store).object_buckets[handle]);
+		EG(objects_store).free_list_head = GET_OBJ_BUCKET_NUMBER(EG(objects_store).object_buckets[handle]);
 	} else {
 		if (EG(objects_store).top == EG(objects_store).size) {
 			EG(objects_store).size <<= 1;
@@ -113,17 +129,16 @@ ZEND_API void zend_objects_store_put(zend_object *object TSRMLS_DC)
 }
 
 #define ZEND_OBJECTS_STORE_ADD_TO_FREE_LIST(handle)															\
-            SET_BUCKET_NUMBER(EG(objects_store).object_buckets[handle], EG(objects_store).free_list_head);	\
+            SET_OBJ_BUCKET_NUMBER(EG(objects_store).object_buckets[handle], EG(objects_store).free_list_head);	\
 			EG(objects_store).free_list_head = handle;
 
 ZEND_API void zend_objects_store_free(zend_object *object TSRMLS_DC) /* {{{ */
 {
-	int handle = object->handle;
+	zend_uint handle = object->handle;
+	void *ptr = ((char*)object) - object->handlers->offset;
 
-	EG(objects_store).object_buckets[handle] = SET_INVALID(object);
-	if (object->handlers->free_obj) {
-		object->handlers->free_obj(object TSRMLS_CC);
-	}
+	GC_REMOVE_FROM_BUFFER(object);
+	efree(ptr);
 	ZEND_OBJECTS_STORE_ADD_TO_FREE_LIST(handle);
 }
 /* }}} */
@@ -135,7 +150,7 @@ ZEND_API void zend_objects_store_del(zend_object *object TSRMLS_DC) /* {{{ */
 		when the refcount reaches 0 a second time
 	 */
 	if (EG(objects_store).object_buckets &&
-	    IS_VALID(EG(objects_store).object_buckets[object->handle])) {
+	    IS_OBJ_VALID(EG(objects_store).object_buckets[object->handle])) {
 		if (GC_REFCOUNT(object) == 0) {
 			int failure = 0;
 
@@ -155,15 +170,24 @@ ZEND_API void zend_objects_store_del(zend_object *object TSRMLS_DC) /* {{{ */
 			
 			if (GC_REFCOUNT(object) == 0) {
 				zend_uint handle = object->handle;
-
-				EG(objects_store).object_buckets[handle] = SET_INVALID(object);
-				if (object->handlers->free_obj) {
-					zend_try {
-						object->handlers->free_obj(object TSRMLS_CC);
-					} zend_catch {
-						failure = 1;
-					} zend_end_try();
+				void *ptr;
+
+				EG(objects_store).object_buckets[handle] = SET_OBJ_INVALID(object);
+				if (!(GC_FLAGS(object) & IS_OBJ_FREE_CALLED)) {
+					GC_FLAGS(object) |= IS_OBJ_FREE_CALLED;
+					if (object->handlers->free_obj) {
+						zend_try {
+							GC_REFCOUNT(object)++;
+							object->handlers->free_obj(object TSRMLS_CC);
+							GC_REFCOUNT(object)++;
+						} zend_catch {
+							failure = 1;
+						} zend_end_try();
+					}
 				}
+				ptr = ((char*)object) - object->handlers->offset;
+				GC_REMOVE_FROM_BUFFER(object);
+				efree(ptr);
 				ZEND_OBJECTS_STORE_ADD_TO_FREE_LIST(handle);
 			}
 			
diff --git a/Zend/zend_objects_API.h b/Zend/zend_objects_API.h
index f3800c855b..4d6afe6842 100644
--- a/Zend/zend_objects_API.h
+++ b/Zend/zend_objects_API.h
@@ -24,43 +24,16 @@
 
 #include "zend.h"
 
-//???typedef void (*zend_objects_store_dtor_t)(zend_object *object TSRMLS_DC);
-//???typedef void (*zend_objects_free_object_storage_t)(void *object TSRMLS_DC);
-//???typedef void (*zend_objects_store_clone_t)(zend_object *object, zend_object **object_clone TSRMLS_DC);
-
-//???typedef union _zend_object_store_bucket {
-//???	zend_object *object;
-//???	int next_free;
-//???	zend_bool destructor_called;
-//???	zend_bool valid;
-//???	zend_uchar apply_count;
-//???	union _store_bucket {
-//???		struct _store_object {
-//???			zend_object *object;
-//???			zend_objects_store_dtor_t dtor;
-//???			zend_objects_free_object_storage_t free_storage;
-//???			zend_objects_store_clone_t clone;
-//???			const zend_object_handlers *handlers;
-//???			zend_uint refcount;
-//???			gc_root_buffer *buffered;
-//???		} obj;
-//???		zend_object *obj;
-//???		struct {
-//???			int next;
-//???		} free_list;
-//???	} bucket;
-//???} zend_object_store_bucket;
-
-#define FREE_BUCKET				1
-
-#define IS_VALID(o)				(!(((zend_uintptr_t)(o)) & FREE_BUCKET))
-
-#define SET_INVALID(o)			((zend_object*)((((zend_uintptr_t)(o)) | FREE_BUCKET)))
-
-#define GET_BUCKET_NUMBER(o)	(((zend_intptr_t)(o)) >> 1)
-
-#define SET_BUCKET_NUMBER(o, n)	do { \
-		(o) = (zend_object*)((((zend_uintptr_t)(n)) << 1) | FREE_BUCKET); \
+#define OBJ_BUCKET_INVALID			(1<<0)
+
+#define IS_OBJ_VALID(o)				(!(((zend_uintptr_t)(o)) & OBJ_BUCKET_INVALID))
+
+#define SET_OBJ_INVALID(o)			((zend_object*)((((zend_uintptr_t)(o)) | OBJ_BUCKET_INVALID)))
+
+#define GET_OBJ_BUCKET_NUMBER(o)	(((zend_intptr_t)(o)) >> 1)
+
+#define SET_OBJ_BUCKET_NUMBER(o, n)	do { \
+		(o) = (zend_object*)((((zend_uintptr_t)(n)) << 1) | OBJ_BUCKET_INVALID); \
 	} while (0)
 
 
@@ -98,7 +71,7 @@ ZEND_API void zend_object_store_ctor_failed(zend_object *object TSRMLS_DC);
 
 ZEND_API void zend_objects_store_free_object_storage(zend_objects_store *objects TSRMLS_DC);
 
-#define ZEND_OBJECTS_STORE_HANDLERS zend_object_free, zend_object_std_dtor, zend_objects_clone_obj
+#define ZEND_OBJECTS_STORE_HANDLERS 0, zend_object_std_dtor, zend_objects_destroy_object, zend_objects_clone_obj
 
 ZEND_API zend_object *zend_object_create_proxy(zval *object, zval *member TSRMLS_DC);
 
diff --git a/Zend/zend_types.h b/Zend/zend_types.h
index 8ace69a7db..8baf8643af 100644
--- a/Zend/zend_types.h
+++ b/Zend/zend_types.h
@@ -305,6 +305,7 @@ static inline zend_uchar zval_get_type(const zval* pz) {
 /* object flags (zval.value->gc.u.flags) */
 #define IS_OBJ_APPLY_COUNT			0x07
 #define IS_OBJ_DESTRUCTOR_CALLED	(1<<3)
+#define IS_OBJ_FREE_CALLED			(1<<4)
 
 #define Z_OBJ_APPLY_COUNT(zval) \
 	(Z_GC_FLAGS(zval) & IS_OBJ_APPLY_COUNT)