76219 2012-01-12 15:06:58 -0800 cairo_t* memory leak in GraphicsContext::platformInit 2012-11-14 10:54:06 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) PC Windows 7 RESOLVED FIXED http://smithmicro.com P2 Normal --- 0 bstuart webkit-unassigned bfulgham dbates hirano-txb mrobinson webkit.review.bot yoshida-hxa oldest_to_newest 535207 0 bstuart 2012-01-12 15:06:58 -0800 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()); } } 743944 1 169104 yoshida-hxa 2012-10-17 00:21:17 -0700 Created attachment 169104 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. 759157 2 webkit.review.bot 2012-11-05 18:59:50 -0800 Attachment 169104 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. 764084 3 173567 yoshida-hxa 2012-11-11 23:12:18 -0800 Created attachment 173567 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 ------ 765684 4 173567 bfulgham 2012-11-13 09:20:20 -0800 Comment on attachment 173567 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. 766572 5 174097 yoshida-hxa 2012-11-14 00:50:59 -0800 Created attachment 174097 Proposal for fixing the memory leak in WinCairo (Revised patch v2) Added ChangeLog. 766969 6 174097 bfulgham 2012-11-14 09:29:15 -0800 Comment on attachment 174097 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. 767056 7 bfulgham 2012-11-14 10:54:06 -0800 Committed r134629: <http://trac.webkit.org/changeset/134629> 169104 2012-10-17 00:21:17 -0700 2012-11-11 23:13:25 -0800 Proposal for fixing the memory leak in WinCairo diff.txt text/plain 605 yoshida-hxa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3dpbi9HcmFwaGljc0NvbnRl eHRDYWlyb1dpbi5jcHANCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0NCi0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9n cmFwaGljcy93aW4vR3JhcGhpY3NDb250ZXh0Q2Fpcm9XaW4uY3BwCShyZXZpc2lvbiAxMzExMDkp DQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvd2luL0dyYXBoaWNzQ29udGV4 dENhaXJvV2luLmNwcAkod29ya2luZyBjb3B5KQ0KQEAgLTg4LDYgKzg4LDcgQEANCiAgICAgICAg IHNldFBsYXRmb3JtRmlsbENvbG9yKGZpbGxDb2xvcigpLCBmaWxsQ29sb3JTcGFjZSgpKTsNCiAg ICAgICAgIHNldFBsYXRmb3JtU3Ryb2tlQ29sb3Ioc3Ryb2tlQ29sb3IoKSwgc3Ryb2tlQ29sb3JT cGFjZSgpKTsNCiAgICAgfQ0KKyAgICBpZiAoY3IpIGNhaXJvX2Rlc3Ryb3koY3IpOw0KIH0NCiAN CiBzdGF0aWMgdm9pZCBzZXRSR0JBQml0bWFwQWxwaGEodW5zaWduZWQgY2hhciogYnl0ZXMsIHNp emVfdCBsZW5ndGgsIHVuc2lnbmVkIGNoYXIgbGV2ZWwpDQo= 173567 2012-11-11 23:12:18 -0800 2012-11-14 00:48:45 -0800 Proposal for fixing the memory leak in WinCairo (Revised patch) diff2.txt text/plain 520 yoshida-hxa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3dpbi9HcmFwaGljc0NvbnRl eHRDYWlyb1dpbi5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3Mvd2luL0dyYXBoaWNzQ29udGV4dENhaXJvV2luLmNwcAkocmV2aXNpb24gMTM0MTc0KQor KysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvd2luL0dyYXBoaWNzQ29udGV4dENh aXJvV2luLmNwcAkod29ya2luZyBjb3B5KQpAQCAtODgsNiArODgsOCBAQAogICAgICAgICBzZXRQ bGF0Zm9ybUZpbGxDb2xvcihmaWxsQ29sb3IoKSwgZmlsbENvbG9yU3BhY2UoKSk7CiAgICAgICAg IHNldFBsYXRmb3JtU3Ryb2tlQ29sb3Ioc3Ryb2tlQ29sb3IoKSwgc3Ryb2tlQ29sb3JTcGFjZSgp KTsKICAgICB9CisgICAgaWYgKGNyKQorICAgICAgICBjYWlyb19kZXN0cm95KGNyKTsKIH0KICNl bmRpZgogCg== 174097 2012-11-14 00:50:59 -0800 2012-11-14 09:29:15 -0800 Proposal for fixing the memory leak in WinCairo (Revised patch v2) diff3.txt text/plain 1438 yoshida-hxa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEzNDU0MikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDEyLTExLTE0ICBIaWRla2kg WW9zaGlkYSAgPHlvc2hpZGEtaHhhQG5lY3N0Lm5lYy5jby5qcD4KKworICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzYyMTkKKyAgICAgICAgRml4IGNhaXJv X3QqIG1lbW9yeSBsZWFrIGluIEdyYXBoaWNzQ29udGV4dDo6cGxhdGZvcm1Jbml0CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhpcyBwYXRjaCBpcyB0 byBmaXggYSBtZW1vcnkgbGVhayBwcm9ibGVtIHdoaWNoIG9jY3VycworICAgICAgICBpbiBldmVy eSByZW5kZXJpbmcgcHJvY2VzcyBvbiBXaW5jYWlybyBwb3J0LgorICAgICAgICBCeSBhcHBseWlu ZyB0aGlzIHBhdGNoLCB0aGUgbWVtb3J5IGFsbG9jYXRlZCBpbiBjYWlybyAKKyAgICAgICAgbGli cmFyeSB3aWxsIGJlIHJlbGVhc2VkIGJ5IGNhbGxpbmcgY2Fpcm9fZGVzdHJveS4KKworICAgICAg ICAqIHBsYXRmb3JtL2dyYXBoaWNzL3dpbi9HcmFwaGljc0NvbnRleHRDYWlyb1dpbi5jcHA6Cisg ICAgICAgIChXZWJDb3JlOjpHcmFwaGljc0NvbnRleHQ6OnBsYXRmb3JtSW5pdCk6CisKIDIwMTIt MTEtMTMgIEViZXJoYXJkIEdyYWV0aGVyICA8ZWdyYWV0aGVyQGdvb2dsZS5jb20+CiAKICAgICAg ICAgY2hlY2tib3ggdG8gdG9nZ2xlIEZQUyBjb3VudGVyIGluIHRoZSBpbnNwZWN0b3IncyBzZXR0 aW5ncwoKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3dpbi9HcmFwaGlj c0NvbnRleHRDYWlyb1dpbi5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3Mvd2luL0dyYXBoaWNzQ29udGV4dENhaXJvV2luLmNwcAkocmV2aXNpb24gMTIw OTU2KQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvd2luL0dyYXBoaWNzQ29u dGV4dENhaXJvV2luLmNwcAkod29ya2luZyBjb3B5KQpAQCAtODgsNiArODgsOCBAQAogICAgICAg ICBzZXRQbGF0Zm9ybUZpbGxDb2xvcihmaWxsQ29sb3IoKSwgZmlsbENvbG9yU3BhY2UoKSk7CiAg ICAgICAgIHNldFBsYXRmb3JtU3Ryb2tlQ29sb3Ioc3Ryb2tlQ29sb3IoKSwgc3Ryb2tlQ29sb3JT cGFjZSgpKTsKICAgICB9CisgICAgaWYgKGNyKQorICAgICAgICBjYWlyb19kZXN0cm95KGNyKTsK IH0KICNlbmRpZgogCg==