sergiotarxz/libvips
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

bg render thread quits on shutdown

Browse Source 
fixes a small memleak
This commit is contained in:
John Cupitt 2014-01-29 09:07:58 +00:00
parent 54f19a00e5
commit 9cb152596c
6 changed files with 129 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
ChangeLog
View File

@ -1,5 +1,6 @@
21/1/14 started 7.39.0 21/1/14 started 7.39.0
- auto-decode for (almost) all operations - auto-decode for (almost) all operations
- background render thread cleans up and quits neatly
22/1/14 started 7.38.2 22/1/14 started 7.38.2
- auto RAD decode for affine - auto RAD decode for affine

36
TODO
View File

@ -1,39 +1,3 @@
- leak ... nip2 ~/pics/babe.jpg, ^Q
==4389== 784 bytes in 1 blocks are possibly lost in loss record 14,881 of
15,393
==4389== at 0x4C2A2DB: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4389== by 0x6749221: vips_tracked_malloc (memory.c:289)
==4389== by 0x67587E9: buffer_move (buffer.c:368)
==4389== by 0x675898B: vips_buffer_new (buffer.c:412)
==4389== by 0x6758C11: vips_buffer_unref_ref (buffer.c:519)
==4389== by 0x674F5FB: vips_region_buffer (region.c:602)
==4389== by 0x674FF73: vips_region_fill (region.c:887)
==4389== by 0x67505AD: vips_region_prepare (region.c:1139)
==4389== by 0x67400FB: vips_image_write_gen (image.c:1930)
==4389== by 0x67504CF: vips_region_generate (region.c:1074)
==4389== by 0x6750718: vips_region_prepare_to_generate (region.c:1196)
==4389== by 0x6750A55: vips_region_prepare_to (region.c:1315)
==4389== 784 bytes in 1 blocks are possibly lost in loss record 14,882 of
15,393
==4389== at 0x4C2A2DB: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4389== by 0x6749221: vips_tracked_malloc (memory.c:289)
==4389== by 0x67587E9: buffer_move (buffer.c:368)
==4389== by 0x6758BC6: vips_buffer_unref_ref (buffer.c:508)
==4389== by 0x674F5FB: vips_region_buffer (region.c:602)
==4389== by 0x674FF73: vips_region_fill (region.c:887)
==4389== by 0x67505AD: vips_region_prepare (region.c:1139)
==4389== by 0x67400FB: vips_image_write_gen (image.c:1930)
==4389== by 0x67504CF: vips_region_generate (region.c:1074)
==4389== by 0x6750718: vips_region_prepare_to_generate (region.c:1196)
==4389== by 0x6750A55: vips_region_prepare_to (region.c:1315)
==4389== by 0x6747EC1: render_work (sinkscreen.c:418)
- vips_colourspace() needs an option from_space param,? - vips_colourspace() needs an option from_space param,?

6
libvips/include/vips/private.h
View File

@ -89,8 +89,8 @@ typedef struct {
GThread *thread; /* Just for sanity checking */ GThread *thread; /* Just for sanity checking */
} VipsBufferThread; } VipsBufferThread;
/* Per-image buffer cache. Hash to this from VipsBufferCache. /* Per-image buffer cache. Hash to this from VipsBufferThread::hash.
* We can't store the GSList directly in the hash table, as GHashTable lacks an * We can't store the GSList directly in the hash table as GHashTable lacks an
* update operation and we'd need to _remove() and _insert() on every list * update operation and we'd need to _remove() and _insert() on every list
* operation. * operation.
*/ */
@ -127,6 +127,8 @@ VipsBuffer *vips_buffer_unref_ref( VipsBuffer *buffer,
struct _VipsImage *im, VipsRect *area ); struct _VipsImage *im, VipsRect *area );
void vips_buffer_print( VipsBuffer *buffer ); void vips_buffer_print( VipsBuffer *buffer );
void vips__render_shutdown( void );
/* Sections of region.h that are private to VIPS. /* Sections of region.h that are private to VIPS.
*/ */

