Before this patch gets lost, and with permission from Dominik received some weeks ago (sorry for the long delay), I committed the latest version of the patch in r1974, with the following commit message:
----------------------------------------------------- Patch by Michal Sudolsky: Base 14 fonts are not thread-safe PdfFontType1Base14 takes a copy of PdfFontMetricsBase14 to be instantiated: currently the copy of PdfFontMetricsBase14 is retrieved from a global array PODOFO_BUILTIN_FONTS. PdfFontMetrics is also used as a container for several state characterstics, like FontSize. Sharing the same mutable instance in different PdfFont instances created in different documents is inherently unsafe. The patch makes the global array PODOFO_BUILTIN_FONTS const and provide a copy of PdfFontMetricsBase14 when required. It also cleans PdfFontType1Base14 destructor which is not needed anymore. Passes tests and has been tested in Win32, linux x86_64. Post-Subject: Re: [Podofo-users] Base 14 fonts are not thread-safe Post-Author: Michal Sudolsky <sudolskym@...> Post-Date: Fri, 16 Nov 2018 18:26:12 +0100 Post-Message-ID: <CABoc_46zM5DR7PGTC_QTWwcYLbz1uA=tsG_j82DVt2R3UTj6Ew@...> Post-Archive-URL: https://sourceforge.net/p/podofo/mailman/message/36468911/ ----------------------------------------------------- On Fri, 16 Nov 2018 at 18:26, Michal Sudolsky <[email protected]> wrote: > > Discard previous diff. I hope final fix but someone should check it. > > > On Fri, Nov 16, 2018 at 6:17 PM Michal Sudolsky <[email protected]> wrote: >> >> One thing I forgot. PODOFO_Base14FontDef_FindBuiltinData is used on places >> where return value is not deallocated. This would cause memory leaks. I am >> sending better patch. >> >> Metrics object is allocated and copy constructed from global only on places >> where it will be eventually deallocated. >> >> Still is good idea to have PODOFO_BUILTIN_FONTS const and >> PODOFO_Base14FontDef_FindBuiltinData to return const pointer. >> >> >> On Fri, Nov 16, 2018 at 5:56 PM Michal Sudolsky <[email protected]> wrote: >>> >>> Fonts created for different documents should be independent. In other case >>> you cannot do anything usable with them when using multiple threads. >>> >>> Code: >>> ``` c++ >>> PdfMemDocument doc0; >>> PdfFont *font0 = doc0.CreateFont("Helvetica"); >>> font0->SetFontSize(10); >>> >>> PdfMemDocument doc1; >>> PdfFont *font1 = doc1.CreateFont("Helvetica"); >>> font1->SetFontSize(20); >>> >>> printf("font0 size %g\n", font0->GetFontSize()); >>> printf("font1 size %g\n", font1->GetFontSize()); >>> ``` >>> >>> Output: >>> ``` >>> font0 size 20 >>> font1 size 20 >>> ``` >>> >>> What I would expect (or what happens when is font for example "Arial"): >>> ``` >>> font0 size 10 >>> font1 size 20 >>> ``` >>> >>> Fonts font0 and font1 are different objects but they share same m_pMetrics >>> (in case of base 14 fonts) which is used for font size and other font >>> settings. This is because PODOFO_Base14FontDef_FindBuiltinData is returning >>> mutable pointer to static global array PODOFO_BUILTIN_FONTS. >>> >>> This is here probably from beginning. >>> >>> I am sending patch. Hopefully I did not forget something. >>> PODOFO_Base14FontDef_FindBuiltinData is now returning newly allocated >>> metrics object copy constructed from PODOFO_BUILTIN_FONTS[i]. I deleted >>> destructor of PdfFontTypeBase14 so metrics object is properly deleted with >>> font. I also changed PODOFO_BUILTIN_FONTS to const for sure that there is >>> nothing else that changes it. >>> > _______________________________________________ > Podofo-users mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/podofo-users _______________________________________________ Podofo-users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/podofo-users
