Bug 76219 – cairo_t* memory leak in GraphicsContext::platformInit

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brian Stuart]

In source\webcore\platform\graphics\win\graphicscontextcairowin.cpp GraphicsContext::platformInit createCairoContextWithHDC is called returning a cairo_t* which has a reference count of 1. Passing that cairo_t* to PlatformContextCairo's ctor causes the reference count to increment to 2. Now when ~PlatformContextCairo is called it's cairo_t* is leaked.

Here is my workaround, a better solution would involve using adoptRef and passing a RefPtr to PlatformContextCairo's ctor:

void GraphicsContext::platformInit(HDC dc, bool hasAlpha)
{
    cairo_t* cr = 0;
    if (dc)
        cr = createCairoContextWithHDC(dc, hasAlpha);//cr is created with ref count of 1
    else
        setPaintingDisabled(true);	
	
    m_data = new GraphicsContextPlatformPrivateToplevel(new PlatformContextCairo(cr));//cr now has a ref count of 2 because of PlatformContextCairo RefPtr<cairo_t> m_cr;
	
    cairo_destroy(cr);//to prevent memory leak restore cr ref count to 1 so ~PlatformContextCairo will destroy with ref count of 1 
	
	m_data->m_hdc = dc;
    if (platformContext()->cr()) {
        // Make sure the context starts in sync with our state.
        setPlatformFillColor(fillColor(), fillColorSpace());
        setPlatformStrokeColor(strokeColor(), strokeColorSpace());
    }
}

[Hideki]

Created attachment 169104 [details]
Proposal for fixing the memory leak in WinCairo

Thank you for useful information, Brain.

My team also found same memory leak problem in WinCairo port and
figured out the same bug.

Here is the patch which we made.

We just moved
 cairo_destroy(cr);
to the end of the function and added if statement to 
make sure that cr is not ZERO for fear that createCairoContextWithHDC
is not called.

[WebKit Review Bot]

Attachment 169104 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/platform/graphics/win/Graph..." exit_code: 1
Source/WebCore/platform/graphics/win/GraphicsContextCairoWin.cpp:91:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 1 in 1 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Hideki]

Created attachment 173567 [details]
Proposal for fixing the memory leak in WinCairo (Revised patch)

We revised the patch refering the comment sent by BOT.
We just modified its format, no context of program has been changed.

------
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/platform/graphics/win/Graph..." exit_code: 1
Source/WebCore/platform/graphics/win/GraphicsContextCairoWin.cpp:91:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 1 in 1 files
------

[Brent Fulgham]

Comment on attachment 173567 [details]
Proposal for fixing the memory leak in WinCairo (Revised patch)

Nice catch.  I think this patch is fine, though there is no ChangeLog entry.  Please add a ChangeLog to the patch and I will be happy to approve it.  r- because of the missing ChangeLog.

[Hideki]

Created attachment 174097 [details]
Proposal for fixing the memory leak in WinCairo (Revised patch v2)

Added ChangeLog.

[Brent Fulgham]

Comment on attachment 174097 [details]
Proposal for fixing the memory leak in WinCairo (Revised patch v2)

View in context: https://bugs.webkit.org/attachment.cgi?id=174097&action=review

r = me; I will correct the URL ordering and land the change.

> Source/WebCore/ChangeLog:3
> +        https://bugs.webkit.org/show_bug.cgi?id=76219

We put the bug URL after the short description.

[Brent Fulgham]

Committed r134629: <http://trac.webkit.org/changeset/134629>