Commits

Ruslan Osmanov committed 5ba016d

issue #3: Segmentation fault on EventHttpRequest->free() (Bitbucket's tracker)

Comments (0)

Files changed (5)

 	pfci->param_count	 = 2;
 	pfci->no_separation  = 1;
 
-    if (zend_call_function(pfci, pfcc TSRMLS_CC) == SUCCESS && retval_ptr) {
-        zval_ptr_dtor(&retval_ptr);
-    } else {
-        php_error_docref(NULL TSRMLS_CC, E_WARNING,
-                "An error occurred while invoking the http request callback");
-    }
-
-    zval_ptr_dtor(&arg_req);
-    zval_ptr_dtor(&arg_data);
+	if (zend_call_function(pfci, pfcc TSRMLS_CC) == SUCCESS && retval_ptr) {
+		zval_ptr_dtor(&retval_ptr);
+	} else {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING,
+				"An error occurred while invoking the http request callback");
+	}
+
+	zval_ptr_dtor(&arg_req);
+	zval_ptr_dtor(&arg_data);
 }
 /* }}} */
 
 	pfci->param_count	 = 2;
 	pfci->no_separation  = 1;
 
-    if (zend_call_function(pfci, pfcc TSRMLS_CC) == SUCCESS && retval_ptr) {
-        zval_ptr_dtor(&retval_ptr);
-    } else {
-        php_error_docref(NULL TSRMLS_CC, E_WARNING,
-                "An error occurred while invoking the http request callback");
-    }
+	if (zend_call_function(pfci, pfcc TSRMLS_CC) == SUCCESS && retval_ptr) {
+		zval_ptr_dtor(&retval_ptr);
+	} else {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING,
+				"An error occurred while invoking the http request callback");
+	}
 
-    zval_ptr_dtor(&arg_req);
-    zval_ptr_dtor(&arg_data);
+	zval_ptr_dtor(&arg_req);
+	zval_ptr_dtor(&arg_data);
 }
 /* }}} */
 

classes/http_request.c

 	/* Tell Libevent that we will free the request ourselves(evhttp_request_free in the free-storage handler)*/
 	evhttp_request_own(http_req->ptr);
 
-    if (zend_call_function(pfci, pfcc TSRMLS_CC) == SUCCESS && retval_ptr) {
-        zval_ptr_dtor(&retval_ptr);
-    } else {
-        php_error_docref(NULL TSRMLS_CC, E_WARNING,
-                "An error occurred while invoking the http request callback");
-    }
-
-    zval_ptr_dtor(&arg_req);
-    zval_ptr_dtor(&arg_data);
+	if (zend_call_function(pfci, pfcc TSRMLS_CC) == SUCCESS && retval_ptr) {
+		zval_ptr_dtor(&retval_ptr);
+	} else {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING,
+				"An error occurred while invoking the http request callback");
+	}
+
+	zval_ptr_dtor(&arg_req);
+	zval_ptr_dtor(&arg_data);
 }
 /* }}} */
 
 		return;
 	}
 
