Skip to content

report the real reason when FFI startup fails, and support ffi.enable=preload - #291

Open
wadakatu wants to merge 1 commit into
libvips:masterfrom
wadakatu:report-real-ffi-error
Open

report the real reason when FFI startup fails, and support ffi.enable=preload#291
wadakatu wants to merge 1 commit into
libvips:masterfrom
wadakatu:report-real-ffi-error

Conversation

@wadakatu

Copy link
Copy Markdown

On master, php-vips won't start on a default PHP install:

$ php -r 'var_export(ini_get("ffi.enable"));'
'preload'

$ php -r 'require "vendor/autoload.php"; Jcupitt\Vips\Config::version();'
Fatal error: Uncaught Jcupitt\Vips\Exception: ffi.enable set to 'preload', not 'true'

$ php -r 'FFI::cdef("int puts(const char *);", "libc.so.6"); echo "FFI works\n";'
FFI works

preload is PHP's compiled-in default and php.ini-production ships the
setting commented out, so that's a stock install. FFI works fine there -- the
restriction doesn't apply to the cli SAPI -- and we throw anyway. 2.6.1
started up.

The check can't be right

Under ffi.enable=preload PHP allows an FFI call iff the SAPI is exactly cli,
or the immediate calling function is ZEND_ACC_PRELOADED, or preload
compilation is running (ext/ffi/ffi.c:2996-3011). So the same value means
usable in one process and dead in another, and ZEND_ACC_PRELOADED isn't
exposed to PHP.

PHP already answers this per call, and says why when the answer is no. We were
dropping that answer: libraryLoad() sends every \FFI\Exception to
Utils::debugLog(), which does nothing without a PSR-3 logger, and then we
throw "Make sure that you've installed libvips" whatever the cause was. That's
what #286 hit -- the reporter had to attach a debugger to find FFI API is restricted by "ffi.enable".

So this deletes the check and puts PHP's own message in the exception.

FFI::preload()

It turns out preloading already works: php-vips' \FFI::cdef() calls are
permitted as long as its own src was compiled during opcache.preload. No
header file, no FFI::scope, no ffi.preload. opcache.preload takes one
script and is INI_SYSTEM, so the app owns that file and we can only be
callable from it:

require __DIR__ . '/vendor/autoload.php';
Jcupitt\Vips\FFI::preload();

Doing this in the library rather than in the README keeps the file list
complete -- a half-preloaded php-vips starts up and then throws a raw
\FFI\Exception, which extends \Error and escapes catch (Vips\Exception).
It also warms psr/log up for DebugLogger, and uses scandir(), since
glob() finds nothing inside a phar.

It deliberately doesn't use FFI::load()/FFI_SCOPE: our declarations are
built at runtime from the detected libvips version and the FFI parser has no
preprocessor, so there's no static header to load. If that changes, the header
load belongs in this same method -- FFI::scope() is restricted too, so the
classes need preloading either way.

Verification

php:8.4-cli + libvips42 8.16.1, as www-data, over php -S so the SAPI is
cli-server and the restriction really applies:

ffi.enable SAPI opcache.preload master this PR
unset (preload) cli - throws OK
true cli, cli-server - OK OK
preload cli-server unset throws throws, now says why
preload cli-server FFI::preload() throws OK
0 cli-server unset throws throws, now says why

I put a dozen operations through the preloaded path -- thumbnail, file and
buffer load, metadata, newFromArray, signalConnect progress,
SourceCustom/TargetCustom streaming -- and the results match
ffi.enable=true exactly.

Suite on 8.4.24 and 7.4.33, as root and unprivileged, with ffi.enable=true
and with no ini at all: 82 tests, 2 skipped, OK. parallel-lint and phpcs
clean. CI green on 7.4 through 8.4.

one-liner to reproduce the preload row
docker run --rm -v "$PWD":/app -w /app php:8.4-cli bash -c '
  apt-get update -qq && apt-get install -y -qq --no-install-recommends libvips42 libffi-dev
  docker-php-ext-install -j1 ffi opcache >/dev/null
  curl -sS https://getcomposer.org/installer | php -- --quiet --install-dir=/usr/local/bin --filename=composer
  composer install -q
  printf "%s\n" "<?php" "require \"/app/vendor/autoload.php\";" \
    "Jcupitt\\Vips\\FFI::preload();" > /tmp/preload.php
  printf "%s\n" "<?php" "require \"/app/vendor/autoload.php\";" \
    "echo PHP_SAPI, \" \", ini_get(\"ffi.enable\"), \"\\n\";" \
    "echo Jcupitt\\Vips\\Config::version(), \" \"," \
    "     strlen(Jcupitt\\Vips\\Image::black(10,10)->writeToBuffer(\".png\")), \"\\n\";" > /tmp/i.php
  chown www-data /tmp/preload.php /tmp/i.php
  su -s /bin/sh www-data -c "php -d ffi.enable=preload -d opcache.enable=1 \
      -d opcache.enable_cli=1 -d opcache.preload=/tmp/preload.php \
      -S 127.0.0.1:9000 -t /tmp &
    curl -s --retry 30 --retry-connrefused localhost:9000/i.php"
'

On tests

tests/PreloadTest.php covers preload(). It shells out to PHP_BINARY, since
opcache.preload can only be set at startup.

The ffi.enable handling has no test. It's ZEND_INI_SYSTEM, and the
restriction is disabled outright for the cli SAPI that PHPUnit runs under
(ffi.c:5462), so it needs a second process on another SAPI. Happy to add a
php -S harness if you'd like one -- I left it out to keep the diff small.

libraryLoad() caught every FFI\Exception and routed it to Utils::debugLog(),
which does nothing unless the user has installed a PSR-3 logger. The caller
then threw "Make sure that you've installed libvips", whatever the real
cause was. That is what issue libvips#286 hit: FFI had refused to run, but the
message pointed at libvips.

Keep the last failure message and include it in the exception, so the
engine's own explanation reaches the user.

That also removes the need to guess at ffi.enable. Under ffi.enable=preload
the engine allows an FFI call iff the SAPI is exactly "cli", or the
immediate calling function is ZEND_ACC_PRELOADED, or preload compilation is
running (php-src ext/ffi/ffi.c:2996-3011), so the same value means usable in
one process and dead in another, and ZEND_ACC_PRELOADED is not exposed to
PHP. PHP already answers this per call, and now says why when the answer is
no.

Two consequences:

- php-vips works under ffi.enable=preload when php-vips' src is in
  opcache.preload -- no header file and no FFI::scope needed. FFI::preload()
  does that from the app's preload script, so the file list is ours rather
  than something every user has to get right.
- "preload" is PHP's compiled-in default (ffi.c:5332) and php.ini-production
  ships the setting commented out, so php-vips no longer refuses to start on a
  stock PHP install.
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

Successfully merging this pull request may close these issues.

1 participant