-
Notifications
You must be signed in to change notification settings - Fork 45
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
GearmanJob::setExceptionCallback() not working #5
Comments
both awesome that we have another clear reproduction and a bummer it's still happening. Thanks for helping out on this @vojta ! I'll take a look soon. |
hey @vojta, just pushed a fix I think takes care of this. Wasn't handling allocation of storing callbacks in zvals correctly, along with some other minor clean up. Give it a shot and lemme know if it works? This might clear up #6 as well (haven't had a change to set that up in a context to test, but fingers crossed). If something's amiss, reopen this. Will leave #6 open while you test |
It calls the exceptionCallback, but after a few dozen successful requests, I get a segfault: Program received signal SIGSEGV, Segmentation fault. |
Ah sounds like poorly allocated memory location. I'll hit it a bunch of times and see if I can replicate it. Isolating it to a zval_dtor helps though, thanks! |
I added #7, might be connected to this one. |
hey @vojta, thanks again for your patience here! I left notes in the commit, but here's the scoop - with this test function, it looks like it's going to call setCompleteCallback (among other callbacks) over and over again unnecessarily. I wasn't taking that into account! So it was just doing a copy over the previous callback without properly deallocating and cleaning up. Hence, memory leaks! This should be fixed for all the callback methods. I do agree, this seems very likely linked to #7 as well. It wasn't deallocating the callback and so it was losing lots of memory until it hit the limit. If #6 was having issues allocating memory as well, that might be tied in too. This is a bug, straight up. It should've handled your code in a cleaner manner. But you might be able to do a little bit less work if you don't set the callback multiple times too! There might be very good reason why you're doing that in your code, so please ignore if that's the case. As always, if you could test those out again, it would be much appreciated! Going to close this out, but should issues persist please open up again. Thanks for helping make the pecl package better! |
hey @wcgallego , I never call setCompleteCallback() multiple times. There must be some another unfixed problem. I leave this issue ( #5 ) closed but the #7 is still repeatable using the latest version. |
My gearman client code is as below: `class FNSGearman {
}` `$gm = new FNSGearman();
|
This is similar to #3
How to reproduce:
gearman_worker.php:
gearman_client.php:
To reproduce, just start the worker:
and run the client:
It runs OK on PHP 5.5 (calls the exceptionCallback):
The text was updated successfully, but these errors were encountered: