fix a deadlock with --vips-leak

We were usingh a global lock for metadata changes, but some functions
triggered from callbacks in the metadata hash table could also attempt
to acquire the same mutex, leading to deadlock.

This patch gives metadata change it's own lock. Thanks DarthSim.

See https://github.com/libvips/libvips/issues/1542

ChangeLog

@@ -1,3 +1,6 @@
+31/1/19 started 8.9.2
+- fix a deadlock with --vips-leak [DarthSim]
+
 20/6/19 started 8.9.1
 - don't use the new source loaders for new_from_file or new_from_buffer, it
   will break the loader priority system

configure.ac

@@ -2,7 +2,7 @@
 
 # also update the version number in the m4 macros below
 
-AC_INIT([vips], [8.9.1], [vipsip@jiscmail.ac.uk])
+AC_INIT([vips], [8.9.2], [vipsip@jiscmail.ac.uk])
 # required for gobject-introspection
 AC_PREREQ(2.62)
 
@@ -18,7 +18,7 @@ AC_CONFIG_MACRO_DIR([m4])
 # user-visible library versioning
 m4_define([vips_major_version], [8])
 m4_define([vips_minor_version], [9])
-m4_define([vips_micro_version], [1])
+m4_define([vips_micro_version], [2])
 m4_define([vips_version],
           [vips_major_version.vips_minor_version.vips_micro_version])
 
@@ -38,7 +38,7 @@ VIPS_VERSION_STRING=$VIPS_VERSION-`date -u -r ChangeLog`
 #   binary interface changes not backwards compatible?: reset age to 0
 
 LIBRARY_CURRENT=54
-LIBRARY_REVISION=1
+LIBRARY_REVISION=2
 LIBRARY_AGE=12
 
 # patched into include/vips/version.h

libvips/include/vips/private.h

@@ -192,6 +192,8 @@ int vips__view_image( struct _VipsImage *image );
  */
 extern int _vips__argument_id;
 
+void vips__meta_init( void );
+
 #ifdef __cplusplus
 }
 #endif /*__cplusplus*/

libvips/iofuncs/header.c

@@ -36,6 +36,8 @@
  * 	- add vips_image_get_n_pages()
  * 20/6/19
  * 	- add vips_image_get/set_array_int()
+ * 31/1/19
+ * 	- lock for metadata changes
  */
 
 /*
@@ -130,6 +132,11 @@
  * these types, it can be copied between images efficiently.
  */
 
+/* Use in various small places where we need a mutex and it's not worth 
+ * making a private one.
+ */
+static GMutex *vips__meta_lock = NULL;
+
 /* We have to keep the gtype as a string, since we statically init this.
  */
 typedef struct _HeaderField {
@@ -934,10 +941,10 @@ meta_cp( VipsImage *dst, const VipsImage *src )
 		/* We lock with vips_image_set() to stop races in highly-
 		 * threaded applications.
 		 */
-		g_mutex_lock( vips__global_lock );
+		g_mutex_lock( vips__meta_lock );
 		vips_slist_map2( src->meta_traverse,
 			(VipsSListMap2Fn) meta_cp_field, dst, NULL );
-		g_mutex_unlock( vips__global_lock );
+		g_mutex_unlock( vips__meta_lock );
 	}
 
 	return( 0 );
@@ -1025,13 +1032,6 @@ vips_image_set( VipsImage *image, const char *name, GValue *value )
 	g_assert( name );
 	g_assert( value );
 
-        /* If this image is shared, block metadata changes.
-         */
-        if( G_OBJECT( image )->ref_count > 1 ) {
-                g_warning( "can't set metadata \"%s\" on shared image", name );
-                return;
-        }
-
 	meta_init( image );
 
 	/* We lock between modifying metadata and copying metadata between
@@ -1041,9 +1041,9 @@ vips_image_set( VipsImage *image, const char *name, GValue *value )
 	 * metadata copy on another -- this can lead to crashes in
 	 * highly-threaded applications.
 	 */
-	g_mutex_lock( vips__global_lock );
+	g_mutex_lock( vips__meta_lock );
 	(void) meta_new( image, name, value );
-	g_mutex_unlock( vips__global_lock );
+	g_mutex_unlock( vips__meta_lock );
 
 	/* If we're setting an EXIF data block, we need to automatically expand 
 	 * out all the tags. This will set things like xres/yres too.
@@ -1240,14 +1240,6 @@ vips_image_remove( VipsImage *image, const char *name )
 
 	result = FALSE;
 
-        /* If this image is shared, block metadata changes. 
-         */
-        if( G_OBJECT( image )->ref_count > 1 ) {
-                g_warning( "can't remove metadata \"%s\" on shared image", 
-			name );
-                return( result );
-        }
-
 	if( image->meta ) {
 		/* We lock between modifying metadata and copying metadata 
 		 * between images, see meta_cp().
@@ -1256,9 +1248,9 @@ vips_image_remove( VipsImage *image, const char *name )
 		 * racing with metadata copy on another -- this can lead to 
 		 * crashes in highly-threaded applications.
 		 */
-		g_mutex_lock( vips__global_lock );
+		g_mutex_lock( vips__meta_lock );
 		result = g_hash_table_remove( image->meta, name );
-		g_mutex_unlock( vips__global_lock );
+		g_mutex_unlock( vips__meta_lock );
 	}
 
 	return( result );
@@ -2027,3 +2019,12 @@ vips_image_get_history( VipsImage *image )
 
 	return( image->Hist ? image->Hist : "" );
 }
+
+/* Called during vips_init().
+ */
+void
+vips__meta_init( void )
+{
+	if( !vips__meta_lock ) 
+		vips__meta_lock = vips_g_mutex_new();
+}

libvips/iofuncs/init.c

@@ -382,6 +382,7 @@ vips_init( const char *argv0 )
 
 	vips__threadpool_init();
 	vips__buffer_init();
+	vips__meta_init();
 
 	/* This does an unsynchronised static hash table init on first call --
 	 * we have to make sure we do this single-threaded. See: