From ff58c67e33358a1d7df12b7edc2c239ac47ef798 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 8 Aug 2019 15:54:16 +0100 Subject: [PATCH] add locks for pdfium load We used to lock within documents, ie. we did not allow two threads to work on the same file. However pdfium is not threadsafe in any way, and this is not supported, see: https://groups.google.com/forum/#!msg/pdfium/kyIdh_J4csg/K1LvfPiHDwAJ This patch adds locks around pdfium calls. see: https://github.com/libvips/libvips/issues/1380 https://github.com/libvips/libvips/issues/1275 --- ChangeLog | 1 + libvips/foreign/pdfiumload.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/ChangeLog b/ChangeLog index d2845ac9..b432b770 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5,6 +5,7 @@ - fix loop in malformed ppm [Kyle-Kyle] - better support for PNGs with long comment names - fix build with GM +- add locks for pdfium load 24/5/19 started 8.8.1 - improve realpath() use on older libc diff --git a/libvips/foreign/pdfiumload.c b/libvips/foreign/pdfiumload.c index 4deb2ba0..0a1e11e3 100644 --- a/libvips/foreign/pdfiumload.c +++ b/libvips/foreign/pdfiumload.c @@ -6,6 +6,8 @@ * - add background param * 16/8/18 * - shut down the input file as soon as we can [kleisauke] + * 8/8/19 + * - add locks, since pdfium is not threadsafe in any way */ /* @@ -139,8 +141,12 @@ vips_pdfium_error( void ) static void vips_foreign_load_pdf_close( VipsForeignLoadPdf *pdf ) { + g_mutex_lock( vips__global_lock ); + VIPS_FREEF( FPDF_ClosePage, pdf->page ); VIPS_FREEF( FPDF_CloseDocument, pdf->doc ); + + g_mutex_unlock( vips__global_lock ); } static void @@ -209,6 +215,8 @@ vips_foreign_load_pdf_get_page( VipsForeignLoadPdf *pdf, int page_no ) if( pdf->current_page != page_no ) { VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( pdf ); + g_mutex_lock( vips__global_lock ); + VIPS_FREEF( FPDF_ClosePage, pdf->page ); pdf->current_page = -1; @@ -217,12 +225,15 @@ vips_foreign_load_pdf_get_page( VipsForeignLoadPdf *pdf, int page_no ) #endif /*DEBUG*/ if( !(pdf->page = FPDF_LoadPage( pdf->doc, page_no )) ) { + g_mutex_unlock( vips__global_lock ); vips_pdfium_error(); vips_error( class->nickname, _( "unable to load page %d" ), page_no ); return( -1 ); } pdf->current_page = page_no; + + g_mutex_unlock( vips__global_lock ); } return( 0 ); @@ -272,7 +283,9 @@ vips_foreign_load_pdf_set_image( VipsForeignLoadPdf *pdf, VipsImage *out ) char text[1024]; int len; + g_mutex_lock( vips__global_lock ); len = FPDF_GetMetaText( pdf->doc, metadata->tag, text, 1024 ); + g_mutex_unlock( vips__global_lock ); if( len > 0 ) { char *str; @@ -312,7 +325,9 @@ vips_foreign_load_pdf_header( VipsForeignLoad *load ) printf( "vips_foreign_load_pdf_header: %p\n", pdf ); #endif /*DEBUG*/ + g_mutex_lock( vips__global_lock ); pdf->n_pages = FPDF_GetPageCount( pdf->doc ); + g_mutex_unlock( vips__global_lock ); /* @n == -1 means until the end of the doc. */ @@ -422,6 +437,8 @@ vips_foreign_load_pdf_generate( VipsRegion *or, /* 4 means RGBA. */ + g_mutex_lock( vips__global_lock ); + bitmap = FPDFBitmap_CreateEx( rect.width, rect.height, 4, VIPS_REGION_ADDR( or, rect.left, rect.top ), VIPS_REGION_LSKIP( or ) ); @@ -432,6 +449,8 @@ vips_foreign_load_pdf_generate( VipsRegion *or, FPDFBitmap_Destroy( bitmap ); + g_mutex_unlock( vips__global_lock ); + top += rect.height; i += 1; } @@ -581,13 +600,18 @@ vips_foreign_load_pdf_file_header( VipsForeignLoad *load ) VipsForeignLoadPdf *pdf = (VipsForeignLoadPdf *) load; VipsForeignLoadPdfFile *file = (VipsForeignLoadPdfFile *) load; + g_mutex_lock( vips__global_lock ); + if( !(pdf->doc = FPDF_LoadDocument( file->filename, NULL )) ) { + g_mutex_unlock( vips__global_lock ); vips_pdfium_error(); vips_error( "pdfload", _( "unable to load \"%s\"" ), file->filename ); return( -1 ); } + g_mutex_unlock( vips__global_lock ); + VIPS_SETSTR( load->out->filename, file->filename ); return( vips_foreign_load_pdf_header( load ) ); @@ -652,14 +676,19 @@ vips_foreign_load_pdf_buffer_header( VipsForeignLoad *load ) VipsForeignLoadPdfBuffer *buffer = (VipsForeignLoadPdfBuffer *) load; + g_mutex_lock( vips__global_lock ); + if( !(pdf->doc = FPDF_LoadMemDocument( buffer->buf->data, buffer->buf->length, NULL )) ) { + g_mutex_unlock( vips__global_lock ); vips_pdfium_error(); vips_error( "pdfload", "%s", _( "unable to load from buffer" ) ); return( -1 ); } + g_mutex_unlock( vips__global_lock ); + return( vips_foreign_load_pdf_header( load ) ); }