diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cb109f7..2850080 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,11 +48,13 @@ jobs: # The JIT rewrites the executor internals z-engine hooks into, so it # stays off; FFI must be enabled for CLI scripts, not only preloading. + # OPcache itself has to be loaded: every .phpt pins opcache.enable_cli + # explicitly and one of them requires it to be switched on. - name: Install PHP uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-version }} - extensions: ffi + extensions: ffi, opcache ini-values: memory_limit=-1, ffi.enable=1, opcache.jit=off coverage: none tools: composer:v2 diff --git a/README.md b/README.md index 27c1193..e10cb57 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,26 @@ ImmutableHandler::install(); Probably, `Z-Engine` will provide an automatic self-registration later, but for now it's ok to perform initialization manually. +### Installation is verified, not assumed + +`ImmutableHandler::install()` does not just install the engine handlers, it proves on a private throwaway class that a +property write is really intercepted afterwards. If it is not, `install()` throws a `RuntimeException` naming the cause +and the workaround instead of returning quietly. An environment where immutability is not enforced now fails loudly at +start-up rather than letting every write outside a constructor succeed in silence. + +Two things can make the handlers unreachable: + +* **Classes linked before `install()` ran.** The library hooks classes as they start to implement `ImmutableInterface`, + so a class that was already linked — most notably anything pulled in through OPcache preloading (`opcache.preload`) — + is never seen. Call `install()` as early as possible, before any immutable class is loaded. +* **OPcache** (`opcache.enable` / `opcache.enable_cli`). With OPcache active, the engine hands the + "interface gets implemented" callback a different `zend_class_entry` than the one it later uses at runtime, so the + property handlers used to land on a structure no object ever reads — + [lisachenko/z-engine#238](https://github.com/lisachenko/z-engine/issues/238). This library works around it by + installing the property handlers from its own `create_object` handler, which does receive the runtime class entry, so + immutability is enforced with OPcache enabled as well. Should a future engine defeat that too, the self-check above + turns it into a start-up error; disabling OPcache for the process remains the fallback. + Applying immutability -------- In order to make your object immutable, you just need to implement the `ImmutableInterface` interface marker in your diff --git a/src/EnforcementProbe.php b/src/EnforcementProbe.php new file mode 100644 index 0000000..be3b370 --- /dev/null +++ b/src/EnforcementProbe.php @@ -0,0 +1,27 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + */ +declare(strict_types=1); + +namespace Immutable; + +/** + * Throwaway class used by ImmutableHandler::install() to verify that the property handlers + * it installs are really reached by the engine. + * + * It must not be referenced anywhere else: the self-check is only meaningful as long as this + * class is linked *after* the "interface gets implemented" handler has been installed, which + * is exactly the condition every user class has to satisfy as well. + * + * @internal + */ +final class EnforcementProbe implements ImmutableInterface +{ + public int $probe = 0; +} diff --git a/src/ImmutableHandler.php b/src/ImmutableHandler.php index 3f02e6b..e71162a 100644 --- a/src/ImmutableHandler.php +++ b/src/ImmutableHandler.php @@ -13,11 +13,13 @@ use Closure; use ReflectionMethod; +use RuntimeException; +use ZEngine\ClassExtension\Hook\CreateObjectHook; use ZEngine\ClassExtension\Hook\GetPropertyPointerHook; use ZEngine\ClassExtension\Hook\InterfaceGetsImplementedHook; use ZEngine\ClassExtension\Hook\UnsetPropertyHook; use ZEngine\ClassExtension\Hook\WritePropertyHook; -use ZEngine\ClassExtension\ObjectCreateTrait; +use ZEngine\ClassExtension\ObjectCreateInterface; use ZEngine\ClassExtension\ObjectGetPropertyPointerInterface; use ZEngine\ClassExtension\ObjectUnsetPropertyInterface; use ZEngine\ClassExtension\ObjectWritePropertyInterface; @@ -28,38 +30,80 @@ * This ImmutableHandler controls the behaviour of */ final class ImmutableHandler implements + ObjectCreateInterface, ObjectWritePropertyInterface, ObjectGetPropertyPointerInterface, ObjectUnsetPropertyInterface { + /** + * Addresses of the zend_class_entry structures that already carry the property handlers + * + * The object handlers table z-engine installs into is keyed by the address of the class + * entry, so the bookkeeping here has to use the very same key. + * + * @var array + */ + private static array $preparedClassEntries = []; + + /** + * Set while install() runs its enforcement self-check + */ + private static bool $isProbingEnforcement = false; + + /** + * Set by __fieldWrite() when the self-check write actually reached the handler + */ + private static bool $wasProbeWriteIntercepted = false; + public static function install(): void { $handler = Closure::fromCallable([self::class, '__interfaceImplemented']); $interface = new ReflectionClass(ImmutableInterface::class); $interface->setInterfaceGetsImplementedHandler($handler); + + self::verifyEnforcementIsActive(); } public static function __interfaceImplemented(InterfaceGetsImplementedHook $hook): int { - $objectCreateHandler = (new ReflectionMethod(ObjectCreateTrait::class, '__init'))->getClosure(); - $objectFieldWriteHandler = (new ReflectionMethod(self::class, '__fieldWrite'))->getClosure(); - $objectFieldPointerHandler = (new ReflectionMethod(self::class, '__fieldPointer'))->getClosure(); - $objectFieldUnsetHandler = (new ReflectionMethod(self::class, '__fieldUnset'))->getClosure(); + $objectCreateHandler = Closure::fromCallable([self::class, '__init']); - $implementor = $hook->getClass(); - $implementor->setCreateObjectHandler($objectCreateHandler); - $implementor->setWritePropertyHandler($objectFieldWriteHandler); - $implementor->setGetPropertyPointerHandler($objectFieldPointerHandler); - $implementor->setUnsetPropertyHandler($objectFieldUnsetHandler); + // Only the create_object handler is installed here, because it lives in the class entry + // itself and therefore survives the OPcache round-trip through shared memory. The + // property handlers live in a separate object handlers table that z-engine keys by the + // address of the class entry, and with OPcache enabled the class entry seen here is not + // the one the engine uses at runtime - so they are installed lazily from __init(), which + // does receive the runtime class entry. See lisachenko/z-engine#238. + $hook->getClass()->setCreateObjectHandler($objectCreateHandler); return Core::SUCCESS; } + /** + * Performs low-level initialization of object during new instances creation + * + * @inheritDoc + */ + public static function __init(CreateObjectHook $hook): object + { + // Must happen before proceed(): the object being created picks up the object handlers + // table for its class entry, and that table is what the property handlers are written to. + self::prepareClassEntry($hook->getClassType()); + + return $hook->proceed(); + } + /** * @inheritDoc */ public static function __fieldWrite(WritePropertyHook $hook) { + if (self::$isProbingEnforcement) { + self::$wasProbeWriteIntercepted = true; + + return $hook->getValue(); + } + $trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS|DEBUG_BACKTRACE_PROVIDE_OBJECT, 3); $frame = $trace[2] ?? []; if (!isset($frame['class'])) { @@ -92,4 +136,69 @@ public static function __fieldUnset(UnsetPropertyHook $hook): void { throw new \LogicException("Unset of immutable field is restricted"); } + + /** + * Installs the property handlers for the runtime class entry of an implementor + * + * @param object $classEntry Raw zend_class_entry pointer received by the create_object handler + */ + private static function prepareClassEntry(object $classEntry): void + { + $classEntryAddress = Core::addressOf($classEntry); + if (isset(self::$preparedClassEntries[$classEntryAddress])) { + return; + } + // Marked as prepared up-front, so that any object creation happening while the handlers + // are being installed can not re-enter this method for the same class entry. + self::$preparedClassEntries[$classEntryAddress] = true; + + $objectFieldWriteHandler = Closure::fromCallable([self::class, '__fieldWrite']); + $objectFieldPointerHandler = Closure::fromCallable([self::class, '__fieldPointer']); + $objectFieldUnsetHandler = Closure::fromCallable([self::class, '__fieldUnset']); + + // Deliberately not guarded by a try/catch: swallowing a failure here would restore the + // silent no-op this whole mechanism exists to prevent. install() has already proven on a + // throwaway class that this code path works in the current environment. + $implementor = ReflectionClass::fromCData($classEntry); + $implementor->setWritePropertyHandler($objectFieldWriteHandler); + $implementor->setGetPropertyPointerHandler($objectFieldPointerHandler); + $implementor->setUnsetPropertyHandler($objectFieldUnsetHandler); + } + + /** + * Proves on a throwaway class that a property write is really intercepted + * + * This is an empirical check on purpose: an environment can defeat the handlers in more ways + * than an ini setting can describe, and the only answer that matters is whether a write to an + * object of a class that implements ImmutableInterface reaches the handler or not. + * + * The probe records a flag instead of throwing, because the handler runs inside an FFI + * callback and an exception crossing that boundary is an uncatchable fatal error. + */ + private static function verifyEnforcementIsActive(): void + { + self::$wasProbeWriteIntercepted = false; + self::$isProbingEnforcement = true; + try { + // The probe class is linked here, i.e. after the handler above has been installed + $probe = new EnforcementProbe(); + $probe->probe = 1; + } finally { + self::$isProbingEnforcement = false; + } + + if (self::$wasProbeWriteIntercepted) { + return; + } + + throw new RuntimeException( + 'Immutability can not be enforced in this environment: a write to a property of a class implementing ' + . ImmutableInterface::class . ' is not intercepted, so writes outside of a constructor would silently ' + . 'succeed instead of raising an error. Check that ImmutableHandler::install() runs before any class ' + . 'implementing ' . ImmutableInterface::class . ' is linked - OPcache preloading links classes ahead of ' + . 'it and those are never seen by the handler. If that is already the case, the engine hooks are not ' + . 'taking effect at all (see https://github.com/lisachenko/z-engine/issues/238): disable OPcache for this ' + . 'process with opcache.enable=0, or opcache.enable_cli=0 on CLI.', + ); + } } diff --git a/tests/Functional/testBoundClosureSetPropertyThrowsAnException.phpt b/tests/Functional/testBoundClosureSetPropertyThrowsAnException.phpt index 72057e0..a0659b7 100644 --- a/tests/Functional/testBoundClosureSetPropertyThrowsAnException.phpt +++ b/tests/Functional/testBoundClosureSetPropertyThrowsAnException.phpt @@ -1,5 +1,9 @@ --TEST-- Updating property of immutable object via Bound closure throws an error +--INI-- +ffi.enable=1 +opcache.jit=off +opcache.enable_cli=0 --FILE-- 200]); +$object->publicProperty = 300; +?> +--EXPECTREGEX-- +OPCACHE ON[\s\S]*Immutable object could be modified only in constructor or static methods diff --git a/tests/Functional/testInstallThrowsWhenEnforcementIsNotActive.phpt b/tests/Functional/testInstallThrowsWhenEnforcementIsNotActive.phpt new file mode 100644 index 0000000..03cf955 --- /dev/null +++ b/tests/Functional/testInstallThrowsWhenEnforcementIsNotActive.phpt @@ -0,0 +1,38 @@ +--TEST-- +Installation reports loudly when the installed handlers would not be reached +--INI-- +ffi.enable=1 +opcache.jit=off +opcache.enable_cli=0 +--FILE-- +getMessage(), 'Immutability can not be enforced') ? 'REASON OK' : 'REASON NO', PHP_EOL; + echo str_contains($exception->getMessage(), 'opcache.enable_cli=0') ? 'WORKAROUND OK' : 'WORKAROUND NO', PHP_EOL; +} +?> +--EXPECT-- +RuntimeException +REASON OK +WORKAROUND OK diff --git a/tests/Functional/testPropertyUnsetThrowsAnException.phpt b/tests/Functional/testPropertyUnsetThrowsAnException.phpt index d26d2f2..a6a5481 100644 --- a/tests/Functional/testPropertyUnsetThrowsAnException.phpt +++ b/tests/Functional/testPropertyUnsetThrowsAnException.phpt @@ -1,5 +1,9 @@ --TEST-- Writing property of immutable object throws an error +--INI-- +ffi.enable=1 +opcache.jit=off +opcache.enable_cli=0 --FILE--