Stop prematurely freeing this pointer (#311)

* Ensure c extension test fails in CI when leaks are detected

* Massage Makefile

* No leaks no memory transgressions

* add Changelog

* remove function used only in 7.x that doesn't compile in 5.x

.circleci/config.yml

@@ -102,7 +102,7 @@ aliases:
   - &STEP_RUN_EXTENSION_TESTS
     run:
       name: Run extension tests with leak detection
-      command: TEST_PHP_JUNIT=$(pwd)/test-results/leak_test.xml REPORT_EXIT_STATUS=1 make test_c_mem
+      command: make test_extension_ci JUNIT_RESULTS_DIR=$(pwd)/test-results
 
   - &STEP_WAIT_AGENT
     run:

CHANGELOG.md

@@ -2,6 +2,8 @@
 All notable changes to this project will be documented in this file - [read more](docs/changelog.md).
 
 ## [Unreleased]
+### Fixed
+- 7.x handling of `$this` pointer passed to the closure causing errors in PHP VM #311
 
 ## [0.13.2]
 ### Added

Makefile

@@ -18,6 +18,8 @@ $(BUILD_DIR)/%: %
 	$(Q) mkdir -p $(dir $@)
 	$(Q) cp -a $* $@
 
+JUNIT_RESULTS_DIR := $(shell pwd)
+
 all: $(BUILD_DIR)/configure $(SO_FILE)
 Q := @
 
@@ -46,6 +48,18 @@ test_c: $(SO_FILE)
 test_c_mem: $(SO_FILE)
 	$(MAKE) -C $(BUILD_DIR) test TESTS="-q --show-all -m $(TESTS)"
 
+test_extension_ci: $(SO_FILE)
+	( \
+	set -xe; \
+	export REPORT_EXIT_STATUS=1; \
+	\
+	export TEST_PHP_JUNIT=$(JUNIT_RESULTS_DIR)/normal-extension-test.xml; \
+	$(MAKE) -C $(BUILD_DIR) test  TESTS="-q --show-all $(TESTS)" && grep -e 'errors="0"' $$TEST_PHP_JUNIT; \
+	\
+	export TEST_PHP_JUNIT=$(JUNIT_RESULTS_DIR)/valgrind-extension-test.xml; \
+	$(MAKE) -C $(BUILD_DIR) test  TESTS="-q  -m --show-all $(TESTS)" && grep -e 'errors="0"' $$TEST_PHP_JUNIT; \
+	)
+
 test_integration: install_ini
 	composer test -- $(PHPUNIT)
 
@@ -111,4 +125,4 @@ verify_version:
 
 verify_all: verify_pecl_file_definitions verify_version
 
-.PHONY: dist_clean clean all clang_format_fix install sudo_install test_c test_c_mem test test_integration install_ini install_all .apk .rpm .deb .tar.gz sudo debug strict run-tests.php verify_pecl_file_definitions verify_version verify_all
+.PHONY: dist_clean clean all clang_format_fix install sudo_install test_c test_c_mem test_extension_ci test test_integration install_ini install_all .apk .rpm .deb .tar.gz sudo debug strict run-tests.php verify_pecl_file_definitions verify_version verify_all

src/ext/dispatch.c

@@ -20,12 +20,27 @@
 #define RETURN_VALUE_USED(opline) (!((opline)->result_type & EXT_TYPE_UNUSED))
 #endif
 
+ZEND_EXTERN_MODULE_GLOBALS(ddtrace);
+
 #if PHP_VERSION_ID < 70000
 #undef EX
 #define EX(x) ((execute_data)->x)
-#endif
 
-ZEND_EXTERN_MODULE_GLOBALS(ddtrace);
+#else  // PHP7.0+
+// imported from PHP 7.2 as 7.0 missed this method
+zend_class_entry *get_executed_scope(void) {
+    zend_execute_data *ex = EG(current_execute_data);
+
+    while (1) {
+        if (!ex) {
+            return NULL;
+        } else if (ex->func && (ZEND_USER_CODE(ex->func->type) || ex->func->common.scope)) {
+            return ex->func->common.scope;
+        }
+        ex = ex->prev_execute_data;
+    }
+}
+#endif
 
 static ddtrace_dispatch_t *lookup_dispatch(const HashTable *lookup, const char *function_name,
                                            uint32_t function_name_length) {
@@ -140,7 +155,16 @@ static void execute_fcall(ddtrace_dispatch_t *dispatch, zend_execute_data *execu
 
 _exit_cleanup:
     if (this) {
+#if PHP_VERSION_ID < 70000
         Z_DELREF_P(this);
+
+#else
+        zend_function *constructor = Z_OBJ_HT_P(this)->get_constructor(Z_OBJ_P(this));
+
+        if ((get_executed_scope() != dispatch->clazz) || constructor) {
+            Z_DELREF_P(this);
+        }
+#endif
     }
 
     Z_DELREF(closure);