Last Comment Bug 781380 - [Azure] Memory leak in Azure/Cairo
: [Azure] Memory leak in Azure/Cairo
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Anthony Jones (:kentuckyfriedtakahe, :k17e)
:
: Milan Sreckovic [:milan]
Mentors:
http://ie.microsoft.com/testdrive/Per...
Depends on:
Blocks: 781371
  Show dependency treegraph
 
Reported: 2012-08-08 16:41 PDT by Anthony Jones (:kentuckyfriedtakahe, :k17e)
Modified: 2012-08-29 06:39 PDT (History)
4 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fixed cairo surface memory leak in DrawTargetCairo::CreateSimilarDrawTarget() (983 bytes, patch)
2012-08-08 16:44 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
no flags Details | Diff | Splinter Review
Fixed cairo surface memory leak in DrawTargetCairo::CreateSimilarDrawTarget() v2 (2.66 KB, patch)
2012-08-09 16:02 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
roc: review+
Details | Diff | Splinter Review
Fixed cairo surface memory leak in DrawTargetCairo::DrawSurfaceWithShadow() (819 bytes, patch)
2012-08-14 14:07 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
roc: review+
Details | Diff | Splinter Review

Description Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-08 16:41:38 PDT
Azure/Cairo content leaks substantial amounts of memory upon completion of the maze. This was traced back to cairo surface memory leak in DrawTargetCairo::CreateSimilarDrawTarget() which is called frequently in Canvas code.
Comment 1 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-08 16:44:48 PDT
Created attachment 650363 [details] [diff] [review]
Fixed cairo surface memory leak in DrawTargetCairo::CreateSimilarDrawTarget()
Comment 2 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-08 16:52:41 PDT
Note that Init() calls cairo_surface_reference() so the call to cairo_surface_destroy() cancels that out.
Comment 3 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-08-08 17:19:02 PDT
Can't we just change Init() to not add a reference to the surface?
Comment 4 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-08 17:33:31 PDT
That would be another option but it would change the meaning of Init(). This would mean Factory::CreateDrawTargetForCairoSurface() would need to call cairo_surface_reference() before calling Init(). This would spread the reference counting logic across files.

Avoiding the extra reference/destroy could be achieved by creating an internal version of the Init() function which doesn't call cairo_surface_reference().
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-08-08 18:58:42 PDT
Let's do that then.
Comment 6 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-09 16:02:42 PDT
Created attachment 650711 [details] [diff] [review]
Fixed cairo surface memory leak in DrawTargetCairo::CreateSimilarDrawTarget() v2

Created an internal init method that doesn't call cario_surface_reference()
Comment 7 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-14 14:07:31 PDT
Created attachment 651879 [details] [diff] [review]
Fixed cairo surface memory leak in DrawTargetCairo::DrawSurfaceWithShadow()

Missing cairo_surface_destroy call.
Comment 8 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-14 19:16:28 PDT
https://tbpl.mozilla.org/?tree=Try&rev=09e71b199e6b
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-08-15 17:56:40 PDT
(In reply to Anthony Jones (:kentuckyfriedtakahe) from comment #8)
> https://tbpl.mozilla.org/?tree=Try&rev=09e71b199e6b

This is showing Windows XP gfxASurface leaks, no?
Comment 10 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-22 22:06:10 PDT
https://tbpl.mozilla.org/?tree=Try&rev=f495fefb620a
Comment 11 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-23 15:22:23 PDT
Resubmitted to try without the faulty patch (from another bug). Note that the DrawTargetCairo code path affected by these patches is yet to be enabled on Linux.
Comment 14 Ryan VanderMeulen [:RyanVM] 2012-08-28 17:56:13 PDT
Apparently part of this fix got lost in the original landing. Landed a follow-up for it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/625f746bedf9
Comment 15 Ryan VanderMeulen [:RyanVM] 2012-08-28 18:01:50 PDT
And on aurora too.
https://hg.mozilla.org/releases/mozilla-aurora/rev/02ba8b87cc48
Comment 16 Ed Morley [:emorley] 2012-08-29 06:39:01 PDT
https://hg.mozilla.org/mozilla-central/rev/625f746bedf9

Note You need to log in before you can comment on or make changes to this bug.