Skip to content
This repository has been archived by the owner on Jan 5, 2022. It is now read-only.

Mediator::process can return null, direct call throw fatal error #37

Open
cturbelin opened this issue Apr 16, 2014 · 5 comments
Open

Mediator::process can return null, direct call throw fatal error #37

cturbelin opened this issue Apr 16, 2014 · 5 comments

Comments

@cturbelin
Copy link

Mediator::process() can return null and direct call made at line 724

$this->process($call->pid)->kill();

can raise a fatal error

@shaneharter
Copy link
Owner

Good find. If you'd care to send a PR I'd be happy to merge, otherwise I'll patch this up this weekend.

@cturbelin
Copy link
Author

Hi !
Actually, I think that's because one entry of the properties $calls of the Mediator object is null. I don't know exactly why and if it is a "normal" case (I have some fail in SysV::put and some dopped calls in the log). If it is, there are several points to correct in the mediator class. I'm investigating.. I corrected this point but the daemon crashed on another point..

@shaneharter
Copy link
Owner

Interesting. If you can provide any more context, debug output, or logging details I'd love to help interpret them. The core functionality of this library hasn't changed materially in over a year and bug reports have significantly slowed as it's become more stable, but there are certainly issues to correct!

I see a possible issue (just looking at the code, not able to run it right this second)... In Core_Worker_Mediator::garbage_collect() we have a foreach() at the top that iterates by-reference into a $call variable, then we don't unset that reference, and then on 917 we assign to the $call variable. So that doesn't necessarily explain why there are null entries, but that would certainly corrupt the call objects in the calls property.

And since you said you have dropped calls -- which is what this code concerns itself with -- the lights are flashing here for me that this might have something to do with it.

Much of this code is a couple years old now, i feel pretty good about there being no big systemic issues, but small things like this can get you all the time. I'll get a fix for this issue up this weekend, but I'd love it if you could make the change and see if that improves things for you. Just add an unset($call) on line 896 in Mediator.php

@cturbelin
Copy link
Author

I tried to protect all calls and logged the mediator calls in some points. It seems that the problem occurs only in garbage_collector method as you said.
I'm trying your proposal at the line 896 with a fresh Mediator.php file (without my ugly patch). I'll send you the result back in few hours !

@cturbelin
Copy link
Author

Hi, there is still a problem...
I dont have $calls entry with null value any more (at least, when the garbage_collector method is called - where I dump this array).
But, my server died at line 724 of the Mediator.php, because of a method call on a non object return of the process() method. So, as this method could return a null value (as well as its internal call to the ProcesseManager instance), I think these calls should be protected to avoid a fatal error.
I have some dropped calls & some SysV errors (shm_get_var() cannot get some keys). I'm exploring this issue. I'm trying to correct (or protect) some points in the Mediator class in order to see if it could correct this problem (I also try to adjust my workers' timeout).
On the starting of the daemon I always have an error : "PHP Warning: msg_send(): msgsnd failed: Identifier removed in vendor/php-daemon/Core/Worker/Via/SysV.php on line 208 pid 15289". I did not have this warning with the previous version (2.0).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants