wxPdfDocument-Add new font (found memory leak) Topic is solved

Talk here about issues with one of the components hosted at wxCode, or suggest features for it.
Post Reply
JenT
Earned some good credits
Earned some good credits
Posts: 114
Joined: Fri Oct 24, 2008 9:23 am

wxPdfDocument-Add new font (found memory leak)

Post by JenT »

Hi~
I add new Font,and I found a memory leak.

As follow:

Code: Select all

bool
wxPdfFontSubsetCff::ReadFontDict(wxPdfCffDictionary* dict, int dictOffset, int dictSize)
{
  bool ok = true;
  SeekI(dictOffset);
  int end = dictOffset + dictSize;
  int argStart, argSize, argTotal, op;
  while (TellI() < end)
  {
    argStart = TellI();
    argTotal = 0;
    do
    {
      argSize = ReadOperandLength();
      argTotal += argSize;
      SeekI(argStart + argTotal);
    }
    while (argSize > 0);
    op = ReadOperator();
    wxPdfCffDictElement* dictElement = new wxPdfCffDictElement(op, m_inFont, argStart, argTotal);
   //add this code to check 
   if ((*dict).count(op)>0)
    {
		delete (*dict)[op];
    }

	(*dict)[op] = dictElement;
  }
  return ok;
}

utelle
Moderator
Moderator
Posts: 1125
Joined: Tue Jul 05, 2005 10:00 pm
Location: Cologne, Germany
Contact:

Re: wxPdfDocument-Add new font (found memory leak)

Post by utelle »

JenT wrote:Hi~
I add new Font,and I found a memory leak.

As follow:

Code: Select all

bool
wxPdfFontSubsetCff::ReadFontDict(wxPdfCffDictionary* dict, int dictOffset, int dictSize)
{
  bool ok = true;
  SeekI(dictOffset);
  int end = dictOffset + dictSize;
  int argStart, argSize, argTotal, op;
  while (TellI() < end)
  {
    argStart = TellI();
    argTotal = 0;
    do
    {
      argSize = ReadOperandLength();
      argTotal += argSize;
      SeekI(argStart + argTotal);
    }
    while (argSize > 0);
    op = ReadOperator();
    wxPdfCffDictElement* dictElement = new wxPdfCffDictElement(op, m_inFont, argStart, argTotal);
   //add this code to check 
   if ((*dict).count(op)>0)
    {
		delete (*dict)[op];
    }

	(*dict)[op] = dictElement;
  }
  return ok;
}

Well, a duplicate entry to the dictionary for the same operator should not happen. That is, the font you are using seems to be "special" in a sense. Which font are you trying to add? Is the font file freely available, or could you provide the font file for further checking of the issue?

Regards,

Ulrich
JenT
Earned some good credits
Earned some good credits
Posts: 114
Joined: Fri Oct 24, 2008 9:23 am

Re: wxPdfDocument-Add new font (found memory leak)

Post by JenT »

utelle wrote:
JenT wrote:Hi~
I add new Font,and I found a memory leak.

As follow:

Code: Select all

bool
wxPdfFontSubsetCff::ReadFontDict(wxPdfCffDictionary* dict, int dictOffset, int dictSize)
{
  bool ok = true;
  SeekI(dictOffset);
  int end = dictOffset + dictSize;
  int argStart, argSize, argTotal, op;
  while (TellI() < end)
  {
    argStart = TellI();
    argTotal = 0;
    do
    {
      argSize = ReadOperandLength();
      argTotal += argSize;
      SeekI(argStart + argTotal);
    }
    while (argSize > 0);
    op = ReadOperator();
    wxPdfCffDictElement* dictElement = new wxPdfCffDictElement(op, m_inFont, argStart, argTotal);
   //add this code to check 
   if ((*dict).count(op)>0)
    {
		delete (*dict)[op];
    }

	(*dict)[op] = dictElement;
  }
  return ok;
}

Well, a duplicate entry to the dictionary for the same operator should not happen. That is, the font you are using seems to be "special" in a sense. Which font are you trying to add? Is the font file freely available, or could you provide the font file for further checking of the issue?

Regards,

Ulrich
I use source-sans-pro font.
and use
AddFont(wxT("SourceSansPro"), wxT(""), wxT("SourceSansPro-Regular.otf"));
utelle
Moderator
Moderator
Posts: 1125
Joined: Tue Jul 05, 2005 10:00 pm
Location: Cologne, Germany
Contact:

Re: wxPdfDocument-Add new font (found memory leak)

Post by utelle »

JenT wrote:
utelle wrote:Well, a duplicate entry to the dictionary for the same operator should not happen. That is, the font you are using seems to be "special" in a sense. Which font are you trying to add? Is the font file freely available, or could you provide the font file for further checking of the issue?
I use source-sans-pro font.
and use
AddFont(wxT("SourceSansPro"), wxT(""), wxT("SourceSansPro-Regular.otf"));
Thanks for the information. Since the font is freely available I will take a look to find out what's going on. However, this may take a while.

Releasing the memory as you did in your quick fix will remove the memory leak, but maybe later on one might experience difficulties with the resulting font subset. An alternative would be to use the TrueType flavor of the font (SourceSansPro-Regular.ttf) - at least for the time being.

Regards,

Ulrich
utelle
Moderator
Moderator
Posts: 1125
Joined: Tue Jul 05, 2005 10:00 pm
Location: Cologne, Germany
Contact:

Re: wxPdfDocument-Add new font (found memory leak)

Post by utelle »

JenT wrote:
utelle wrote:
JenT wrote:Hi~
I add new Font,and I found a memory leak.
Well, a duplicate entry to the dictionary for the same operator should not happen. That is, the font you are using seems to be "special" in a sense. Which font are you trying to add? Is the font file freely available, or could you provide the font file for further checking of the issue?
I use source-sans-pro font.
and use
AddFont(wxT("SourceSansPro"), wxT(""), wxT("SourceSansPro-Regular.otf"));
I tested to reproduce the memory leak using the font SourceSansPro-Regular.otf. However, in my tests no memory leak occurred. Maybe it depends on the glyphs actually used. Could you provide the source code with which you experienced the memory leak?

Regards,

Ulrich
JenT
Earned some good credits
Earned some good credits
Posts: 114
Joined: Fri Oct 24, 2008 9:23 am

Re: wxPdfDocument-Add new font (found memory leak)

Post by JenT »

Code: Select all

#include "wx/pdfdoc.h"
#include "wx/PdfFontManager.h"
class TestPdf : public wxPdfDocument
{
	public:
		TestPdf()
		{
			wxString fontPath = wxGetCwd() +wxFileName::GetPathSeparator()+ _T("Fonts");
			wxSetEnv(wxT("WXPDF_FONTPATH"), fontPath);
			AddFont(wxT("SourceSansPro"), wxT(""), wxT("Regular.otf"));
			AddFont(wxT("SourceSansPro"), wxT("B"), wxT("Bold.otf"));
			AddFont(wxT("SourceSansPro"), wxT("I"), wxT("It.otf"));
			AddFont(wxT("SourceSansPro"), wxT("BI"), wxT("BoldIt.otf"));
			m_FontName=wxT("SourceSansPro");
		}
		
		void SetFontStyle(wxString Style=wxT("B"))
		{
			int iSize=GetFontSize();

			SetFont(m_FontName,Style,iSize);
		}
		
		// Page header
		void Header()
		{
			SetDrawColour(wxColour(0,0,0));
			SetFont(m_FontName,wxT("B"),20);
		    Ln(5);
			Cell(80);
			// Title
			Cell(40,10,wxT("Summary Report"),wxPDF_BORDER_NONE,0,wxPDF_ALIGN_CENTER);
			// Line break
			Ln(18);
			SetFont(m_FontName,wxT(""),10);
			SetLineWidth(0.3);
			Cell(14);
			Cell(20,5,m_Description,wxPDF_BORDER_NONE,0,wxPDF_ALIGN_LEFT);
			Ln(6);
			Cell(14);
			Cell(20,5,m_StandardNo,wxPDF_BORDER_NONE,0,wxPDF_ALIGN_LEFT);
			Ln(6);
			Cell(14);
			Cell(20,5,m_AnalysisFormat,wxPDF_BORDER_NONE,0,wxPDF_ALIGN_LEFT);
			Ln();
			// set the line width

			// draw start frame bar
			Line(20, 32, 180, 32 );
			// draw start frame bar
			Line(20, 53, 180, 53 );
			Ln();
		}
		// Page footer
		void Footer()
		{
			// Position at 1.5 cm from bottom
			SetY(-15);
			// Helvetica italic 8
			SetFont(m_FontName,wxT("I"),8);
			// Page number
			Cell(0,10,wxString::Format(wxT("Page %d"),PageNo()),0,0,wxPDF_ALIGN_CENTER);
		}
	private:
		wxString m_FontName;
};
I pick some fonts and modify those font's name
Because SourceSansPro are more fonts,
See attachment.

I simple my code,but I don't test it.
I hope it is useful.
Attachments
Fonts.7z
(184.92 KiB) Downloaded 156 times
utelle
Moderator
Moderator
Posts: 1125
Joined: Tue Jul 05, 2005 10:00 pm
Location: Cologne, Germany
Contact:

Re: wxPdfDocument-Add new font (found memory leak)

Post by utelle »

JenT wrote: [...]I pick some fonts and modify those font's name
Because SourceSansPro are more fonts,
See attachment.

I simple my code,but I don't test it.
I hope it is useful.
I used your code to reproduce the problem. I had to adjust the code to make it compilable, but that wasn't too much work.

In fact, the memory leak surfaces only for the bold font. In my previous tests I used the regular font, because you mentioned it explicitly.

Since the bug surfaced during reading a CFF dictionary, this means that either the font file is corrupt or the code reading the dictionary has a bug. Reading a CFF dictionary is not trivial, that is, introducing bugs is easy. And in fact, the memory leak is a result of a bug in the code

Line 685 of original unmodified file pdffontsubsetcff.cpp reads:

Code: Select all

    operandLength = TellI() - begin + 1;
But it should read:

Code: Select all

    operandLength = TellI() - begin;
That is, the operand length was too large by 1. After this fix the memory leak will be gone. (The code has already been fixed in the wxCode SVN repository.)

It's interesting that it took so long to discover this bug in wxPdfDocument. Obviously people seldom use OpenType fonts in CFF format. Additionally, operators with real number operands seem to be rare. Otherwise, this bug would have certainly been discovered earlier.

Thanks for reporting the problem.

Regards,

Ulrich
Post Reply