From 366262dc7854ba54f64905df8d275358be41edf5 Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Thu, 12 May 2016 14:17:15 -0700 Subject: Only have a single call into pdfium at a time. Pdfium is not thread safe and uses global variables, hence no parallel call pdfium is safe Fixes: 28705066 Change-Id: I04309ee691bd9cea37587e0af5be2c07ce8c9f2c (cherry picked from commit 0768a7dc450caf4c873c5b0883a75135536e1546) --- core/jni/android/graphics/pdf/PdfEditor.cpp | 3 - core/jni/android/graphics/pdf/PdfRenderer.cpp | 3 - graphics/java/android/graphics/pdf/PdfEditor.java | 65 +++++++++++++++++----- .../java/android/graphics/pdf/PdfRenderer.java | 37 +++++++++--- 4 files changed, 79 insertions(+), 29 deletions(-) diff --git a/core/jni/android/graphics/pdf/PdfEditor.cpp b/core/jni/android/graphics/pdf/PdfEditor.cpp index d2d39cd0b286..0468b644862d 100644 --- a/core/jni/android/graphics/pdf/PdfEditor.cpp +++ b/core/jni/android/graphics/pdf/PdfEditor.cpp @@ -52,11 +52,9 @@ static struct { } gRectClassInfo; // Also used in PdfRenderer.cpp -Mutex sPdfiumLock; int sUnmatchedPdfiumInitRequestCount = 0; static void initializeLibraryIfNeeded() { - Mutex::Autolock _l(sPdfiumLock); if (sUnmatchedPdfiumInitRequestCount == 0) { FPDF_InitLibrary(); } @@ -64,7 +62,6 @@ static void initializeLibraryIfNeeded() { } static void destroyLibraryIfNeeded() { - Mutex::Autolock _l(sPdfiumLock); sUnmatchedPdfiumInitRequestCount--; if (sUnmatchedPdfiumInitRequestCount == 0) { FPDF_DestroyLibrary(); diff --git a/core/jni/android/graphics/pdf/PdfRenderer.cpp b/core/jni/android/graphics/pdf/PdfRenderer.cpp index 71bec7845fd7..43550ac9ed72 100644 --- a/core/jni/android/graphics/pdf/PdfRenderer.cpp +++ b/core/jni/android/graphics/pdf/PdfRenderer.cpp @@ -44,11 +44,9 @@ static struct { } gPointClassInfo; // See PdfEditor.cpp -extern Mutex sPdfiumLock; extern int sUnmatchedPdfiumInitRequestCount; static void initializeLibraryIfNeeded() { - Mutex::Autolock _l(sPdfiumLock); if (sUnmatchedPdfiumInitRequestCount == 0) { FPDF_InitLibrary(); } @@ -56,7 +54,6 @@ static void initializeLibraryIfNeeded() { } static void destroyLibraryIfNeeded() { - Mutex::Autolock _l(sPdfiumLock); sUnmatchedPdfiumInitRequestCount--; if (sUnmatchedPdfiumInitRequestCount == 0) { FPDF_DestroyLibrary(); diff --git a/graphics/java/android/graphics/pdf/PdfEditor.java b/graphics/java/android/graphics/pdf/PdfEditor.java index 2b70b6a45f82..cd1f8de6ee0f 100644 --- a/graphics/java/android/graphics/pdf/PdfEditor.java +++ b/graphics/java/android/graphics/pdf/PdfEditor.java @@ -79,8 +79,12 @@ public final class PdfEditor { } mInput = input; - mNativeDocument = nativeOpen(mInput.getFd(), size); - mPageCount = nativeGetPageCount(mNativeDocument); + + synchronized (PdfRenderer.sPdfiumLock) { + mNativeDocument = nativeOpen(mInput.getFd(), size); + mPageCount = nativeGetPageCount(mNativeDocument); + } + mCloseGuard.open("close"); } @@ -102,7 +106,10 @@ public final class PdfEditor { public void removePage(int pageIndex) { throwIfClosed(); throwIfPageNotInDocument(pageIndex); - mPageCount = nativeRemovePage(mNativeDocument, pageIndex); + + synchronized (PdfRenderer.sPdfiumLock) { + mPageCount = nativeRemovePage(mNativeDocument, pageIndex); + } } /** @@ -125,11 +132,16 @@ public final class PdfEditor { if (clip == null) { Point size = new Point(); getPageSize(pageIndex, size); - nativeSetTransformAndClip(mNativeDocument, pageIndex, transform.native_instance, - 0, 0, size.x, size.y); + + synchronized (PdfRenderer.sPdfiumLock) { + nativeSetTransformAndClip(mNativeDocument, pageIndex, transform.native_instance, + 0, 0, size.x, size.y); + } } else { - nativeSetTransformAndClip(mNativeDocument, pageIndex, transform.native_instance, - clip.left, clip.top, clip.right, clip.bottom); + synchronized (PdfRenderer.sPdfiumLock) { + nativeSetTransformAndClip(mNativeDocument, pageIndex, transform.native_instance, + clip.left, clip.top, clip.right, clip.bottom); + } } } @@ -143,7 +155,10 @@ public final class PdfEditor { throwIfClosed(); throwIfOutSizeNull(outSize); throwIfPageNotInDocument(pageIndex); - nativeGetPageSize(mNativeDocument, pageIndex, outSize); + + synchronized (PdfRenderer.sPdfiumLock) { + nativeGetPageSize(mNativeDocument, pageIndex, outSize); + } } /** @@ -156,7 +171,10 @@ public final class PdfEditor { throwIfClosed(); throwIfOutMediaBoxNull(outMediaBox); throwIfPageNotInDocument(pageIndex); - return nativeGetPageMediaBox(mNativeDocument, pageIndex, outMediaBox); + + synchronized (PdfRenderer.sPdfiumLock) { + return nativeGetPageMediaBox(mNativeDocument, pageIndex, outMediaBox); + } } /** @@ -169,7 +187,10 @@ public final class PdfEditor { throwIfClosed(); throwIfMediaBoxNull(mediaBox); throwIfPageNotInDocument(pageIndex); - nativeSetPageMediaBox(mNativeDocument, pageIndex, mediaBox); + + synchronized (PdfRenderer.sPdfiumLock) { + nativeSetPageMediaBox(mNativeDocument, pageIndex, mediaBox); + } } /** @@ -182,7 +203,10 @@ public final class PdfEditor { throwIfClosed(); throwIfOutCropBoxNull(outCropBox); throwIfPageNotInDocument(pageIndex); - return nativeGetPageCropBox(mNativeDocument, pageIndex, outCropBox); + + synchronized (PdfRenderer.sPdfiumLock) { + return nativeGetPageCropBox(mNativeDocument, pageIndex, outCropBox); + } } /** @@ -195,7 +219,10 @@ public final class PdfEditor { throwIfClosed(); throwIfCropBoxNull(cropBox); throwIfPageNotInDocument(pageIndex); - nativeSetPageCropBox(mNativeDocument, pageIndex, cropBox); + + synchronized (PdfRenderer.sPdfiumLock) { + nativeSetPageCropBox(mNativeDocument, pageIndex, cropBox); + } } /** @@ -205,7 +232,10 @@ public final class PdfEditor { */ public boolean shouldScaleForPrinting() { throwIfClosed(); - return nativeScaleForPrinting(mNativeDocument); + + synchronized (PdfRenderer.sPdfiumLock) { + return nativeScaleForPrinting(mNativeDocument); + } } /** @@ -219,7 +249,10 @@ public final class PdfEditor { public void write(ParcelFileDescriptor output) throws IOException { try { throwIfClosed(); - nativeWrite(mNativeDocument, output.getFd()); + + synchronized (PdfRenderer.sPdfiumLock) { + nativeWrite(mNativeDocument, output.getFd()); + } } finally { IoUtils.closeQuietly(output); } @@ -247,7 +280,9 @@ public final class PdfEditor { } private void doClose() { - nativeClose(mNativeDocument); + synchronized (PdfRenderer.sPdfiumLock) { + nativeClose(mNativeDocument); + } IoUtils.closeQuietly(mInput); mInput = null; mCloseGuard.close(); diff --git a/graphics/java/android/graphics/pdf/PdfRenderer.java b/graphics/java/android/graphics/pdf/PdfRenderer.java index 520ebe5f2db8..cfc130990e92 100644 --- a/graphics/java/android/graphics/pdf/PdfRenderer.java +++ b/graphics/java/android/graphics/pdf/PdfRenderer.java @@ -99,6 +99,12 @@ import java.lang.annotation.RetentionPolicy; * @see #close() */ public final class PdfRenderer implements AutoCloseable { + /** + * Any call the native pdfium code has to be single threaded as the library does not support + * parallel use. + */ + final static Object sPdfiumLock = new Object(); + private final CloseGuard mCloseGuard = CloseGuard.get(); private final Point mTempPoint = new Point(); @@ -154,8 +160,12 @@ public final class PdfRenderer implements AutoCloseable { } mInput = input; - mNativeDocument = nativeCreate(mInput.getFd(), size); - mPageCount = nativeGetPageCount(mNativeDocument); + + synchronized (sPdfiumLock) { + mNativeDocument = nativeCreate(mInput.getFd(), size); + mPageCount = nativeGetPageCount(mNativeDocument); + } + mCloseGuard.open("close"); } @@ -189,7 +199,10 @@ public final class PdfRenderer implements AutoCloseable { */ public boolean shouldScaleForPrinting() { throwIfClosed(); - return nativeScaleForPrinting(mNativeDocument); + + synchronized (sPdfiumLock) { + return nativeScaleForPrinting(mNativeDocument); + } } /** @@ -224,7 +237,9 @@ public final class PdfRenderer implements AutoCloseable { if (mCurrentPage != null) { mCurrentPage.close(); } - nativeClose(mNativeDocument); + synchronized (sPdfiumLock) { + nativeClose(mNativeDocument); + } try { mInput.close(); } catch (IOException ioe) { @@ -277,7 +292,9 @@ public final class PdfRenderer implements AutoCloseable { private Page(int index) { Point size = mTempPoint; - mNativePage = nativeOpenPageAndGetSize(mNativeDocument, index, size); + synchronized (sPdfiumLock) { + mNativePage = nativeOpenPageAndGetSize(mNativeDocument, index, size); + } mIndex = index; mWidth = size.x; mHeight = size.y; @@ -384,8 +401,10 @@ public final class PdfRenderer implements AutoCloseable { final long transformPtr = (transform != null) ? transform.native_instance : 0; - nativeRenderPage(mNativeDocument, mNativePage, destination, contentLeft, - contentTop, contentRight, contentBottom, transformPtr, renderMode); + synchronized (sPdfiumLock) { + nativeRenderPage(mNativeDocument, mNativePage, destination, contentLeft, + contentTop, contentRight, contentBottom, transformPtr, renderMode); + } } /** @@ -412,7 +431,9 @@ public final class PdfRenderer implements AutoCloseable { } private void doClose() { - nativeClosePage(mNativePage); + synchronized (sPdfiumLock) { + nativeClosePage(mNativePage); + } mNativePage = 0; mCloseGuard.close(); mCurrentPage = null; -- cgit v1.2.3-59-g8ed1b