781380 - [Azure] Memory leak in Azure/Cairo

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

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 (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Fixed cairo surface memory leak in DrawTargetCairo::CreateSimilarDrawTarget()

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

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)]

Can't we just change Init() to not add a reference to the surface?

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

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)]

Let's do that then.

Comment 6

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: 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 (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Fixed cairo surface memory leak in DrawTargetCairo::DrawSurfaceWithShadow()

Missing cairo_surface_destroy call.

Comment 8

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

https://tbpl.mozilla.org/?tree=Try&rev=09e71b199e6b

Comment 9

[Ryan VanderMeulen [:RyanVM]]

(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 (:ajones, :kentuckyfriedtakahe, :k17e)]

https://tbpl.mozilla.org/?tree=Try&rev=f495fefb620a

Comment 11

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

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 12

[Ryan VanderMeulen [:RyanVM]]

(In reply to Anthony Jones (:kentuckyfriedtakahe) from comment #10)
> https://tbpl.mozilla.org/?tree=Try&rev=f495fefb620a

Green on Try.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c97a0ffcf500
https://hg.mozilla.org/integration/mozilla-inbound/rev/e137f28dfe70

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c97a0ffcf500
https://hg.mozilla.org/mozilla-central/rev/e137f28dfe70

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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]]

And on aurora too.
https://hg.mozilla.org/releases/mozilla-aurora/rev/02ba8b87cc48

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/625f746bedf9