Merge pull request #12 from pinepain/fix_notifiers_calling

Fix notifiers calling behavior

README.md

@@ -27,14 +27,15 @@ $obj = null; // outputs "Object destroyed"
 
 This extension adds `Weak` namespace and all entities are created inside it.
 
-There are no INI setting, constants or exceptions provided by this extension.
+There are no INI setting or constants provided by this extension.
 
 Brief docs about [`Weak\Reference` class](./stubs/weak/Reference.php) and [functions](./stubs/weak/functions.php)
 may be seen in [stub files](./stubs/weak).
 
 Short list if what provided by this extension is:
 
   - `class Weak\Reference`
+  - `class Weak\NotifierException extend Exception`
   - `function Weak\refcounted()`
   - `function Weak\refcount()`
   - `function Weak\weakrefcounted()`
@@ -51,8 +52,9 @@ Note that notification happens *after* referent object destruction, so at the ti
 will return `null` (unless rare case when object refcount get incremented in destructor, e.g. by storing destructing value
 somewhere else).
 
-Callback notifier will not be called if referent object destructor or previous notifier callback throws exception, whether
-array notifier get executed even in such cases.
+If object destructor or one or more notifiers throw exception, all further notifier callbacks will be called as if
+that exception was thrown inside `try-catch` block. In case one or more exception was thrown, `Weak\NotifierException`
+will be thrown and all thrown exceptions will be available via `Weak\NotifierException::getExceptions()` method.
 
 
 ### Cloning
