diff --git a/CHANGELOG.md b/CHANGELOG.md index bfe47c7..8b215c0 100644 --- a/CHANGELOG.md +++ b/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] diff --git a/src/FFI.php b/src/FFI.php index 6db84ff..f7a8277 100644 --- a/src/FFI.php +++ b/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(<< 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 = []; diff --git a/src/VipsObject.php b/src/VipsObject.php index 05abdc6..74bb83b 100644 --- a/src/VipsObject.php +++ b/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 diff --git a/src/VipsOperation.php b/src/VipsOperation.php index b1eb86e..cf212af 100644 --- a/src/VipsOperation.php +++ b/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,7 +311,7 @@ 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 @@ -317,14 +319,14 @@ public static function callBase( /* 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);