fix a memleak

we were always copying images! argh

speed is much better now, and memory use is stable

Author: jcupitt

ChangeLog

@@ -1,5 +1,9 @@
 vips extension changelog
 
+Version 0.1.3 (2016-11-04)
+--------------------------
+	* Fix memleak
+
 Version 0.1.2 (2016-10-24)
 --------------------------
 	* Always dereference REFERENCE zvalues

examples/leaktest.php

@@ -0,0 +1,27 @@
+#!/usr/bin/env php
+<?php
+
+for($i = 0; $i < 2000; $i++) {
+    echo "Loop $i ...\r";
+
+    $x = vips_image_new_from_file($argv[1], ["access" => "sequential"])["out"];
+
+    $width = vips_image_get($x, "width")["out"];
+    $height = vips_image_get($x, "height")["out"];
+
+    $x = vips_call("crop", $x, 100, 100, $width - 200, $height - 200)["out"];
+
+    vips_image_write_to_file($x, $argv[2]);
+
+    $x = NULL;
+}
+
+/*
+ * Local variables:
+ * tab-width: 4
+ * c-basic-offset: 4
+ * End:
+ * vim600: expandtab sw=4 ts=4 fdm=marker
+ * vim<600: expandtab sw=4 ts=4
+ */
+

vips.c

@@ -566,7 +566,8 @@ vips_php_zval_to_gval(VipsImage *match_image, zval *zvalue, GValue *gvalue)
 }
 
 static int
-vips_php_set_value(VipsPhpCall *call, GParamSpec *pspec, zval *zvalue)
+vips_php_set_value(VipsPhpCall *call, 
+	GParamSpec *pspec, VipsArgumentFlags flags, zval *zvalue)
 {
 	const char *name = g_param_spec_get_name(pspec);
 	GType pspec_type = G_PARAM_SPEC_VALUE_TYPE(pspec);
@@ -581,7 +582,8 @@ vips_php_set_value(VipsPhpCall *call, GParamSpec *pspec, zval *zvalue)
 	/* If we are setting a MODIFY VipsArgument with an image, we need to take a
 	 * copy.
 	 */
-	if (g_type_is_a(pspec_type, VIPS_TYPE_IMAGE)) {
+	if (g_type_is_a(pspec_type, VIPS_TYPE_IMAGE) &&
+		(flags & VIPS_ARGUMENT_MODIFY)) {
 		VipsImage *image;
 		VipsImage *memory;
 
@@ -647,7 +649,7 @@ vips_php_set_required_input(VipsObject *object,
 		}
 
 		if (arg &&
-			vips_php_set_value(call, pspec, arg)) {
+			vips_php_set_value(call, pspec, argument_class->flags, arg)) {
 			return call;
 		}
 	}
@@ -686,7 +688,7 @@ vips_php_set_optional_input(VipsPhpCall *call, zval *options)
 		if (!(argument_class->flags & VIPS_ARGUMENT_REQUIRED) &&
 			(argument_class->flags & VIPS_ARGUMENT_INPUT) &&
 			!(argument_class->flags & VIPS_ARGUMENT_DEPRECATED) &&
-			vips_php_set_value(call, pspec, value)) {
+			vips_php_set_value(call, pspec, argument_class->flags, value)) {
 			return -1;
 		}
 	} ZEND_HASH_FOREACH_END();
@@ -891,6 +893,12 @@ vips_php_get_optional_output(VipsPhpCall *call, zval *options,
 			continue;
 		}
 
+		/* value should always be TRUE.
+		 */
+		if (Z_TYPE_P(value) != IS_TRUE) {
+			continue;
+		}
+
 		name = ZSTR_VAL(key);
 		if (vips_object_get_argument(VIPS_OBJECT(call->operation), name,
 			&pspec, &argument_class, &argument_instance)) {
@@ -1046,7 +1054,6 @@ PHP_FUNCTION(vips_call)
 	char *operation_name;
 	size_t operation_name_len;
 	zval *instance;
-	int i;
 
 	VIPS_DEBUG_MSG("vips_call:\n");
 
@@ -1182,7 +1189,7 @@ PHP_FUNCTION(vips_image_new_from_array)
 	int width;
 	int height;
 	VipsImage *mat;
-	int x, y;
+	int x;
 	zval *row;
 
 	VIPS_DEBUG_MSG("vips_image_new_from_array:\n");
@@ -1237,6 +1244,8 @@ PHP_FUNCTION(vips_image_write_to_file)
 	zval *options = NULL;
 	VipsImage *image;
 
+	VIPS_DEBUG_MSG("vips_image_write_to_file:\n");
+
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "rp|a", 
 		&IM, &filename, &filename_len, &options) == FAILURE) {
 		RETURN_LONG(-1);
@@ -1247,6 +1256,8 @@ PHP_FUNCTION(vips_image_write_to_file)
 		RETURN_LONG(-1);
 	}
 
+	VIPS_DEBUG_MSG("\t%p -> %s\n", image, filename);
+
 	if (vips_image_write_to_file(image, filename, NULL)) {
 		RETURN_LONG(-1);
 	}
@@ -1477,7 +1488,6 @@ PHP_FUNCTION(vips_image_remove)
 	char *field_name;
 	size_t field_name_len;
 	VipsImage *image;
-	GType type;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "rs", 
 		&im, &field_name, &field_name_len) == FAILURE) {
@@ -1606,6 +1616,12 @@ PHP_MINIT_FUNCTION(vips)
 	le_gobject = zend_register_list_destructors_ex(php_free_gobject, 
 		NULL, "GObject", module_number);
 
+#ifdef VIPS_DEBUG
+	printf( "php-vips-ext init\n" );
+	printf( "enabling vips leak testing ...\n" );
+	vips_leak_set( TRUE ); 
+#endif /*VIPS_DEBUG*/
+
 	return SUCCESS;
 }
 /* }}} */
@@ -1618,6 +1634,10 @@ PHP_MSHUTDOWN_FUNCTION(vips)
 	UNREGISTER_INI_ENTRIES();
 	*/
 
+#ifdef VIPS_DEBUG
+	printf( "php-vips-ext shutdown\n" );
+#endif /*VIPS_DEBUG*/
+
 	vips_shutdown();
 
 	return SUCCESS;