89
libvips/iofuncs/buffer.c
View File

@ -50,10 +50,10 @@
*/ */
/* /*
#define DEBUG_CREATE
#define DEBUG_VERBOSE #define DEBUG_VERBOSE
#define DEBUG #define DEBUG_CREATE
*/ */
#define DEBUG
#ifdef HAVE_CONFIG_H #ifdef HAVE_CONFIG_H
#include <config.h> #include <config.h>
@ -70,11 +70,11 @@
#ifdef DEBUG #ifdef DEBUG
/* Track all regions here for debugging. /* Track all regions here for debugging.
*/ */
static GSList *vips__buffers_all = NULL; static GSList *vips__buffer_all = NULL;
#endif /*DEBUG*/ #endif /*DEBUG*/
#ifdef DEBUG_CREATE #ifdef DEBUG_CREATE
static int buffer_cache_n = 0; static GSList *vips__buffer_cache_all = NULL;
#endif /*DEBUG_CREATE*/ #endif /*DEBUG_CREATE*/
/* The maximum numbers of buffers we hold in reserve per image. /* The maximum numbers of buffers we hold in reserve per image.
@ -87,13 +87,19 @@ static GPrivate *buffer_thread_key = NULL;
static void * static void *
vips_buffer_dump( VipsBuffer *buffer, size_t *reserve, size_t *alive ) vips_buffer_dump( VipsBuffer *buffer, size_t *reserve, size_t *alive )
{ {
vips_buffer_print( buffer );
if( buffer->im && if( buffer->im &&
buffer->buf ) { buffer->buf &&
printf( "buffer %p, %gMB\n", buffer->cache ) {
printf( "buffer %p, %.3g MB\n",
buffer, buffer->bsize / (1024 * 1024.0) ); buffer, buffer->bsize / (1024 * 1024.0) );
*alive += buffer->bsize; *alive += buffer->bsize;
} }
else if( !buffer->im ) else
if( buffer->im &&
buffer->buf &&
!buffer->cache )
*reserve += buffer->bsize; *reserve += buffer->bsize;
else else
printf( "buffer craziness!\n" ); printf( "buffer craziness!\n" );
@ -101,18 +107,47 @@ vips_buffer_dump( VipsBuffer *buffer, size_t *reserve, size_t *alive )
return( NULL ); return( NULL );
} }
#ifdef DEBUG_CREATE
static void *
vips_buffer_cache_dump( VipsBufferCache *cache )
{
printf( "VipsBufferCache: %p\n", cache );
printf( "\t%d buffers\n", g_slist_length( cache->buffers ) );
printf( "\tthread %p\n", cache->thread );
printf( "\timage %p\n", cache->im );
printf( "\tbuffer_thread %p\n", cache->buffer_thread );
printf( "\t%d in reserve\n", g_slist_length( cache->reserve ) );
return( NULL );
}
#endif /*DEBUG_CREATE*/
void void
vips_buffer_dump_all( void ) vips_buffer_dump_all( void )
{ {
size_t reserve; if( vips__buffer_all ) {
size_t alive; size_t reserve;
size_t alive;
reserve = 0; printf( "buffers:\n" );
alive = 0;
vips_slist_map2( vips__buffers_all, reserve = 0;
(VipsSListMap2Fn) vips_buffer_dump, &reserve, &alive ); alive = 0;
printf( "%gMB alive\n", alive / (1024 * 1024.0) ); vips_slist_map2( vips__buffer_all,
printf( "%gMB in reserve\n", reserve / (1024 * 1024.0) ); (VipsSListMap2Fn) vips_buffer_dump, &reserve, &alive );
printf( "%.3g MB alive\n", alive / (1024 * 1024.0) );
printf( "%.3g MB in reserve\n", reserve / (1024 * 1024.0) );
}
#ifdef DEBUG_CREATE
if( vips__buffer_cache_all ) {
printf( "buffers: %d buffer cache still alive\n",
g_slist_length( vips__buffer_cache_all ) );
vips_slist_map2( vips__buffer_cache_all,
(VipsSListMap2Fn) vips_buffer_cache_dump, NULL, NULL );
printf( "g_thread_self() == %p\n", g_thread_self() );
}
#endif /*DEBUG_CREATE*/
} }
#endif /*DEBUG*/ #endif /*DEBUG*/
@ -126,8 +161,8 @@ vips_buffer_free( VipsBuffer *buffer )
#ifdef DEBUG #ifdef DEBUG
g_mutex_lock( vips__global_lock ); g_mutex_lock( vips__global_lock );
g_assert( g_slist_find( vips__buffers_all, buffer ) ); g_assert( g_slist_find( vips__buffer_all, buffer ) );
vips__buffers_all = g_slist_remove( vips__buffers_all, buffer ); vips__buffer_all = g_slist_remove( vips__buffer_all, buffer );
g_mutex_unlock( vips__global_lock ); g_mutex_unlock( vips__global_lock );
#endif /*DEBUG*/ #endif /*DEBUG*/
@ -146,11 +181,15 @@ buffer_cache_free( VipsBufferCache *cache )
GSList *p; GSList *p;
#ifdef DEBUG_CREATE #ifdef DEBUG_CREATE
buffer_cache_n -= 1; g_mutex_lock( vips__global_lock );
vips__buffer_cache_all =
g_slist_remove( vips__buffer_cache_all, cache );
g_mutex_unlock( vips__global_lock );
printf( "buffer_cache_free: freeing cache %p on thread %p\n", printf( "buffer_cache_free: freeing cache %p on thread %p\n",
cache, g_thread_self() ); cache, g_thread_self() );
printf( "\t(%d caches left)\n", buffer_cache_n ); printf( "\t(%d caches left)\n",
g_slist_length( vips__buffer_cache_all ) );
#endif /*DEBUG_CREATE*/ #endif /*DEBUG_CREATE*/
/* Need to mark undone so we don't try and take them off this hash on /* Need to mark undone so we don't try and take them off this hash on
@ -187,11 +226,15 @@ buffer_cache_new( VipsBufferThread *buffer_thread, VipsImage *im )
cache->n_reserve = 0; cache->n_reserve = 0;
#ifdef DEBUG_CREATE #ifdef DEBUG_CREATE
buffer_cache_n += 1; g_mutex_lock( vips__global_lock );
vips__buffer_cache_all =
g_slist_prepend( vips__buffer_cache_all, cache );
g_mutex_unlock( vips__global_lock );
printf( "buffer_cache_new: new cache %p for thread %p\n", printf( "buffer_cache_new: new cache %p for thread %p\n",
cache, g_thread_self() ); cache, g_thread_self() );
printf( "\t(%d caches now)\n", buffer_cache_n ); printf( "\t(%d caches now)\n",
g_slist_length( vips__buffer_cache_all ) );
#endif /*DEBUG_CREATE*/ #endif /*DEBUG_CREATE*/
return( cache ); return( cache );
@ -403,8 +446,8 @@ vips_buffer_new( VipsImage *im, VipsRect *area )
#ifdef DEBUG #ifdef DEBUG
g_mutex_lock( vips__global_lock ); g_mutex_lock( vips__global_lock );
vips__buffers_all = vips__buffer_all =
g_slist_prepend( vips__buffers_all, buffer ); g_slist_prepend( vips__buffer_all, buffer );
g_mutex_unlock( vips__global_lock ); g_mutex_unlock( vips__global_lock );
#endif /*DEBUG*/ #endif /*DEBUG*/
} }

