Commits

Ruslan Osmanov  committed 02864a8

Fix: assignment reference to data property caused 'Fatal error: Cannot assign by reference to overloaded object'
Fix: evnt object dtors sometimes didn't free the 'data' member until the script shutdown phase

  • Participants
  • Parent commits 8b85a8b

Comments (0)

Files changed (4)

     <email>osmanov@php.net</email>
     <active>yes</active>
   </lead>
-  <date>2013-07-20</date>
+  <date>2013-07-21</date>
   <!--{{{ Current version -->
   <version>
     <release>1.7.1</release>
   <license uri="http://www.php.net/license">PHP</license>
   <notes><![CDATA[
   Fix: segmentation fault on gc_collect_cycles() after calling Event::free(), 5lava @ Bitbucket reported
+  Fix: assignment reference to "data" property caused 'Fatal error:  Cannot assign by reference to overloaded object'
+  Fix: evnt object dtors sometimes didn't free the 'data' member until the script shutdown phase
   ]]></notes>
   <!--}}}-->
   <!--{{{ Contents -->
         <file role="src" name="07-listener-error.phpt"/>
         <file role="src" name="08-buffer.phpt"/>
         <file role="src" name="09-gc-cycles.phpt"/>
+        <file role="src" name="10-event-data-dtor.phpt"/>
       </dir>
     </dir>
   </contents>
       <license uri="http://www.php.net/license">PHP</license>
       <notes><![CDATA[
   Fix: segmentation fault on gc_collect_cycles() after calling Event::free(), 5lava @ Bitbucket reported
+  Fix: assignment reference to "data" property caused 'Fatal error:  Cannot assign by reference to overloaded object'
+  Fix: evnt object dtors sometimes didn't free the 'data' member until the script shutdown phase
   ]]></notes>
     </release>
     <!--}}}-->
 	}
 	if (ret == SUCCESS) {
 	    hnd->write_func(obj, value TSRMLS_CC);
-	    if (! PZVAL_IS_REF(value) && Z_REFCOUNT_P(value) == 0) {
-	        Z_ADDREF_P(value);
-	        zval_ptr_dtor(&value);
-	    }
 	} else {
 	    zend_object_handlers * std_hnd = zend_get_std_object_handlers();
 	    std_hnd->write_property(object, member, value, key TSRMLS_CC);
 		if (!(x)) return FAILURE; \
 	} while (0);
 
-static inline void _prop_write_zval(zval **ppz, const zval *value)
+static inline void _prop_write_zval(zval **ppz, zval *value)
 {
+#if 0
 	if (!*ppz) {
 		MAKE_STD_ZVAL(*ppz);
 	}
 	/* Make a copy of the zval, avoid direct binding to the address
 	 * of value, since it breaks refcount in read_property()
 	 * causing further leaks and memory access violations */
-	REPLACE_ZVAL_VALUE(ppz, value, 1);
+	REPLACE_ZVAL_VALUE(ppz, value, PZVAL_IS_REF((zval *)value));
+#endif
+	if (!*ppz) {
+		/* if we assign referenced variable, we should separate it */
+		Z_ADDREF_P(value);
+		if (PZVAL_IS_REF(value)) {
+			SEPARATE_ZVAL(&value);
+		}
+		*ppz = value;
+	} else if (PZVAL_IS_REF(*ppz)) {
+		zval garbage = **ppz; /* old value should be destroyed */
+
+		/* To check: can't *ppz be some system variable like error_zval here? */
+		Z_TYPE_PP(ppz) = Z_TYPE_P(value);
+		(*ppz)->value = value->value;
+		if (Z_REFCOUNT_P(value) > 0) {
+			zval_copy_ctor(*ppz);
+		}
+		zval_dtor(&garbage);
+	} else {
+		zval *garbage = *ppz;
+
+		/* if we assign referenced variable, we should separate it */
+		Z_ADDREF_P(value);
+		if (PZVAL_IS_REF(value)) {
+			SEPARATE_ZVAL(&value);
+		}
+		*ppz = value;
+		zval_ptr_dtor(&garbage);
+	}
 }
 
 static inline void _prop_read_zval(zval *pz, zval **retval)
 	}
 
 	MAKE_STD_ZVAL(*retval);
-	/*REPLACE_ZVAL_VALUE(retval, pz, 1);*/
 	ZVAL_ZVAL(*retval, pz, 1, 0);
 }
 
 	php_event_t *e = (php_event_t *) obj;
 
 	if (!e->event) return NULL;
-
-	return (e->data ? &e->data : NULL);
+	if (!e->data) {
+		MAKE_STD_ZVAL(e->data);
+	}
+	return &e->data;
 }
 /* }}} */
 

File tests/10-event-data-dtor.phpt

+--TEST--
+Check for event destructor depending on the data property value
+--FILE--
+<?php
+class _Indicator {
+	public $i;
+	public function __construct($i) {
+		$this->i = $i;
+	}
+	public function __destruct() {
+		echo $this->i, "\n";
+	}
+}
+
+$base = new EventBase();
+
+$e1 = new Event($base, -1, Event::TIMEOUT, function() {
+	echo "2\n";
+});
+$e1->addTimer(0.10);
+$e2 = new Event($base, -1, Event::TIMEOUT, function() {
+	echo "3\n";
+});
+$e2->addTimer(0.11);
+
+// obj
+$i1 = new _Indicator(1);
+$e1->data = &$i1;
+
+// obj by ref
+$i2 = new _Indicator(4);
+$e2->data = $i2;
+
+echo "start\n";
+$i1 = null;
+$i2 = null;
+
+$base->loop();
+
+$e1 = null;
+$e2 = null;
+echo "end";
+?>
+--EXPECT--
+start
+1
+2
+3
+4
+end