WIP -- work around php-ffi memory leaks

CHANGELOG.md

@@ -2,6 +2,13 @@
 
 All notable changes to `:vips` will be documented in this file.
 
+## master
+
+- refactor callBase() for maintainability
+- work around a php-ffi memory leak in getPspec() [levmv]
+- work around a php-ffi memory leak in arrayType()
+- better test for disabled ffi
+
 ## 2.1.0 - 2022-10-11
 
 - allow "-" as a name component separator [andrews05]

src/FFI.php

@@ -201,6 +201,13 @@ private static function init(): void
             return;
         }
 
+        // try experimentally binding a bit of stdio ... if this fails, FFI
+        // has probably not been installed or enabled, and php will throw a
+        // useful error message
+        $stdio = \FFI::cdef(<<<EOS
+            int printf(const char *, ...);
+        EOS);
+
         $vips_libname = self::libraryName("libvips", 42);
         if (PHP_OS_FAMILY === "Windows") {
             $glib_libname = self::libraryName("libglib-2.0", 0);
@@ -252,7 +259,7 @@ private static function init(): void
         if ($vips === null) {
             array_shift($libraryPaths);
 
-            $msg = "Unable to find library '$vips_libname'";
+            $msg = "Unable to open library '$vips_libname'";
             if (!empty($libraryPaths)) {
                 $msg .= " in any of ['" . implode("', '", $libraryPaths) . "']";
             }
@@ -541,8 +548,8 @@ private static function init(): void
     unsigned int offset;
 } VipsArgumentClass;
 
-int vips_object_get_argument (VipsObject* object,
-    const char *name, GParamSpec** pspec,
+int vips_object_get_argument (VipsObject* object, const char *name, 
+    GParamSpec** pspec,
     VipsArgumentClass** argument_class,
     VipsArgumentInstance** argument_instance);
 

src/GValue.php

@@ -147,8 +147,7 @@ public function set($value): void
                     $value = [$value];
                 }
                 $n = count($value);
-                $ctype = \FFI::arrayType(\FFI::type("int"), [$n]);
-                $array = \FFI::new($ctype);
+                $array = \FFI::new("int[$n]");
                 for ($i = 0; $i < $n; $i++) {
                     $array[$i] = $value[$i];
                 }
@@ -161,8 +160,7 @@ public function set($value): void
                     $value = [$value];
                 }
                 $n = count($value);
-                $ctype = \FFI::arrayType(\FFI::type("double"), [$n]);
-                $array = \FFI::new($ctype);
+                $array = \FFI::new("double[$n]");
                 for ($i = 0; $i < $n; $i++) {
                     $array[$i] = $value[$i];
                 }
@@ -189,8 +187,7 @@ public function set($value): void
                 # we need to set the blob to a copy of the data that vips_lib
                 # can own and free
                 $n = strlen($value);
-                $ctype = \FFI::arrayType(\FFI::type("char"), [$n]);
-                $memory = \FFI::new($ctype, false, true);
+                $memory = \FFI::new("char[$n]", false, true);
                 for ($i = 0; $i < $n; $i++) {
                     $memory[$i] = $value[$i];
                 }

src/Image.php

