-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Observer: fake closures #6607
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
Observer: fake closures #6607
Conversation
TIL that the |
It looks like we might have access to the original runtime cache if we "unwrap" the fake closure in |
It might be that they never actually ran ... The problem seems to be that the extension is located in |
I've opened #6613 to change the extension name, which should also result in the tests being run. |
They were running in TravisCI but I think exclusively so. I'll be rebasing this now that #6613 is merged (thanks again Nikita!) and start digging into this again later in the week (most likely on Friday). |
I also got into the same problem with skipping fake closures. |
After some testing, I think we may remove ZEND_ACC_FAKE_CLOSURE limitation. |
92b603e
to
da89066
Compare
da89066
to
3fd35de
Compare
@dstogov Thanks for checking this out. I tried to track back why we avoided fake closures when adding the API but couldn't find the source. At any rate, the tests are passing now so I think this should be good to go. :) |
Methods are not observed from closures created via
Closure::fromCallable()
because it creates a fake closure. The observer API explicitly cannot observe fake closures because the observer handlers are stored in the runtime cache of op_arrays.I haven't figured out an obvious solution yet so I thought I'd throw this out there in case anyone has any good ideas. :)
@derickr: @morrisonlevi shared your use case with me and I think this is the root issue as to why
RouterListener->onKernelFinishRequest
isn't being observed. It looks like it's being called after being converted into a fake closure in theEventDispatcher
.