libvips / php-vips

php binding for libvips
MIT License
615 stars 25 forks source link

Freezes/crashes when running in PHP-FPM, probably due to repeated initialization #142

Closed AssortedPelican closed 2 years ago

AssortedPelican commented 2 years ago

We're trying to use the latest version from Packagist (2.0.1) on Debian 11 with PHP 7.4. Unfortunately, we ran into some issues where the FPM processes would freeze or crash when calling into libvips.

A simple test script calling \Jcupitt\Vips\Image::newFromArray() is sufficient to reproduce the issue. It works fine when running the script multiple times from the command line, but when running in PHP-FPM, it only works the first time, and subsequent requests will freeze or crash the PHP process.

The issue appears to have something to do with libvips and the related GLib types being initialized multiple times. On the CLI, a fresh PHP process is created for each run of the script, but PHP-FPM uses long running processes, and a subsequent request may cause libvips to be initialized again.

We have a repository containing a docker-compose setup which may be used to reproduce this bug. Simply running docker-compose up in the cloned repository, then navigating to http://localhost:8080/ should display a var_dump() of an Image object. Refreshing the page causes the FPM process to hang, eventually resulting in a HTTP 504 Gateway Timeout error. On the command line, the following errors appear:

php_1    | (.:7): GLib-GObject-WARNING **: 11:47:53.920: cannot register existing type 'VipsObject'
php_1    | (.:7): GLib-CRITICAL **: 11:47:53.920: g_once_init_leave: assertion 'result != 0' failed
php_1    | (.:7): GLib-GObject-CRITICAL **: 11:47:53.920: g_type_register_static: assertion 'parent_type > 0' failed
php_1    | (.:7): GLib-CRITICAL **: 11:47:53.920: g_once_init_leave: assertion 'result != 0' failed
lucasnetau commented 2 years ago

Related to #139

kleisauke commented 2 years ago

Thanks for the reproduction. It looks like building libvips with -Wl,-z,nodelete fixes this, see for more info: https://github.com/libvips/php-vips-ext/pull/44

Here's an adapted Dockerfile that seems to work for me:

Dockerfile ```Dockerfile FROM php:7.4-fpm RUN \ apt-get update && \ apt-get install -y \ git \ glib-2.0-dev \ libexif-dev \ libexpat-dev \ libffi-dev \ libjpeg-dev \ liblcms2-dev \ libpng-dev \ libtiff-dev \ meson \ unzip \ wget RUN \ git clone -b master --single-branch https://github.com/libvips/libvips.git vips-master && \ cd vips-master && \ LDFLAGS='-Wl,-z,nodelete' meson setup build --prefix=/usr --buildtype=release -Ddeprecated=false -Dintrospection=false && \ meson compile -C build && \ meson install -C build RUN \ docker-php-ext-install ffi && \ wget 'https://getcomposer.org/download/2.3.4/composer.phar' -O /usr/local/bin/composer && \ chmod +x /usr/local/bin/composer COPY composer.json composer.lock ./ RUN composer install && \ echo "ffi.enable = true" >> "/usr/local/etc/php/conf.d/ffi.ini" && \ sed -i -e 's/pm.max_children.*/pm.max_children = 1/' \ -e 's/pm.start_servers.*/pm.start_servers = 1/' \ -e 's/pm.min_spare_servers.*/pm.min_spare_servers = 1/' \ -e 's/pm.max_spare_servers.*/pm.max_spare_servers = 1/' /usr/local/etc/php-fpm.d/www.conf \ ```

@jcupitt Should we link libvips by default with -Wl,-z,nodelete, if supported? I could open a PR for that.

jcupitt commented 2 years ago

Oh yes, I'd forgotten about nodelete.

Sure, let's do it!

kleisauke commented 2 years ago

I just opened https://github.com/libvips/php-vips/pull/151 to fix var_dump() of instances (which was broken on v2.0.3), and https://github.com/libvips/libvips/pull/2934 to prevent unloading.

jcupitt commented 2 years ago

Well done for finding and fixing these issues Kleis!

@AssortedPelican, @lucasnetau would you be able to test again?

lucasnetau commented 2 years ago

@jcupitt Testing now, so far no errors. Will stress a bit further.

kleisauke commented 2 years ago

Great! Here's another Dockerfile using https://github.com/kleisauke/elf-set-nodelete to set RTLD_NODELETE on ELF files. This could be an alternative if you couldn't compile libvips yourself. However, patching ELF files could be a bit dangerous, so use it at your own risk.

Dockerfile ```Dockerfile FROM php:7.4-fpm RUN \ apt-get update && \ apt-get install -y --no-install-recommends \ git \ libffi-dev \ libvips \ meson \ unzip \ wget RUN \ git clone -b main --single-branch https://github.com/kleisauke/elf-set-nodelete.git elf-set-nodelete && \ cd elf-set-nodelete && \ meson setup build --prefix=/usr --buildtype=release && \ meson compile -C build && \ meson install -C build && \ elf-set-nodelete /usr/lib/x86_64-linux-gnu/libvips.so.42 RUN \ docker-php-ext-install ffi && \ wget 'https://getcomposer.org/download/2.3.10/composer.phar' -O /usr/local/bin/composer && \ chmod +x /usr/local/bin/composer COPY composer.json composer.lock ./ RUN \ composer install && \ echo "ffi.enable = true" >> /usr/local/etc/php/conf.d/ffi.ini && \ sed -i -e 's/pm.max_children.*/pm.max_children = 1/' \ -e 's/pm.start_servers.*/pm.start_servers = 1/' \ -e 's/pm.min_spare_servers.*/pm.min_spare_servers = 1/' \ -e 's/pm.max_spare_servers.*/pm.max_spare_servers = 1/' /usr/local/etc/php-fpm.d/www.conf ```
lucasnetau commented 2 years ago

Have not been able to trigger the GLib errors previously seen under repeated stress testing. Looks great. Will this land in the 8.13 release?

jcupitt commented 2 years ago

Great! Yes, this is in 8.13.

kleisauke commented 2 years ago

libvips 8.13 was released, see: https://github.com/libvips/libvips/releases/tag/v8.13.0.