Bug 82445 – Leak in WebCore::SVGFontElement::create

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: SVG

[Kenichi Ishibashi]

See the stacktrace at http://crbug.com/120453.

r76990 introduced RefPtrs to hold SVGFontElement and looks like it is the cause of the leak. I checked removing RefPtrs fixed the leak and svg/custom/use-multiple-on-nested-disallowed-font.html passed successfully (DRT didn't crash). I think zimmermann's recent changes (e.g. r109333) disallow invalid svg contents like the test and the test is no longer effective.

jschuh@, zimmermann@, Do you think it is safe to remove RefPtrs which were introduces by r76990?

[Kenichi Ishibashi]

Created attachment 134254 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 134254 [details]
Patch

I think they were added on purpose - don't you see any crashes now?? Maybe something has changed, but I'd be careful here.
Can you try running tests under guard malloc? nrwt --tolerance 0 -p svg -g, and/or with --gc-between-tests.

[Kenichi Ishibashi]

(In reply to comment #2)
> (From update of attachment 134254 [details])
> I think they were added on purpose - don't you see any crashes now?? Maybe something has changed, but I'd be careful here.

I didn't see any crashes with this patch, but I agree that we should be careful. I'd like to hear jschuh's opinion.

> Can you try running tests under guard malloc? nrwt --tolerance 0 -p svg -g, and/or with --gc-between-tests.

All svg tests ran as expected with above switches (Debug/Release).

[Tim Horton]

(In reply to comment #0)
> See the stacktrace at http://crbug.com/120453.

I don't see one.

[Justin Schuh]

No, it's not safe to remove the RefPtr without making other changes. This is discussed in bug 66438, along with an explanation of the solution to the memory leak. I just haven't had a chance to do any WebKit work in the last few months.

*** This bug has been marked as a duplicate of bug 66438 ***

[Kenichi Ishibashi]

Comment on attachment 134254 [details]
Patch

Thanks. Removing r?.