> If we want to avoid UB in the multithreaded world, I’m afraid we will have > to make a C++11 compiler a requirement, as C++03 never acknowledged the > existence of threads. (That is not limited to this place, a lot of methods > like PdfEncodingFactory::GlobalPdfRomanEncodingInstance are not currently > threadsafe in C++03, as discussed earlier.) > > That is not thread-safe even in C++11.
There's no need for extra support > on the C++ level, when there are existing tools for it already. Except that some things are not so available as threads like for example thread_local and atomic operations. DCLP in GlobalWinAnsiEncodingInstance and similar would need load-acquire (or "consume" may be enough) and store-release. Without C++11 this would need to be compiler specific for example GCC has "__atomic" builtins and also something for thread_local. So why were these things not fixed in podofo already a long time ago? Maybe because it is somehow simpler to just use a simple multi-platform library like pthread than to use different things on each compiler. > Option 1) > > #if !defined(PODOFO_MULTI_THREAD) > static int s_nRecursionDepth; // PoDoFo is single threaded > #elif defined(thread_local) > static int thread_local s_nRecursionDepth; // PoDoFo has threading > support and using C++11 compiler > #else > #error C++11 thread_local is required for multi-thread > #endif > > The thread_local macro is available in Seems that macro is not available in C++ (it is present probably only when compiling source as C). I think this is a better option. I like this idea. As PODOFO_MULTI_THREAD will enable C++11 then also other things like broken DCLP in GlobalWinAnsiEncodingInstance can be easily fixed. It would simply use correct atomic operations when this macro is enabled or does not use any mutex here at all if this macro is not defined as in a single threaded environment there is no need to use locking in GlobalWinAnsiEncodingInstance (and in similar functions). Or it may just use correct atomics when it is compiled with C++11 and be as it is now when not so it would be fixed for C++11 but it will still be buggy without C++11 (it would be simpler to fix it just for C++11 as to use compiler specific things like GCC "__atomic" builtins). The 500 limit should be enough for a 32-bit release build with a 256 KB > stack (it would use about 195 KB) Not when the user of podofo already used some 64 KB before calling podofo. To me it seems more reasonable to use a more conservative value which would not consume more than some half (or tenth?) of the available stack in the worst case. Also worth discussing – should it be possible to disable the recursion > guard completely with SetMaxRecursionDepth(0) ? This is a bad idea with > untrusted input, but might make sense in some situations. Not worth it. The user may just set it to 1 million or something. Only supporting compilers that still get security updates is a simple way > to get rid of old compilers, and easy to justify. I agree. (Btw will podofo run in Turbo C++ 3.0?) >Do we agree this would implement different functionality, with the > potential of hard to debug sporadic effects depending on how the threads > actually run? > > > > Yes, although this already happens with PoDoFo – all the PoDoFo mutex > methods are defined as no-ops unless PODOFO_MULTI_THREAD is defined. > > I do not see how having or not having these mutexes in a single-threaded > situation makes a functional difference. Nor does global vs thread local counter make difference in single-threaded situation. > What I meant was: Do we agree there is a fundamental difference between > using thread local counters vs. global counters (atomic or behind mutexes), > both in the PODOFO_MULTI_THREAD situation? I suppose he meant that in a multi-threaded situation yes global counter would mean different functionality. But also if someone forgets to define PODOFO_MULTI_THREAD (in a multi-threaded situation) he will also get some hard to debug sporadic effects (I think it happened to me when I first "played" with podofo). >
_______________________________________________ Podofo-users mailing list Podofo-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/podofo-users
