Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Garbage collection stops after exception in SoapClient #10026

Open
abienvenu opened this issue Nov 30, 2022 · 3 comments
Open

Garbage collection stops after exception in SoapClient #10026

abienvenu opened this issue Nov 30, 2022 · 3 comments

Comments

@abienvenu
Copy link

Description

The following code:

<?php

class A
{
	public $b;
}

class B
{
	public $a;
}

function buildCycle()
{
	$a = new A();
	$b = new B();
	$a->b = $b;
	$b->a = $a;
}

function failySoapCall()
{

	try {
		new SoapClient('https://nowhere.inexistent.space/?WSDL');
	}
	catch (Exception) {
		echo "Soap call failed\n";
	}
}

function gc()
{
	gc_collect_cycles();
	echo "GC Runs: " . gc_status()['runs'] . "\n";
}


buildCycle();
gc();
buildCycle();
gc();
buildCycle();
gc();

failySoapCall();

buildCycle();
gc();
buildCycle();
gc();
buildCycle();
gc();

Resulted in this output:

GC Runs: 1
GC Runs: 2
GC Runs: 3
Soap call failed
GC Runs: 3
GC Runs: 3
GC Runs: 3

But I expected this output instead:

GC Runs: 1
GC Runs: 2
GC Runs: 3
Soap call failed
GC Runs: 4
GC Runs: 5
GC Runs: 6

The SOAP exception apparently stops the garbage collector. I experienced this problem with a large, long running process. Its memory usage is intensive but stable. Until it encounters a SOAP exception: its memory usage skyrockets, and gc_status() reports that no more gc run is done.

PHP Version

PHP 8.1.13

Operating System

Alpine 3.16

@cmb69
Copy link
Contributor

cmb69 commented Dec 1, 2022

The problem is Soap's error handling, which calls _zend_bailout() and there:

gc_protect(1);

The fix for the reported issue appears to be trivial:

 ext/soap/soap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ext/soap/soap.c b/ext/soap/soap.c
index de29917cd8..5b02f74ba9 100644
--- a/ext/soap/soap.c
+++ b/ext/soap/soap.c
@@ -124,7 +124,8 @@ static void soap_error_handler(int error_num, zend_string *error_filename, const
 	SOAP_GLOBAL(soap_version) = _old_soap_version;\
 	if (_bailout) {\
 		zend_bailout();\
-	}
+	}\
+	gc_protect(0);
 
 #define FETCH_THIS_SDL(ss) \
 	{ \

However, there are more issues in other places (probably not only in ext/soap).

@nielsdos
Copy link
Member

nielsdos commented Jan 8, 2023

If no one is working on this yet, I can try to check other places too soon-ish, and write a PR.

@cmb69
Copy link
Contributor

cmb69 commented Jan 8, 2023

If no one is working on this yet, I can try to check other places too soon-ish, and write a PR.

Great! I'm not planning to work on this. :)

nielsdos added a commit to nielsdos/php-src that referenced this issue Jan 10, 2023
Co-authored-by: abienvenu <abi@abienvenu.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants