From 20ae6b8af784889d4f9d4e1ffa77d34db219fa74 Mon Sep 17 00:00:00 2001 From: Lovell Fuller Date: Fri, 19 Sep 2014 09:18:58 +0100 Subject: [PATCH 1/2] Prevent non-critical race condition in op cache First thread to add an operation 'wins' --- libvips/iofuncs/cache.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libvips/iofuncs/cache.c b/libvips/iofuncs/cache.c index 15308214..5371720d 100644 --- a/libvips/iofuncs/cache.c +++ b/libvips/iofuncs/cache.c @@ -599,12 +599,12 @@ vips_cache_ref( VipsOperation *operation ) static void vips_cache_insert( VipsOperation *operation ) { - VipsOperationCacheEntry *entry = g_new( VipsOperationCacheEntry, 1 ); - /* It must not be in cache. */ - g_assert( !g_hash_table_lookup( vips_cache_table, operation ) ); + if( g_hash_table_lookup( vips_cache_table, operation ) ) + return; + VipsOperationCacheEntry *entry = g_new( VipsOperationCacheEntry, 1 ); entry->operation = operation; entry->time = 0; entry->invalidate_id = 0; From 862d7f03ea375012614279559b1e2e10f53bf3ea Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 19 Sep 2014 11:38:16 +0100 Subject: [PATCH 2/2] tiny cache clean-up --- ChangeLog | 1 + libvips/iofuncs/cache.c | 45 ++++++++++++++++++++++++----------------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4cb84f82..b789165f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,7 @@ - vipsthumbnail retries with specified input profile if embedded profile is broken - add @profile option to pngsave, matching tiff and jpeg +- fix a race in the operation cache [Lovell] 8/9/14 started 7.40.8 - fix configure on rhel6 [Lovell] diff --git a/libvips/iofuncs/cache.c b/libvips/iofuncs/cache.c index 5371720d..89ab91d9 100644 --- a/libvips/iofuncs/cache.c +++ b/libvips/iofuncs/cache.c @@ -599,12 +599,9 @@ vips_cache_ref( VipsOperation *operation ) static void vips_cache_insert( VipsOperation *operation ) { - /* It must not be in cache. - */ - if( g_hash_table_lookup( vips_cache_table, operation ) ) - return; - VipsOperationCacheEntry *entry = g_new( VipsOperationCacheEntry, 1 ); + + entry = g_new( VipsOperationCacheEntry, 1 ); entry->operation = operation; entry->time = 0; entry->invalidate_id = 0; @@ -759,28 +756,38 @@ vips_cache_operation_buildp( VipsOperation **operation ) *operation = hit->operation; } + /* We have to unlock between search and add so that more than one + * _build() can run at once. + */ g_mutex_unlock( vips_cache_lock ); if( !hit ) { if( vips_object_build( VIPS_OBJECT( *operation ) ) ) return( -1 ); - /* Has to be after _build() so we can see output args. - */ - if( vips__cache_trace ) { - if( vips_operation_get_flags( *operation ) & - VIPS_OPERATION_NOCACHE ) - printf( "vips cache : " ); - else - printf( "vips cache+: " ); - vips_object_print_summary( VIPS_OBJECT( *operation ) ); - } - g_mutex_lock( vips_cache_lock ); - if( !(vips_operation_get_flags( *operation ) & - VIPS_OPERATION_NOCACHE) ) - vips_cache_insert( *operation ); + /* If two threads call the same operation at the same time, + * we can get multiple adds. Let the first one win. See + * https://github.com/jcupitt/libvips/pull/181 + */ + if( !g_hash_table_lookup( vips_cache_table, operation ) ) { + /* Has to be after _build() so we can see output args. + */ + if( vips__cache_trace ) { + if( vips_operation_get_flags( *operation ) & + VIPS_OPERATION_NOCACHE ) + printf( "vips cache : " ); + else + printf( "vips cache+: " ); + vips_object_print_summary( + VIPS_OBJECT( *operation ) ); + } + + if( !(vips_operation_get_flags( *operation ) & + VIPS_OPERATION_NOCACHE) ) + vips_cache_insert( *operation ); + } g_mutex_unlock( vips_cache_lock ); }