Closed Bug 1024223 Opened 5 years ago Closed 5 years ago

PixmanTransform leaks |dest| and |src| if pixman_transform_invert returns false

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: mccr8, Assigned: wlitwinczyk, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, memory-leak, Whiteboard: [MemShrink:P3][CID 1136595][CID 1136596])

Attachments

(1 file, 1 obsolete file)

pixman_image_t* dest = pixman_image_create_bits(aDest->Format() == gfxImageFormat::ARGB32 ? PIXMAN_a8r8g8b8 : PIXMAN_x8r8g8b8,
                                                  destSize.width,
                                                  destSize.height,
                                                  (uint32_t*)aDest->Data(),
                                                  aDest->Stride());
  ...
  pixman_image_t* src = pixman_image_create_bits(aSrc->GetFormat() == SurfaceFormat::B8G8R8A8 ? PIXMAN_a8r8g8b8 : PIXMAN_x8r8g8b8,
                                                 srcSize.width,
                                                 srcSize.height,
                                                 (uint32_t*)aSrc->GetData(),
                                                 aSrc->Stride());
  ...
  if (!pixman_transform_invert(&pixTransformInverted, &pixTransform)) {
    // src and dst leak
    return;
  }
Whiteboard: [MemShrink][CID 1136595][CID 1136596] → [MemShrink:P3][CID 1136595][CID 1136596]
http://mxr.mozilla.org/mozilla-central/source/gfx/layers/basic/BasicLayerManager.cpp#753

Note that BasicCompositor seems to have this same code, except it doesn't leak, so maybe all that is needed is to do the same thing:
  http://mxr.mozilla.org/mozilla-central/source/gfx/layers/basic/BasicCompositor.cpp#215
Mentor: continuation
Assignee: nobody → wlitwinczyk
Attached patch skia_bug_1024223 (obsolete) — Splinter Review
Hi Andrew,

Would you mind seeing if this patch fixes the leak?
Flags: needinfo?(continuation)
This is actually something detected by Coverity static analysis, so I don't have any way to check this myself, but I think based on comment 1 it should be okay, as I think Coverity didn't give a leak warning for the BasicCompositor.
Flags: needinfo?(continuation)
Attachment #8453198 - Attachment is obsolete: true
Attachment #8453318 - Flags: review?(matt.woodrow)
Attachment #8453318 - Flags: review?(matt.woodrow) → review+
https://hg.mozilla.org/mozilla-central/rev/23c1ad883992
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.