-	evhttp_request_free(http_req->ptr);
-	http_req->ptr = NULL;
+	if (http_req->ptr) {
+		/*
+		 * We're not calling evhttp_request_free(http_req->ptr) because AFAIK
+		 * Libevent handles the memory of evhttp_request all right.
+		 *
+		 * It just so happens that libevent invokes the function itself in
+		 * evhttp_connection_cb_cleanup() despite the ownership of the request
+		 * (thus causing a SEGFAULT). See bitbucket issue #3.
+		 *
+		 * By marking http_req as `internal` we prevent calling evhttp_request_free()
+		 * within event_http_req_object_free_storage().
+		 */
+		http_req->internal = 1;
+		/*http_req->ptr = NULL;*/
+	}
 
 	/* Do it once */
 	if (http_req->self) {
 #endif
 
 /* {{{ proto EventHttpConnection EventHttpRequest::getConnection(void);
- * Returns EventHttpConnection object. 
+ * Returns EventHttpConnection object.
  *
  * Warning! Libevent API allows http request objects not bound to any http connection.
  * Therefore we can't unambiguously associate EventHttpRequest with EventHttpConnection.
     <email>remi@php.net</email>
     <active>yes</active>
   </contributor>
-  <date>2014-03-23</date>
+  <date>2014-04-12</date>
   <!--{{{ Current version -->
   <version>
-    <release>1.9.1</release>
+    <release>1.9.2</release>
     <api>1.8.0</api>
   </version>
   <stability>
   </stability>
   <license uri="http://www.php.net/license">PHP</license>
   <notes><![CDATA[
-    Fix: return value of EventBase::reInit() was inverted
-    issue #7: PHP_EVENT_REQUIRE_BASE_BY_REF() didn't work in 5.6.0-dev (fixed by Remi Collet)
+    issue #3: Segmentation fault on EventHttpRequest->free() (Bitbucket's tracker)
   ]]></notes>
   <!--}}}-->
   <!--{{{ Contents -->
         <file role="test" name="20-bevent-error.phpt"/>
         <file role="test" name="21-bevent-sslfilter-error.phpt"/>
         <file role="test" name="21-bevent-sslsocket-error.phpt"/>
+        <file role="test" name="issue-3.phpt"/>
       </dir>
     </dir>
   </contents>
   </extsrcrelease>
   <!--{{{ changelog-->
   <changelog>
+    <!--{{{ 1.9.2 -->
+    <release>
+      <version>
+        <release>1.9.2</release>
+        <api>1.8.0</api>
+      </version>
+      <stability>
+        <release>stable</release>
+        <api>stable</api>
+      </stability>
+      <license uri="http://www.php.net/license">PHP</license>
+      <notes><![CDATA[
+    issue #3: Segmentation fault on EventHttpRequest->free() (Bitbucket's tracker)
+  ]]></notes>
+    </release>
+    <!--}}}-->
     <!--{{{ 1.9.1 -->
     <release>
       <version>
 #ifndef PHP_EVENT_H
 #define PHP_EVENT_H
 
-#define PHP_EVENT_VERSION "1.9.1"
+#define PHP_EVENT_VERSION "1.9.2"
 
 #define PHP_EVENT_SUN_PREFIX "unix:"
 

tests/issue-3.phpt

+--TEST--
+Check for issue #3: SEGFAULT after calling EventHttpRequest::free()
+--SKIPIF--
+<?php
+if (!class_exists("EventHttpRequest")) die("skip Event extra functions are disabled");
+?>
+--FILE--
+<?php
+function _callback_server($req) {
+	$req->sendReply(200, "OK");
+}
+
+function _callback_client($req) {
+	global $inc;
+	echo $inc, PHP_EOL;
+	//echo $inc++, ' -> ',  memory_get_usage(true), PHP_EOL;
+	// Free request and get SegFault or... get memory grows
+	$req->free();
+}
+
+function _callback_timer(){
+	global $timer, $conn, $base, $inc;
+
+	$req = new EventHttpRequest("_callback_client", $base);
+	$conn->makeRequest($req, EventHttpRequest::CMD_GET, "/");
+
+	if (++$inc < 30) {
+		$timer->addTimer(0.01);
+	}
+}
+
+
+$inc  = 0;
+$host = "0.0.0.0";
+$port = 8181;
+
+$base = new EventBase();
+
+$http = new EventHttp($base);
+$http->setAllowedMethods(EventHttpRequest::CMD_GET);
+$http->setDefaultCallback("_callback_server");
+$http->bind($host, $port);
+
+$timer = Event::timer($base,  '_callback_timer');
+$timer->addTimer(1);
+
+$conn = new EventHttpConnection($base, NULL, $host, $port);
+
+$base->loop();
+?>
+--EXPECT--
+1
+2
+3
+4
+5
+6
+7
+8
+9
+10
+11
+12
+13
+14
+15
+16
+17
+18
+19
+20
+21
+22
+23
+24
+25
+26
+27
+28
+29
+30
+