lisachenko / z-engine

:zap: PHP Engine Direct API
MIT License
448 stars 22 forks source link

[Feature] New hook API #31

Closed lisachenko closed 4 years ago

lisachenko commented 4 years ago

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 commented 4 years ago

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.