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

[Feature] New hook API #31

Merged
merged 4 commits into from
Feb 4, 2020
Merged

[Feature] New hook API #31

merged 4 commits into from
Feb 4, 2020

Conversation

lisachenko
Copy link
Owner

This PR refactors a lot of low-level code into more general and reusable Hook classes. Each hook is responsible for handling raw arguments from C callback, convert them to PHP representation, and provide methods to receive these values in user-defined callbacks.

HookInterface is declared as following:

interface HookInterface
{
    /**
     * This method accepts raw C arguments for current hook and performs handling of this callback
     *
     * @param mixed ...$rawArguments
     */
    public function handle(...$rawArguments);

    /**
     * Performs installation of current hook
     */
    public function install(): void;

    /**
     * Checks if original handler is present to call it later with proceed
     */
    public function hasOriginalHandler(): bool;
}

@lisachenko lisachenko added the enhancement New feature or request label Feb 4, 2020
@lisachenko lisachenko added this to the 0.7.0 milestone Feb 4, 2020
@lisachenko lisachenko merged commit 750df6c into master Feb 4, 2020
@lisachenko lisachenko deleted the feature/hook-refactoring branch February 4, 2020 13:27
@lisachenko
Copy link
Owner Author

There is unpleasant side-effect of current implementation. When FFI invokes a C callback via trampoline and this callback is declared as a PHP method in the class, then scope will be changed for that low-level call by PHP itself. After that original operation with private or protected members may fail (for example, writing to private property within constructor body) because of changed scope.

To resolve this issue @nikic suggested to use the EG(fake_scope) to modify the scope before invocation of original handler and restore it back after invocation. This trick works, but requires to prepare all private and protected members of hook itself to be copied into local variables within callback body, otherwise hook won’t have access to them due to changed fake scope.

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

Successfully merging this pull request may close these issues.

1 participant