@@ -157,16 +159,28 @@ You may also want to add php-weak extension as a [composer.json dependency](http
 with custom one, which meta-code is:
 
 ```php
-run_original_dtor_obj($object);
+$exceptions = [];
+
+try {
+    run_original_dtor_obj($object);
+} catch(Throwable $e) {
+    $exceptions[] = $e;
+}
 
 foreach($weak_references as $weak_ref_object_handle => $weak_reference) {
     if (is_array($weak_reference->notifier)) {
         $weak_reference->notifier[] = $weak_reference;
-    } elseif (is_callable($weak_reference->notifier) && $no_exception_thrown) {
-        $weak_reference->notifier($weak_reference);
+    } elseif (is_callable($weak_reference->notifier)) {
+        try {
+            $weak_reference->notifier($weak_reference);
+        } catch(Throwable $e) {
+            $exceptions[] = $e;
+        }
     }
-
-    unset($weak_references[$weak_ref_object_handle]);
+}
+
+if ($exceptions) {
+    throw new Weak\NotifierException('One or more exceptions thrown during notifiers calling', $exceptions);
 }
 ```
 

config.m4

@@ -27,9 +27,10 @@ if test "$PHP_WEAK" != "no"; then
         fi
     fi
 
-    PHP_NEW_EXTENSION(weak, [   \
-        weak.c                  \
-        php_weak_reference.c    \
-        php_weak_functions.c    \
+    PHP_NEW_EXTENSION(weak, [           \
+        weak.c                          \
+        php_weak_notifier_exception.c   \
+        php_weak_reference.c            \
+        php_weak_functions.c            \
     ], $ext_shared,, -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1)
 fi

config.w32

@@ -4,5 +4,5 @@
 ARG_ENABLE("weak", "enable weak support", "no");
 
 if (PHP_WEAK != "no") {
-	EXTENSION("weak", "weak.c php_weak_reference.c php_weak_functions.c", PHP_WEAK_SHARED, "/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1");
+	EXTENSION("weak", "weak.c php_weak_notifier_exception.c php_weak_reference.c php_weak_functions.c", PHP_WEAK_SHARED, "/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1");
 }

php_weak_notifier_exception.c

@@ -0,0 +1,131 @@
+/*
+  +----------------------------------------------------------------------+
+  | This file is part of the pinepain/php-weak PHP extension.            |
+  |                                                                      |
+  | Copyright (c) 2016 Bogdan Padalko <[email protected]>            |
+  |                                                                      |
+  | Licensed under the MIT license: http://opensource.org/licenses/MIT   |
+  |                                                                      |
+  | For the full copyright and license information, please view the      |
+  | LICENSE file that was distributed with this source or visit          |
+  | http://opensource.org/licenses/MIT                                   |
+  +----------------------------------------------------------------------+
+*/
+
+#include "php_weak_notifier_exception.h"
+#include "php_weak.h"
+#include "zend_exceptions.h"
+
+
+zend_class_entry *php_weak_notifier_exception_class_entry;
+#define this_ce php_weak_notifier_exception_class_entry
+
+
+static zend_object *php_weak_notifier_exception_ctor(zend_class_entry *ce) /* {{{ */
+{
+    zval obj, thrown;
+    zend_object *object;
+
+    Z_OBJ(obj) = object = ce->parent->create_object(ce);
+
+    array_init_size(&thrown, 0);
+    zend_update_property(php_weak_notifier_exception_class_entry, &obj, ZEND_STRL("exceptions"), &thrown);
+
+    return object;
+} /* }}} */
+
+
+void php_weak_create_notifier_exception(zval *exception, const char *message, zval *thrown) /* {{{ */
+{
+    object_init_ex(exception, this_ce);
+    zend_update_property_string(zend_ce_exception, exception, ZEND_STRL("message"), message);
+    zend_update_property(php_weak_notifier_exception_class_entry, exception, ZEND_STRL("exceptions"), thrown);
+} /* }}} */
+
+static PHP_METHOD(NotifierException, __construct)  /* {{{ */
+{
+	zend_string *message = NULL;
+	zend_long   code = 0;
+
+	zval  tmp;
+    zval *exceptions = NULL;
+    zval *previous = NULL;
+
+    if (zend_parse_parameters(ZEND_NUM_ARGS(), "|SalO!", &message, &exceptions, &code, &previous, zend_ce_throwable) == FAILURE) {
+        return;
+    }
+
+    if (message) {
+		zend_update_property_str(zend_ce_exception, getThis(), ZEND_STRL("message"), message);
+	}
+
+    if (exceptions) {
+        zend_update_property(this_ce, getThis(), ZEND_STRL("exceptions"), exceptions);
+    } else {
+        array_init_size(&tmp, 0);
+        zend_update_property(this_ce, getThis(), ZEND_STRL("exceptions"), &tmp);
+    }
+
+    if (code) {
+        zend_update_property_long(zend_ce_exception, getThis(), ZEND_STRL("code"), code);
+	}
+
+	if (previous) {
+        zend_update_property(zend_ce_exception, getThis(), ZEND_STRL("previous"), previous);
+    }
+}
+
+static PHP_METHOD(NotifierException, getExceptions)  /* {{{ */
+{
+    zval rv;
+
+    if (zend_parse_parameters_none() == FAILURE) {
+        return;
+    }
+
+    RETVAL_ZVAL(zend_read_property(php_weak_notifier_exception_class_entry, getThis(), ZEND_STRL("exceptions"), 0, &rv), 1, 0);
+} /* }}} */
+
+
+ZEND_BEGIN_ARG_INFO_EX(arginfo_notifier_exception___construct, ZEND_SEND_BY_VAL, ZEND_RETURN_VALUE, 0)
+	ZEND_ARG_INFO(0, message)
+    ZEND_ARG_INFO(0, exceptions)
+	ZEND_ARG_INFO(0, code)
+	ZEND_ARG_INFO(0, previous)
+ZEND_END_ARG_INFO()
+
+ZEND_BEGIN_ARG_INFO_EX(arginfo_notifier_exception_getExceptions, ZEND_SEND_BY_VAL, ZEND_RETURN_VALUE, 0)
+ZEND_END_ARG_INFO()
+
+
+static const zend_function_entry php_weak_notifier_exception_methods[] = { /* {{{ */
+        PHP_ME(NotifierException, __construct,   arginfo_notifier_exception___construct,   ZEND_ACC_PUBLIC | ZEND_ACC_CTOR)
+        PHP_ME(NotifierException, getExceptions, arginfo_notifier_exception_getExceptions, ZEND_ACC_PUBLIC)
+
+        PHP_FE_END
+}; /* }}} */
+
+
+PHP_MINIT_FUNCTION (php_weak_notifier_exception) /* {{{ */
+{
+    zend_class_entry ce;
+
+    INIT_NS_CLASS_ENTRY(ce, PHP_WEAK_NS, "NotifierException", php_weak_notifier_exception_methods);
+    this_ce = zend_register_internal_class_ex(&ce, zend_ce_exception);
+    /*this_ce->create_object = php_weak_notifier_exception_ctor;*/
+
+    zend_declare_property_null(this_ce, ZEND_STRL("exceptions"), ZEND_ACC_PRIVATE);
+
+
+    return SUCCESS;
+} /* }}} */
+
+
+/*
+ * Local variables:
+ * tab-width: 4
+ * c-basic-offset: 4
+ * End:
+ * vim600: noet sw=4 ts=4 fdm=marker
+ * vim<600: noet sw=4 ts=4
+ */

php_weak_notifier_exception.h

@@ -0,0 +1,41 @@
+/*
+  +----------------------------------------------------------------------+
+  | This file is part of the pinepain/php-weak PHP extension.            |
+  |                                                                      |
+  | Copyright (c) 2016 Bogdan Padalko <[email protected]>            |
+  |                                                                      |
+  | Licensed under the MIT license: http://opensource.org/licenses/MIT   |
+  |                                                                      |
+  | For the full copyright and license information, please view the      |
+  | LICENSE file that was distributed with this source or visit          |
+  | http://opensource.org/licenses/MIT                                   |
+  +----------------------------------------------------------------------+
+*/
+
+#ifndef PHP_WEAK_NOTIFIER_EXCEPTION_H
+#define PHP_WEAK_NOTIFIER_EXCEPTION_H
+
+#include "php.h"
+
+#ifdef ZTS
+#include "TSRM.h"
+#endif
+
+extern zend_class_entry *php_weak_notifier_exception_class_entry;
+
+void php_weak_create_notifier_exception(zval *exception, const char *message, zval *thrown);
+
+PHP_MINIT_FUNCTION(php_weak_notifier_exception);
+
+
+#endif /* PHP_WEAK_NOTIFIER_EXCEPTION_H */
+
+
+/*
+ * Local variables:
+ * tab-width: 4
+ * c-basic-offset: 4
+ * End:
+ * vim600: noet sw=4 ts=4 fdm=marker
+ * vim<600: noet sw=4 ts=4
+ */

php_weak_reference.c

@@ -13,6 +13,7 @@
 */
 
 #include "php_weak_reference.h"
+#include "php_weak_notifier_exception.h"
 #include "php_weak.h"
 #include "zend_exceptions.h"
 #include "zend_interfaces.h"
@@ -32,6 +33,7 @@ static zend_object_get_debug_info_t spl_object_storage_get_debug_info_orig_handl
 
 php_weak_reference_t *php_weak_reference_init(zval *this_ptr, zval *referent_zv, zval *notifier_zv);
 
+static inline void php_weak_store_exceptions(zval *exceptions, zval *tmp);
 static int php_weak_reference_check_notifier(zval *notifier, zval *this);
 
 
@@ -197,17 +199,29 @@ void php_weak_reference_call_notifier(zval *reference, zval *notifier) /* {{{ */
 
 } /* }}} */
 
+
 void php_weak_referent_object_dtor_obj(zend_object *object) /* {{{ */
 {
     php_weak_referent_t *referent = php_weak_referent_find_ptr(object->handle);
 
+    zval exceptions;
+    zval tmp;
+
+    ZVAL_UNDEF(&exceptions);
+
     assert(NULL != referent);
     assert(NULL != PHP_WEAK_G(referents));
 
     zend_ulong handle;
     php_weak_reference_t *reference;
 
-    referent->original_handlers->dtor_obj(object);
+    if (referent->original_handlers->dtor_obj) {
+        referent->original_handlers->dtor_obj(object);
+
+        if (EG(exception)) {
+            php_weak_store_exceptions(&exceptions, &tmp);
+        }
+    }
 
     ZEND_HASH_REVERSE_FOREACH_PTR(&referent->weak_references, reference) {
         handle = _p->h;
@@ -221,11 +235,12 @@ void php_weak_referent_object_dtor_obj(zend_object *object) /* {{{ */
                 break;
             case PHP_WEAK_NOTIFIER_CALLBACK:
                 /* callback notifier */
-                if (!EG(exception)) {
-                    php_weak_reference_call_notifier(&reference->this_ptr, &reference->notifier);
+                php_weak_reference_call_notifier(&reference->this_ptr, &reference->notifier);
+
+                if (EG(exception)) {
+                    php_weak_store_exceptions(&exceptions, &tmp);
                 }
                 break;
-
             default:
                 break;
         }
@@ -234,6 +249,13 @@ void php_weak_referent_object_dtor_obj(zend_object *object) /* {{{ */
     } ZEND_HASH_FOREACH_END();
 
     zend_hash_index_del(PHP_WEAK_G(referents), referent->handle);
+
+    if (!Z_ISUNDEF(exceptions)) {
+        zval exception;
+        php_weak_create_notifier_exception(&exception, "One or more exceptions thrown during notifiers calling", &exceptions);
+
+        zend_throw_exception_object(&exception);
+    }
 } /* }}} */
 
 void php_weak_globals_referents_ht_dtor(zval *zv) /* {{{ */
@@ -340,6 +362,18 @@ php_weak_reference_t *php_weak_reference_init(zval *this_ptr, zval *referent_zv,
     return reference;
 } /* }}} */
 
+static inline void php_weak_store_exceptions(zval *exceptions, zval *tmp)
+{
+    if (Z_ISUNDEF_P(exceptions)) {
+        array_init(exceptions);
+    }
+
+    ZVAL_OBJ(tmp, EG(exception));
+    Z_ADDREF_P(tmp);
+    add_next_index_zval(exceptions, tmp);
+
+    zend_clear_exception();
+}
 
 static int php_weak_reference_check_notifier(zval *notifier, zval *this) /* {{{ */
 {
@@ -628,10 +662,10 @@ PHP_MINIT_FUNCTION (php_weak_reference) /* {{{ */
     zend_class_entry ce;
 
     INIT_NS_CLASS_ENTRY(ce, PHP_WEAK_NS, "Reference", php_weak_reference_methods);
-    ce.serialize = zend_class_serialize_deny;
-    ce.unserialize = zend_class_unserialize_deny;
     this_ce = zend_register_internal_class(&ce);
     this_ce->create_object = php_weak_reference_ctor;
+    this_ce->serialize = zend_class_serialize_deny;
+    this_ce->unserialize = zend_class_unserialize_deny;
 
     memcpy(&php_weak_reference_object_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
 

stubs/weak/NotifierException.php

@@ -0,0 +1,23 @@
+<?php
+
+
+namespace Weak;
+
+
+use Exception;
+
+
+class NotifierException extends Exception
+{
+    private $exceptions = [];
+
+    /**
+     * Get exceptions thrown from notifiers
+     *
+     * @return array
+     */
+    public function getExceptions() : array
+    {
+        return $this->exceptions;
+    }
+}