On Wed, 2009-08-19 at 10:25 -0500, Mike Slegeir wrote:
> It seems that PoDoFo is not thread-safe because of its use of
> fontconfig, which according to this post I've found:
> http://osdir.com/ml/fonts.fontconfig/2006-02/msg00135.html is not thread
> thread-safe. Attached is a patch which uses PdfMutexWrapper with
> PdfMutex in order to access fontconfig in a thread-safe manner.
Thanks for the patch. I can't commit it as it because it has some issues
that'd break builds on some platforms and compilers. You can't rely on
being able to declare a static member of undefined (forward-declared)
type on all compilers, as would happen when BUILDING_PODOFO isn't
defined in the below:
+#ifdef BUILDING_PODOFO
+#if defined(PODOFO_HAVE_FONTCONFIG)
+#include "util/PdfMutex.h"
+#endif
+#else // BUILDING_PODOFO
+class PdfMutex;
+#endif // BUILDING_PODOFO
... then later, in the scope of a class declaration:
+#if defined(PODOFO_HAVE_FONTCONFIG)
+ static Util::PdfMutex m_FcMutex;
+#endif
I also really don't like conditional inclusion of headers. Much better
to just keep the header inclusion to the .cpp files if possible.
That much was easy to adjust, but there's a bigger issue. FcFontCache is
re-entrant and FcMutex, being a simple mutex, isn't. As a result,
self-deadlocks are practically guaranteed. See, for example, what
happens in CreationTest:
cr...@wallace:~/build/podofo-build/test/CreationTest$ gdb -q --args
./CreationTest out.pdf
(gdb) run
Starting program: /home/craig/build/podofo-build/test/CreationTest/CreationTest
out.pdf
[Thread debugging using libthread_db enabled]
This test tests the PdfWriter and PdfDocument classes.
It creates a new PdfFile from scratch.
---
PoDoFo DataType Size Information:
---
sizeof variant=24
sizeof object=48
sizeof reference=12
---
Drawing the first page with various lines.
^C[New Thread 0xb7c3b6d0 (LWP 19250)]
Program received signal SIGINT, Interrupt.
[Switching to Thread 0xb7c3b6d0 (LWP 19250)]
0xb804e424 in __kernel_vsyscall ()
(gdb) bt
#0 0xb804e424 in __kernel_vsyscall ()
#1 0xb7ef8cf9 in __lll_lock_wait () from /lib/tls/i686/cmov/libpthread.so.0
#2 0xb7ef4129 in _L_lock_89 () from /lib/tls/i686/cmov/libpthread.so.0
#3 0xb7ef3a32 in pthread_mutex_lock () from /lib/tls/i686/cmov/libpthread.so.0
#4 0x08157d5d in PoDoFo::Util::PdfMutex::Lock (this=0x81c3d3c) at
/home/craig/build/podofo/src/util/PdfMutex.cpp:65
#5 0x08157f57 in PdfMutexWrapper (this=0xbfe68f7c, rmut...@0x81c3d3c)
at /home/craig/build/podofo/src/util/PdfMutexWrapper.cpp:30
#6 0x080e8bed in PoDoFo::PdfFontCache::GetFontConfigFontPath
(pConfig=0x8c9fd48, pszFontName=0x81870d0 "Comic Sans MS",
bBold=false, bItalic=false) at
/home/craig/build/podofo/src/PdfFontCache.cpp:555
#7 0x080e8de2 in PoDoFo::PdfFontCache::GetFontPath (this=0xbfe69994,
pszFontName=0x81870d0 "Comic Sans MS", bBold=false,
bItalic=false) at /home/craig/build/podofo/src/PdfFontCache.cpp:593
#8 0x080e8f99 in PoDoFo::PdfFontCache::GetFont (this=0xbfe69994,
pszFontName=0x81870d0 "Comic Sans MS", bBold=false,
bItalic=false, bEmbedd=true, pEncoding=0x8c9f658, pszFileName=0x0) at
/home/craig/build/podofo/src/PdfFontCache.cpp:243
#9 0x080dcf57 in PoDoFo::PdfDocument::CreateFont (this=0xbfe69990,
pszFontName=0x81870d0 "Comic Sans MS", bBold=false,
bItalic=false, pEncoding=0x8c9f658, bEmbedd=true, pszFileName=0x0) at
/home/craig/build/podofo/src/PdfDocument.cpp:178
#10 0x080c0385 in LineTest (pPainter=0xbfe69840, pPage=0x8c9eb18,
pDocument=0xbfe69990)
at /home/craig/build/podofo/test/CreationTest/CreationTest.cpp:98
#11 0x080c16f0 in main (argc=2, argv=0xbfe69bc4) at
/home/craig/build/podofo/test/CreationTest/CreationTest.cpp:707
(gdb)
See what's happened? If you can't, just add debug logging lines to
PdfMutexWrapper's ctor and dtor and you'll be able to in a couple of
seconds. I won't bother going through the details, but basically the
main (and only) thread acquired the PdfFontCache lock then went on to
call back into PdfFontCache, try to acquire the lock again, and deadlock
against its self.
To fix this we need a counting mutex with thread ownership. When first
acquired, the mutex must not only mark its self locked but must also
record which thread (how is platform specific) holds the lock.
If the mutex is already locked and an attempt to acquire it is made, the
mutex's action depends on the thread trying to acquire it. If it's the
thread that already holds it the mutex must increment its counter and
return. If it's a different thread, the lock acquisition must fail
(block or return failure depending on if it was a blocking or nowait
lock attempt).
When released, the mutex must decrement its counter. It is considered
released when its counter is zero again.
With such a counting, owner-tracking mutex re-entrant calls are possible
and trouble-free.
PoDoFo doesn't have one yet, but it'd be easy to implement. I'm pretty
sure you'd only need to add thread identification for pthreads platforms
since the only platforms PoDoFo uses FontConfig on use pthreads. Of
course you'd want to make it easy to add other platforms anyway via a
generic PdfThreadIdentifier class whose concrete implementation could be
provided by different source files depending on platform. Eg:
class PdfThreadIdentifer {
void * m_pPrivateData;
public:
/**
* Instantiate a PdfThreadIdentifier that identifies the thread
* that called the ctor.
*/
PdfThreadIdentifer();
~PdfThreadIdentifier();
bool operator==(const PdfThreadIdentifier& rhs) const;
inline bool operator!=(const PdfThreadIdentifier& rhs) const;
}
bool PdfThreadIdentifier::operator!=(const PdfThreadIdentifier& rhs)
const {
return !( *this == rhs );
}
You'd then provide a source file, PdfThreadIdentifier_pthread.cpp, that
used the private data pointer to store suitable pthread-specific thread
identification info. Then you implement operator== and you're done.
--
Craig Ringer
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Podofo-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/podofo-users
