From b836749b75f0deddb89b3960f66f381763cd18d4 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 16 Aug 2018 15:47:48 +0100 Subject: [PATCH] close jpeg read early The current behaviour (close input handles on unref) works for languages like C / C++ / Python / Rust / etc. where things get unreffed automatically when they go out of scope. On languages like Ruby / C# / node / etc. where things are unreffed on GC, files can stay open for a long time after you've finished with them. This interacts in an unfortunate way with the Windows default of refusing to remove open files. This change closes file handles as soon as the scan of the input file finishes, and therefore produces something closer to expected behaviour for GCd languages on Windows. see https://github.com/kleisauke/net-vips/issues/12 --- libvips/foreign/jpeg2vips.c | 45 ++++++++++++++++++--------------- test/test-suite/test_foreign.py | 5 ++-- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/libvips/foreign/jpeg2vips.c b/libvips/foreign/jpeg2vips.c index d67239e9..79521ff6 100644 --- a/libvips/foreign/jpeg2vips.c +++ b/libvips/foreign/jpeg2vips.c @@ -188,15 +188,30 @@ typedef struct _ReadJpeg { int output_height; } ReadJpeg; +/* This can be called many times. It's called directly at the end of image + * read. + */ +static void +readjpeg_close_input( ReadJpeg *jpeg ) +{ + VIPS_FREEF( fclose, jpeg->eman.fp ); + + /* Don't call jpeg_finish_decompress(). It just checks the tail of the + * file and who cares about that. All mem is freed in + * jpeg_destroy_decompress(). + */ + + /* I don't think this can fail. It's harmless to call many times. + */ + jpeg_destroy_decompress( &jpeg->cinfo ); + +} + /* This can be called many times. */ static int readjpeg_free( ReadJpeg *jpeg ) { - int result; - - result = 0; - if( jpeg->eman.pub.num_warnings != 0 ) { g_warning( _( "read gave %ld warnings" ), jpeg->eman.pub.num_warnings ); @@ -207,24 +222,15 @@ readjpeg_free( ReadJpeg *jpeg ) jpeg->eman.pub.num_warnings = 0; } - /* Don't call jpeg_finish_decompress(). It just checks the tail of the - * file and who cares about that. All mem is freed in - * jpeg_destroy_decompress(). - */ + readjpeg_close_input( jpeg ); - VIPS_FREEF( fclose, jpeg->eman.fp ); VIPS_FREE( jpeg->filename ); - jpeg->eman.fp = NULL; - /* I don't think this can fail. It's harmless to call many times. - */ - jpeg_destroy_decompress( &jpeg->cinfo ); - - return( result ); + return( 0 ); } static void -readjpeg_close( VipsObject *object, ReadJpeg *jpeg ) +readjpeg_close_cb( VipsObject *object, ReadJpeg *jpeg ) { (void) readjpeg_free( jpeg ); } @@ -261,7 +267,7 @@ readjpeg_new( VipsImage *out, int shrink, gboolean fail, gboolean autorotate ) jpeg_create_decompress( &jpeg->cinfo ); g_signal_connect( out, "close", - G_CALLBACK( readjpeg_close ), jpeg ); + G_CALLBACK( readjpeg_close_cb ), jpeg ); return( jpeg ); } @@ -664,11 +670,10 @@ read_jpeg_generate( VipsRegion *or, jpeg->y_pos += 1; } - /* Progressive images can have a lot of memory in the decompress - * object, destroy as soon as we can. Safe to call many times. + /* Shut down the input file as soon as we can. */ if( jpeg->y_pos >= or->im->Ysize ) - jpeg_destroy_decompress( &jpeg->cinfo ); + readjpeg_close_input( jpeg ); VIPS_GATE_STOP( "read_jpeg_generate: work" ); diff --git a/test/test-suite/test_foreign.py b/test/test-suite/test_foreign.py index e729c624..98c6874b 100644 --- a/test/test-suite/test_foreign.py +++ b/test/test-suite/test_foreign.py @@ -385,9 +385,10 @@ class TestForeign: self.file_loader("magickload", BMP_FILE, bmp_valid) self.buffer_loader("magickload_buffer", BMP_FILE, bmp_valid) - # we should have rgba for svg files + # we should have rgb or rgba for svg files ... different versions of + # IM handle this differently im = pyvips.Image.magickload(SVG_FILE) - assert im.bands == 4 + assert im.bands == 3 or im.bands == 4 # density should change size of generated svg im = pyvips.Image.magickload(SVG_FILE, density='100')