Hi Michal,
Unfortunately, without memory barriers, DCLP as found in these places is not
correct on x64 either, and I would not want to rely on std::lock::lock()
containing fences. As I said, the worst thing likely to happen in practice is
that two encoding objects are allocated, as writing a pointer to aligned memory
is going to be an atomic operation in virtually all cases.
Personally, I would either use the safe form where available or just initialize
all the encoding classes statically:
Option 1:
const PdfEncoding* PdfEncodingFactory::GlobalPdfDocEncodingInstance()
{
#if __cplusplus >= 201103L
static PdfDocEncoding docEncoding;
return &docEncoding;
#else // pre-C++11
if(!s_pDocEncoding) // First check
{
Util::PdfMutexWrapper wrapper( PdfEncodingFactory::s_mutex );
if(!s_pDocEncoding) // Double check
s_pDocEncoding = new PdfDocEncoding();
}
return s_pDocEncoding;
#endif
}
Option 2:
class PODOFO_API PdfEncodingFactory {
…
private:
static const PdfDocEncoding docEnconding;
…
}
const PdfEncoding* PdfEncodingFactory::GlobalPdfDocEncodingInstance()
{
return &docEncoding;
}
Cheers,
Christopher
From: Michal Sudolsky <sudols...@gmail.com>
Sent: Tuesday, May 4, 2021 17:08
To: Christopher Creutzig <ccreu...@mathworks.com>
Cc: podofo-users@lists.sourceforge.net
Subject: Re: [Podofo-users] DCLP in PoDoFo
Hi,
See:
https://sourceforge.net/p/podofo/mailman/message/35915862/<https://sourceforge.net/p/podofo/mailman/message/35915862/>
On Mon, May 3, 2021 at 2:05 PM Christopher Creutzig
<ccreu...@mathworks.com<mailto:ccreu...@mathworks.com>> wrote:
Hi list,
PdfEncodingFactory.cpp uses a broken form of double-checked
locking<https://preshing.com/20130930/double-checked-locking-is-fixed-in-cpp11/>
to initialize its encoding instances. Not a big deal, as these objects don’t
do much; technically, that is a data race and can lead to undefined behaviour,
but realistically, I would be surprised to see anything worse than a small
memory leak, if even that.
It is undefined behaviour in C++. Actually it should cause problems only on
weakly ordered processors like ARM. So you should never see it on x64.
Does PoDoFo require C++11 or newer (where there are simple fixes available)?
Will it ever?
If podofo cannot depend on C++11 then I would suggest to use "single"-checked
locking (just remove the first check):
//if(!s_pWinAnsiEncoding) // First check
//{
Util::PdfMutexWrapper wrapper( PdfEncodingFactory::s_mutex );
if(!s_pWinAnsiEncoding) // Double check
s_pWinAnsiEncoding = new PdfWinAnsiEncoding();
//}
return s_pWinAnsiEncoding;
Another solution would be to use some additional library that provides atomic
primitives for older C++ but I do not think that it would be worth it in this
case.
Cheers,
Christopher
The MathWorks GmbH | Friedlandstr.18 | 52064 Aachen | District Court Aachen |
HRB 8082 | Managing Directors: Bertrand Dissler, Steven D. Barbo, Jeanne O’Keefe
_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net<mailto:Podofo-users@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/podofo-users<https://lists.sourceforge.net/lists/listinfo/podofo-users>
_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users