6
libvips/iofuncs/init.c
View File

@ -368,6 +368,10 @@ vips_leak( void )
fprintf( stderr, "%s", vips_buf_all( &buf ) ); fprintf( stderr, "%s", vips_buf_all( &buf ) );
vips__type_leak(); vips__type_leak();
#ifdef DEBUG
#endif /*DEBUG*/
vips_buffer_dump_all();
} }
/** /**
@ -418,6 +422,8 @@ vips_shutdown( void )
vips__thread_gate_stop( "init: main" ); vips__thread_gate_stop( "init: main" );
} }
vips__render_shutdown();
vips_thread_shutdown(); vips_thread_shutdown();
vips__thread_profile_stop(); vips__thread_profile_stop();

80
libvips/iofuncs/sinkscreen.c
View File

@ -5,6 +5,8 @@
* 25/11/10 * 25/11/10
* - in synchronous mode, use a single region for input and save huge * - in synchronous mode, use a single region for input and save huge
* mem use * mem use
* 20/1/14
* - bg render thread quits on shutdown
*/ */
/* /*
@ -160,6 +162,10 @@ static GThread *render_thread = NULL;
*/ */
static VipsSemaphore render_dirty_sem; static VipsSemaphore render_dirty_sem;
/* Set this to ask the render thread to quit.
*/
static gboolean render_kill = FALSE;
/* All the renders with dirty tiles. /* All the renders with dirty tiles.
*/ */
static GMutex *render_dirty_lock = NULL; static GMutex *render_dirty_lock = NULL;
@ -476,44 +482,62 @@ render_dirty_put( Render *render )
static void * static void *
render_thread_main( void *client ) render_thread_main( void *client )
{ {
for(;;) { Render *render;
Render *render;
if( (render = render_dirty_get()) ) { while( (render = render_dirty_get()) &&
/* Ignore errors, I'm not sure what we'd do with them !render_kill ) {
* anyway. VIPS_DEBUG_MSG_GREEN( "render_thread_main: "
*/ "threadpool start\n" );
VIPS_DEBUG_MSG_GREEN( "render_thread_main: "
"threadpool start\n" );
render_reschedule = FALSE; render_reschedule = FALSE;
if( vips_threadpool_run( render->in, if( vips_threadpool_run( render->in,
render_thread_state_new, render_thread_state_new,
render_allocate, render_allocate,
render_work, render_work,
NULL, NULL,
render ) ) render ) )
VIPS_DEBUG_MSG_RED( "render_thread_main: " VIPS_DEBUG_MSG_RED( "render_thread_main: "
"threadpool_run failed\n" ); "threadpool_run failed\n" );
VIPS_DEBUG_MSG_GREEN( "render_thread_main: " VIPS_DEBUG_MSG_GREEN( "render_thread_main: "
"threadpool return\n" ); "threadpool return\n" );
/* Add back to the jobs list, if we need to. /* Add back to the jobs list, if we need to.
*/ */
render_dirty_put( render ); render_dirty_put( render );
/* _get() does a ref to make sure we keep the render /* _get() does a ref to make sure we keep the render
* alive during processing ... unref before we loop. * alive during processing ... unref before we loop.
* This can kill off the render. * This can kill off the render.
*/ */
render_unref( render ); render_unref( render );
}
} }
return( NULL ); return( NULL );
} }
void
vips__render_shutdown( void )
{
g_mutex_lock( render_dirty_lock );
if( render_thread ) {
GThread *thread;
thread = render_thread;
render_thread = NULL;
g_mutex_unlock( render_dirty_lock );
render_reschedule = TRUE;
render_kill = TRUE;
vips_semaphore_up( &render_dirty_sem );
(void) g_thread_join( thread );
}
else
g_mutex_unlock( render_dirty_lock );
}
/* Create our set of RenderThread. Assume we're single-threaded here. /* Create our set of RenderThread. Assume we're single-threaded here.
*/ */
static int static int