From 9c30242745d2c93c5207bc81e248d72321102c87 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Wed, 18 Dec 2013 12:50:22 +0000 Subject: [PATCH] fix erroneous leak report from vipsprofile --- TODO | 2 +- libvips/iofuncs/buffer.c | 11 ++++++++-- libvips/iofuncs/gate.c | 41 +++++++++++++++++++++++++++++------- libvips/iofuncs/init.c | 2 +- libvips/iofuncs/threadpool.c | 3 +++ 5 files changed, 47 insertions(+), 12 deletions(-) diff --git a/TODO b/TODO index 4ba86623..a47eeeea 100644 --- a/TODO +++ b/TODO @@ -1,7 +1,7 @@ - vipsthumbnail could have a --zoom option, does a crop to ensure output is always exactly the specified --size -- vipsprofile reports a leak, strangely + or --expand? - vipsprofile needs a man page for Debian, I guess diff --git a/libvips/iofuncs/buffer.c b/libvips/iofuncs/buffer.c index 7e2a9863..9da8ebfa 100644 --- a/libvips/iofuncs/buffer.c +++ b/libvips/iofuncs/buffer.c @@ -535,6 +535,13 @@ vips_buffer_print( VipsBuffer *buffer ) printf( "bsize = %zd\n", buffer->bsize ); } +static void +vips__buffer_init_cb( VipsBufferThread *buffer_thread ) +{ + vips_error_exit( "vips__buffer_shutdown() not called for thread %p", + g_thread_self() ); +} + /* Init the buffer cache system. */ void @@ -542,13 +549,13 @@ vips__buffer_init( void ) { #ifdef HAVE_PRIVATE_INIT static GPrivate private = - G_PRIVATE_INIT( (GDestroyNotify) buffer_thread_free ); + G_PRIVATE_INIT( (GDestroyNotify) vips__buffer_init_cb ); buffer_thread_key = &private; #else if( !buffer_thread_key ) buffer_thread_key = g_private_new( - (GDestroyNotify) buffer_thread_free ); + (GDestroyNotify) vips__buffer_init_cb ); #endif if( buffer_cache_max_reserve < 1 ) diff --git a/libvips/iofuncs/gate.c b/libvips/iofuncs/gate.c index d8f07f2f..58c60168 100644 --- a/libvips/iofuncs/gate.c +++ b/libvips/iofuncs/gate.c @@ -30,6 +30,10 @@ */ +/* Very verbose. +#define VIPS_DEBUG_RED + */ + /* #define VIPS_DEBUG */ @@ -124,8 +128,10 @@ vips_thread_profile_save( VipsThreadProfile *profile ) if( !vips__thread_fp ) { vips__thread_fp = vips__file_open_write( "vips-profile.txt", TRUE ); - if( !vips__thread_fp ) + if( !vips__thread_fp ) { + g_mutex_unlock( vips__global_lock ); vips_error_exit( "unable to create profile log" ); + } printf( "recording profile in vips-profile.txt\n" ); } @@ -172,18 +178,32 @@ vips_thread_gate_free( VipsThreadGate *gate ) VIPS_FREE( gate ); } +static void +vips__thread_profile_init_cb( VipsThreadProfile *profile ) +{ + /* Threads (including the main thread) must call + * vips__thread_profile_detach() before exiting. Check that they have. + * + * We can't save automatically, because the shutdown order is + * important. We must free all memory before saving the thread + * profile, for example. + */ + vips_error_exit( "vips__thread_profile_detach() not called " + "for thread %p", g_thread_self() ); +} + static void vips__thread_profile_init( void ) { #ifdef HAVE_PRIVATE_INIT static GPrivate private = - G_PRIVATE_INIT( (GDestroyNotify) vips_thread_profile_free ); + G_PRIVATE_INIT( (GDestroyNotify) vips__thread_profile_init_cb ); vips_thread_profile_key = &private; #else if( !vips_thread_profile_key ) vips_thread_profile_key = g_private_new( - (GDestroyNotify) vips_thread_profile_free ); + (GDestroyNotify) vips__thread_profile_init_cb ); #endif } @@ -239,6 +259,8 @@ vips__thread_profile_detach( void ) { VipsThreadProfile *profile; + VIPS_DEBUG_MSG( "vips__thread_profile_detach:\n" ); + if( (profile = vips_thread_profile_get()) ) { vips_thread_profile_free( profile ); g_private_set( vips_thread_profile_key, NULL ); @@ -274,7 +296,7 @@ vips__thread_gate_start( const char *gate_name ) { VipsThreadProfile *profile; - VIPS_DEBUG_MSG( "vips__thread_gate_start: %s\n", gate_name ); + VIPS_DEBUG_MSG_RED( "vips__thread_gate_start: %s\n", gate_name ); if( (profile = vips_thread_profile_get()) ) { gint64 time = vips_get_time(); @@ -293,7 +315,7 @@ vips__thread_gate_start( const char *gate_name ) gate->start->time[gate->start->i++] = time; - VIPS_DEBUG_MSG( "\t %" G_GINT64_FORMAT "\n", time ); + VIPS_DEBUG_MSG_RED( "\t %" G_GINT64_FORMAT "\n", time ); } } @@ -302,7 +324,7 @@ vips__thread_gate_stop( const char *gate_name ) { VipsThreadProfile *profile; - VIPS_DEBUG_MSG( "vips__thread_gate_stop: %s\n", gate_name ); + VIPS_DEBUG_MSG_RED( "vips__thread_gate_stop: %s\n", gate_name ); if( (profile = vips_thread_profile_get()) ) { gint64 time = vips_get_time(); @@ -321,7 +343,7 @@ vips__thread_gate_stop( const char *gate_name ) gate->stop->time[gate->stop->i++] = time; - VIPS_DEBUG_MSG( "\t %" G_GINT64_FORMAT "\n", time ); + VIPS_DEBUG_MSG_RED( "\t %" G_GINT64_FORMAT "\n", time ); } } @@ -332,7 +354,10 @@ vips__thread_malloc_free( gint64 size ) { VipsThreadProfile *profile; - VIPS_DEBUG_MSG( "vips__thread_malloc_free: %zd\n", size ); + VIPS_DEBUG_MSG_RED( "vips__thread_malloc_free: %zd\n", size ); + + if( !(profile = vips_thread_profile_get()) ) + printf( "argh no block to record free() in!\n" ); if( (profile = vips_thread_profile_get()) ) { gint64 time = vips_get_time(); diff --git a/libvips/iofuncs/init.c b/libvips/iofuncs/init.c index 7de07213..dbdb8065 100644 --- a/libvips/iofuncs/init.c +++ b/libvips/iofuncs/init.c @@ -395,8 +395,8 @@ vips_shutdown( void ) vips__thread_gate_stop( "init: main" ); } - vips__thread_profile_detach(); vips__buffer_shutdown(); + vips__thread_profile_detach(); vips__thread_profile_stop(); /* In dev releases, always show leaks. But not more than once, it's diff --git a/libvips/iofuncs/threadpool.c b/libvips/iofuncs/threadpool.c index 0dc08f75..24ba90c0 100644 --- a/libvips/iofuncs/threadpool.c +++ b/libvips/iofuncs/threadpool.c @@ -174,6 +174,9 @@ vips_thread_run( gpointer data ) result = info->func( info->data ); + vips__buffer_shutdown(); + vips__thread_profile_detach(); + return( result ); }