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

Namespaced functions failing inside returned closure #78

Closed
brentkelly opened this issue Aug 31, 2020 · 5 comments
Closed

Namespaced functions failing inside returned closure #78

brentkelly opened this issue Aug 31, 2020 · 5 comments

Comments

@brentkelly
Copy link

brentkelly commented Aug 31, 2020

Hi guys - found a bit of a curly one.

TLDR; clone this repo & run index.php: https://github.com/brentkelly/opis-closure-bug-demo/

Take the following scenario:

  • You have namespaced functions, and a separate namespaced class in the same namespace.
  • Inside the class, you generate a closure that references the namespaced function (without the full path because you are in the same namespace).
  • Serialize & then unserialize that closure.
  • On executing the closure you get a Call to undefined function fatal error.

Essentially - it loses track that the function call within the closure should be referencing the namespaced function - not the global scope. If you have a global function of the same name - it will call the global function instead of throwing the fatal error - producing potentially even more bizarre behavior :).

If you update the closure to instead reference the full function namespace path, everything then works smoothly.

Because there are a few moving parts, I've put together a quick repo demonstrating the behavior https://github.com/brentkelly/opis-closure-bug-demo/.

  • The class \Services\Foo has methods that generate closures referencing the function \Services\foo() (inside /services/functions.php).
  • index.php creates a closure with a full namespace reference to \Services\foo() and executes it, demonstrating it working fine.
  • it then creates a closure relying the class being in the same \Services namespace, and executes it before & after serializing/unserializing.

The last approch shows it execute fine prior to serializing, but after serialize/unserialize it throws:

PHP Fatal error:  Uncaught Error: Call to undefined function foo() in closure://function($name) {
                        foo("Hello $name!");
                }:3
Stack trace:
#0 /home/brent/code/demos/promise-serialize-basic/index.php(41): Services\Foo::{closure}()
#1 {main}
  thrown in closure://function($name) {
                        foo("Hello $name!");
                } on line 3

Much respect for this project - its awesome.

@brentkelly
Copy link
Author

brentkelly commented Aug 31, 2020

FYI the real world scenario I've encountered this issue is attempting to meld opis/closure with reactphp/promise to get Promises that can be defined in one thread, and then resolved in another later thread.

It essentially plays pretty nicely - aside from this 1 issue. \React\Promise\Promise generates callbacks for resolving that call a namespaced resolve function. Given that I was doing this in a Laravel project, the issue I was encountering was it was attempting to call Laravel's global resolve function after unserializing the promise, rather than \React\Promise\resolve.

Here is a demo of this in action: https://github.com/brentkelly/serialized-promise-demo. You will see from the PR referenced in the README.md the exact lines in src/Promise.php (line 226 being the main culprit) causing the issue.

I was hoping to get them to work around the issue by referencing the full namespaced path on the function call but they're pushing back saying its a bug in opis/closure.

sorinsarca added a commit that referenced this issue Sep 5, 2020
@sorinsarca
Copy link
Member

@brentkelly instead of replacing every function call with the absolute form you could just import it once

namespace My\Ns;

use function My\Ns\my_func; // reference my_func once instead of replacing every function call

$f = function() {
   my_func();
   // ...
   my_func();
};

However a PR was submitted and probably this issue will be fixed in the next version.

@msarca msarca closed this as completed in 70ee7d6 Sep 6, 2020
@brentkelly
Copy link
Author

Great - thanks for the reply @sorinsarca :).

Good suggestion re just using once - although still means I'd have to fork reactphp/promise.

Look forward to the next version.

@sorinsarca
Copy link
Member

@brentkelly this issue is fixed in version 3.5.7 of opis/closure

@brentkelly
Copy link
Author

@sorinsarca Brilliant - thanks!

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

No branches or pull requests

2 participants