diff --git a/ChangeLog b/ChangeLog index 85751c41..08448e20 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ 13/2/14 started 7.38.4 - --sharpen=none option to vipsthumbnail was broken, thanks ferryfax +- more locking on property create and lookup to help very-threaded systems, + thanks Nick 22/1/14 started 7.38.3 - undeprecate VIPS_MASK_IDEAL_HIGHPASS and friends, ruby-vips was using them, diff --git a/libvips/foreign/dzsave.c b/libvips/foreign/dzsave.c index 6ec7b092..7f0ebeb0 100644 --- a/libvips/foreign/dzsave.c +++ b/libvips/foreign/dzsave.c @@ -36,9 +36,6 @@ * - add --angle option * 19/6/13 * - faster --centre logic, thanks Kacey - * 25/6/13 - * - ping classes before starting workers, see comment below, thanks - * Kacey */ /* @@ -1170,37 +1167,6 @@ pyramid_strip( VipsRegion *region, VipsRect *area, void *a ) return( 0 ); } -static void * -vips_class_ping( VipsObjectClass *class, void *dummy ) -{ - return( NULL ); -} - -/* Loop over all classes. This will make sure they are all built. - * - * vips_dzsave() runs a set of pipelines from worker threads, and if the - * operations it uses have not all been used previously, they can run their - * class_init in parallel. - * - * This should be safe but seems not to be for reasons I don't understand. - * For now, just ping all classes from the main thread before we set the - * workers going. - * - * This stops a crash on very many core machines, see - * - * https://github.com/jcupitt/libvips/issues/64 - */ -static void -vips_class_ping_all( void ) -{ - GType base; - - if( !(base = g_type_from_name( "VipsObject" )) ) - return; - vips_class_map_all( base, - (VipsClassMapFn) vips_class_ping, NULL ); -} - static int vips_foreign_save_dz_build( VipsObject *object ) { @@ -1328,10 +1294,6 @@ vips_foreign_save_dz_build( VipsObject *object ) if( pyramid_mkdir( dz ) ) return( -1 ); - /* Build all vips classes, see comment above. - */ - vips_class_ping_all(); - if( vips_sink_disc( save->ready, pyramid_strip, dz ) ) return( -1 ); diff --git a/libvips/iofuncs/object.c b/libvips/iofuncs/object.c index bb93e76f..99837b28 100644 --- a/libvips/iofuncs/object.c +++ b/libvips/iofuncs/object.c @@ -349,7 +349,13 @@ vips_argument_instance_free( VipsArgumentInstance *argument_instance ) VipsArgument * vips__argument_table_lookup( VipsArgumentTable *table, GParamSpec *pspec ) { - return( g_hash_table_lookup( table, pspec ) ); + VipsArgument *argument; + + g_mutex_lock( vips__global_lock ); + argument = (VipsArgument *) g_hash_table_lookup( table, pspec ); + g_mutex_unlock( vips__global_lock ); + + return( argument ); } static void @@ -1400,6 +1406,7 @@ vips_object_class_install_argument( VipsObjectClass *object_class, GParamSpec *pspec, VipsArgumentFlags flags, int priority, guint offset ) { VipsArgumentClass *argument_class = g_new( VipsArgumentClass, 1 ); + GSList *argument_table_traverse; #ifdef DEBUG printf( "vips_object_class_install_argument: %p %s %s\n", @@ -1408,10 +1415,13 @@ vips_object_class_install_argument( VipsObjectClass *object_class, g_param_spec_get_name( pspec ) ); #endif /*DEBUG*/ + /* object_class->argument* is shared, so we must lock. + */ + g_mutex_lock( vips__global_lock ); + /* Must be a new one. */ - g_assert( !vips__argument_table_lookup( object_class->argument_table, - pspec ) ); + g_assert( !g_hash_table_lookup( object_class->argument_table, pspec ) ); /* Mustn't have INPUT and OUTPUT both set. */ @@ -1444,10 +1454,21 @@ vips_object_class_install_argument( VipsObjectClass *object_class, G_TYPE_FROM_CLASS( object_class ); } - object_class->argument_table_traverse = g_slist_prepend( - object_class->argument_table_traverse, argument_class ); - object_class->argument_table_traverse = g_slist_sort( - object_class->argument_table_traverse, traverse_sort ); + /* We read argument_table_traverse without a lock (eg. see + * vips_argument_map()), so we must be very careful updating it. + */ + argument_table_traverse = + g_slist_copy( object_class->argument_table_traverse ); + + argument_table_traverse = g_slist_prepend( + argument_table_traverse, argument_class ); + argument_table_traverse = g_slist_sort( + argument_table_traverse, traverse_sort ); + VIPS_SWAP( GSList *, + argument_table_traverse, + object_class->argument_table_traverse ); + + g_slist_free( argument_table_traverse ); #ifdef DEBUG { @@ -1467,6 +1488,8 @@ vips_object_class_install_argument( VipsObjectClass *object_class, } } #endif /*DEBUG*/ + + g_mutex_unlock( vips__global_lock ); } static void