feedback then now we have to exercise the two cases separately.

Author: devnexen

ext/zip/php_zip.c

@@ -607,6 +607,12 @@ static bool php_zipobj_close(ze_zip_object *obj) /* {{{ */
 	}
 
 	obj->za = NULL;
+
+	if (obj->bailout_callback) {
+		obj->bailout_callback = false;
+		zend_bailout();
+	}
+
 	return success;
 }
 /* }}} */
@@ -1049,10 +1055,16 @@ static void php_zip_object_dtor(zend_object *object)
 
 	if (intern->za) {
 		if (zip_close(intern->za) != 0) {
-			php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za));
+			if (!intern->bailout_callback) {
+				php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za));
+			}
 			zip_discard(intern->za);
 		}
 		intern->za = NULL;
+		if (intern->bailout_callback) {
+			intern->bailout_callback = false;
+			zend_bailout();
+		}
 	}
 }
 
@@ -2902,17 +2914,20 @@ PHP_METHOD(ZipArchive, getStream)
 #ifdef HAVE_PROGRESS_CALLBACK
 static void php_zip_progress_callback(zip_t *arch, double state, void *ptr)
 {
-	if (!EG(active)) {
+	ze_zip_object *obj = ptr;
+
+	if (!EG(active) || obj->bailout_callback) {
 		return;
 	}
 
 	zval cb_args[1];
-	ze_zip_object *obj = ptr;
 
 	ZVAL_DOUBLE(&cb_args[0], state);
 
 	zend_try {
 		zend_call_known_fcc(&obj->progress_callback, NULL, 1, cb_args, NULL);
+	} zend_catch {
+		obj->bailout_callback = true;
 	} zend_end_try();
 }
 
@@ -2956,13 +2971,14 @@ static int php_zip_cancel_callback(zip_t *arch, void *ptr)
 	zval cb_retval;
 	ze_zip_object *obj = ptr;
 
-	if (!EG(active)) {
+	if (!EG(active) || obj->bailout_callback) {
 		return 0;
 	}
 
 	zend_try {
 		zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL);
 	} zend_catch {
+		obj->bailout_callback = true;
 		/* Cancel if a bailout occurs to allow cleanup to happen */
 		return -1;
 	} zend_end_try();

ext/zip/php_zip.h

@@ -72,6 +72,7 @@ typedef struct _ze_zip_object {
 	zip_int64_t last_id;
 	int err_zip;
 	int err_sys;
+	bool bailout_callback;
 #ifdef HAVE_PROGRESS_CALLBACK
 	zend_fcall_info_cache progress_callback;
 #endif

ext/zip/tests/gh22176.phpt

@@ -1,59 +1,33 @@
 --TEST--
-GH-22176 (Memory leak when a ZipArchive cancel/progress callback bails out in the shutdown destructor)
+GH-22176 (Memory leak when a ZipArchive cancel callback bails out in the shutdown destructor)
 --EXTENSIONS--
 zip
 --SKIPIF--
 <?php
 if (!method_exists('ZipArchive', 'registerCancelCallback')) die('skip libzip too old');
-if (!method_exists('ZipArchive', 'registerProgressCallback')) die('skip libzip too old');
 ?>
 --FILE--
 <?php
-$dir = __DIR__ . '/';
-
-$cancel = new ZipArchive;
-$cancel->open($dir . 'gh22176_cancel.zip', ZipArchive::CREATE);
-$cancel->registerCancelCallback(function () {
+$zip = new ZipArchive;
+$zip->open(__DIR__ . '/gh22176_cancel.zip', ZipArchive::CREATE);
+$zip->registerCancelCallback(function () {
     throw new \Exception('cancel boom');
 });
-$cancel->addFromString('test', 'test');
-
-$progress = new ZipArchive;
-$progress->open($dir . 'gh22176_progress.zip', ZipArchive::CREATE);
-$progress->registerProgressCallback(0.5, function ($r) {
-    throw new \Exception('progress boom');
-});
-$progress->addFromString('test', 'test');
-
+$zip->addFromString('test', 'test');
 echo "done\n";
-// Both archives are flushed and the objects destroyed during request
-// shutdown; the thrown exceptions bail out through libzip's zip_close()
-// without leaking its internal state.
+// The archive is flushed and the object destroyed during request shutdown;
+// the thrown exception bails out through libzip's zip_close() without leaking
+// its internal state, and the bailout resumes once libzip has unwound.
 ?>
 --CLEAN--
 <?php
 @unlink(__DIR__ . '/gh22176_cancel.zip');
-@unlink(__DIR__ . '/gh22176_progress.zip');
 ?>
 --EXPECTF--
 done
 
-Fatal error: Uncaught Exception: progress boom in %s:%d
-Stack trace:
-#0 [internal function]: {closure:%s:%d}(0.0)
-#1 {main}
-  thrown in %s on line %d
-
-Fatal error: Uncaught Exception: progress boom in %s:%d
-Stack trace:
-#0 [internal function]: {closure:%s:%d}(1.0)
-#1 {main}
-  thrown in %s on line %d
-
 Fatal error: Uncaught Exception: cancel boom in %s:%d
 Stack trace:
 #0 [internal function]: {closure:%s:%d}()
 #1 {main}
   thrown in %s on line %d
-
-Warning: PHP Request Shutdown: Cannot destroy the zip context: %s in Unknown on line 0

ext/zip/tests/gh22176_progress.phpt

@@ -0,0 +1,33 @@
+--TEST--
+GH-22176 (Memory leak when a ZipArchive progress callback bails out in the shutdown destructor)
+--EXTENSIONS--
+zip
+--SKIPIF--
+<?php
+if (!method_exists('ZipArchive', 'registerProgressCallback')) die('skip libzip too old');
+?>
+--FILE--
+<?php
+$zip = new ZipArchive;
+$zip->open(__DIR__ . '/gh22176_progress.zip', ZipArchive::CREATE);
+$zip->registerProgressCallback(0.5, function ($r) {
+    throw new \Exception('progress boom');
+});
+$zip->addFromString('test', 'test');
+echo "done\n";
+// The archive is flushed and the object destroyed during request shutdown;
+// the thrown exception bails out through libzip's zip_close() without leaking
+// its internal state, and the bailout resumes once libzip has unwound.
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__ . '/gh22176_progress.zip');
+?>
+--EXPECTF--
+done
+
+Fatal error: Uncaught Exception: progress boom in %s:%d
+Stack trace:
+#0 [internal function]: {closure:%s:%d}(0.0)
+#1 {main}
+  thrown in %s on line %d