@@ -797,8 +797,7 @@ public static function newFromArray(
         $width = count($array[0]);
 
         $n = $width * $height;
-        $ctype = \FFI::arrayType(\FFI::type("double"), [$n]);
-        $a = \FFI::new($ctype, true, true);
+        $a = \FFI::new("double[$n]", true, true);
         for ($y = 0; $y < $height; $y++) {
             for ($x = 0; $x < $width; $x++) {
                 $a[$x + $y * $width] = $array[$y][$x];
@@ -975,8 +974,7 @@ public function writeToBuffer(string $suffix, array $options = []): string
      */
     public function writeToMemory(): string
     {
-        $ctype = \FFI::arrayType(\FFI::type("size_t"), [1]);
-        $p_size = \FFI::new($ctype);
+        $p_size = \FFI::new("size_t[1]");
 
         $pointer = FFI::vips()->
             vips_image_write_to_memory($this->pointer, $p_size);
@@ -1017,8 +1015,7 @@ public function writeToMemory(): string
      */
     public function writeToArray(): array
     {
-        $ctype = \FFI::arrayType(\FFI::type("size_t"), [1]);
-        $p_size = \FFI::new($ctype);
+        $p_size = \FFI::new("size_t[1]");
 
         $pointer = FFI::vips()->
             vips_image_write_to_memory($this->pointer, $p_size);
@@ -1027,10 +1024,9 @@ public function writeToArray(): array
         }
 
         // wrap pointer up as a C array of the right type
-        $n = $this->width * $this->height * $this->bands;
         $type_name = FFI::ftypes($this->format);
-        $ctype = \FFI::arrayType(\FFI::type($type_name), [$n]);
-        $array = \FFI::cast($ctype, $pointer);
+        $n = $this->width * $this->height * $this->bands;
+        $array = \FFI::cast("{$type_name}[$n]", $pointer);
 
         // copy to PHP memory as a flat array
         $result = [];

src/VipsObject.php

@@ -88,26 +88,39 @@ public function getDescription(): string
     // get the pspec for a property
     // NULL for no such name
     // very slow! avoid if possible
-    // FIXME add a cache for this thing
+    // FIXME add a cache for this thing, see code in pyvips
     public function getPspec(string $name): ?\FFI\CData
     {
         $name = str_replace("-", "_", $name);
-        $pspec = FFI::gobject()->new("GParamSpec*[1]");
-        $argument_class = FFI::vips()->new("VipsArgumentClass*[1]");
-        $argument_instance = FFI::vips()->new("VipsArgumentInstance*[1]");
+        $pspec_array = FFI::gobject()->new("GParamSpec*[1]");
+        $argument_class_array = FFI::vips()->new("VipsArgumentClass*[1]");
+        $argument_instance_array = FFI::vips()->new("VipsArgumentInstance*[1]");
         $result = FFI::vips()->vips_object_get_argument(
             $this->pointer,
             $name,
-            $pspec,
-            $argument_class,
-            $argument_instance
+            $pspec_array,
+            $argument_class_array,
+            $argument_instance_array
         );
 
         if ($result != 0) {
-            return null;
+            $pspec = null;
         } else {
-            return $pspec[0];
+            /* php-ffi seems to leak if we do the obvious $pspec_array[0] to
+             * get the return result ... instead, we must make a new pointer
+             * object and copy the value ourselves
+             *
+             * the returns values from vips_object_get_argument() are static,
+             * so this is safe
+             */
+            $pspec = FFI::gobject()->new("GParamSpec*");
+            $to_pointer = \FFI::addr($pspec);
+            $from_pointer = \FFI::addr($pspec_array);
+            $size = \FFI::sizeof($from_pointer);
+            \FFI::memcpy($to_pointer, $from_pointer, $size);
         }
+
+        return $pspec;
     }
 
     // get the type of a property from a VipsObject

src/VipsOperation.php

@@ -81,8 +81,9 @@ public static function newFromName($name): VipsOperation
         if ($pointer == null) {
             throw new Exception();
         }
+        $operation = new VipsOperation($pointer);
 
-        return new VipsOperation($pointer);
+        return $operation;
     }
 
     public function setMatch($name, $match_image, $value): void
@@ -223,6 +224,7 @@ public static function callBase(
 
         $operation = self::newFromName($operation_name);
         $operation->introspect = self::introspect($operation_name);
+        $introspect = $operation->introspect;
 
         /* the first image argument is the thing we expand constants to
          * match ... look inside tables for images, since we may be passing
@@ -244,11 +246,11 @@ public static function callBase(
          * args, using instance if required, and only check the nargs after
          * this pass.
          */
-        $n_required = count($operation->introspect->required_input);
+        $n_required = count($introspect->required_input);
         $n_supplied = count($arguments);
         $n_used = 0;
-        foreach ($operation->introspect->required_input as $name) {
-            if ($name == $operation->introspect->member_this) {
+        foreach ($introspect->required_input as $name) {
+            if ($name == $introspect->member_this) {
                 if (!$instance) {
                     $operation->unrefOutputs();
                     throw new Exception("instance argument not supplied");
@@ -291,8 +293,8 @@ public static function callBase(
          */
         foreach ($options as $name => $value) {
             $name = str_replace("-", "_", $name);
-            if (!in_array($name, $operation->introspect->optional_input) &&
-                !in_array($name, $operation->introspect->optional_output)) {
+            if (!in_array($name, $introspect->optional_input) &&
+                !in_array($name, $introspect->optional_output)) {
                 $operation->unrefOutputs();
                 throw new Exception("optional argument '$name' does not exist");
             }
@@ -309,22 +311,22 @@ public static function callBase(
             throw new Exception();
         }
         $operation = new VipsOperation($pointer);
-        $operation->introspect = self::introspect($operation_name);
+        $operation->introspect = $introspect;
 
         # TODO .. need to attach input refs to output, see _find_inside in
         # pyvips
 
         /* Fetch required output args (and modified input args).
          */
         $result = [];
-        foreach ($operation->introspect->required_output as $name) {
+        foreach ($introspect->required_output as $name) {
             $result[$name] = $operation->get($name);
         }
 
         /* Any optional output args.
          */
         $option_keys = array_keys($options);
-        foreach ($operation->introspect->optional_output as $name) {
+        foreach ($introspect->optional_output as $name) {
             $name = str_replace("-", "_", $name);
             if (in_array($name, $option_keys)) {
                 $result[$name] = $operation->get($name);