Issue 120028

Summary:

[From Symphony] There is a memory leak in function SwWW8ImplReader::MatchSdrBoxIntoFlyBoxItem

Product:

performance

Reporter:

ChaoHuang <chao.dev.h>

Component:

code

Assignee:

AOO issues mailing list <issues>

Status:

CLOSED FIXED

QA Contact:

Severity:

Normal

Priority:

CC:

hdu, issues, liushenf

Version:

AOO 3.4.0

Target Milestone:

4.0.0

Hardware:

All

OS:

All

Issue Type:

DEFECT

Latest Confirmation in:

---

Developer Difficulty:

---

Issue Depends on:

Issue Blocks:

120975, 121366

Attachments:

Description	Flags
for file "main/sw/source/filter/ww8/ww8graf.cxx"	chao.dev.h: review?

Description ChaoHuang 2012-06-19 05:32:18 UTC

Steps to reproduce the defect:
1) Launch Aoo3.4
2) New a odt file, insert a line in it
3) Save it as MS word 97 file, close it
4) Reopen the doc file

Defect : There is a memory leak in function SwWW8ImplReader::MatchSdrBoxIntoFlyBoxItem

Comment 1 ChaoHuang 2012-06-19 05:38:19 UTC

Code snippet:

       for(sal_uInt16 nLine = 0; nLine < 4; ++nLine)
            rBox.SetLine(new SvxBorderLine( aLine ), nLine);

A temporary object typed SvxBorderLine will be created to pass to function "void SvxBoxItem::SetLine( const SvxBorderLine* pNew, sal_uInt16 nLine )", in which the first argument will be used to construct a new object. But this temporary object will not be released.

Comment 2 ChaoHuang 2012-06-19 05:41:20 UTC

Created attachment 78388 [details]
for file "main/sw/source/filter/ww8/ww8graf.cxx"

Comment 3 hdu@apache.org 2012-06-19 09:22:25 UTC

Indeed, SvxBoxItem::SetLine() clones the provided SvxBorderLine object and that leaks. Thanks for finding and solving it!
Applied as revision 1351616.

As a next step one could investigate whether SvxBoxItem::SetLine() should be changed to use the provided SvxBorderLine directly instead of cloning it. This would save two heap operations per line.

Comment 4 hdu@apache.org 2012-06-19 09:38:26 UTC

BTW: The patch has almost no risk and if it saves a significant amount of memory then this issue should become a AOO 3.4.1 candidate.
ChaoHuang: can you give a rough estimate how much memory will be saved in a common scenario?

Comment 5 ChaoHuang 2012-06-20 04:35:08 UTC

(In reply to comment #3)
> Indeed, SvxBoxItem::SetLine() clones the provided SvxBorderLine object and
> that leaks. Thanks for finding and solving it!
> Applied as revision 1351616.
> 
> As a next step one could investigate whether SvxBoxItem::SetLine() should be
> changed to use the provided SvxBorderLine directly instead of cloning it.
> This would save two heap operations per line.

We can also reuse the existing local object. The code should be like this

      for(sal_uInt16 nLine = 0; nLine < 4; ++nLine)
            rBox.SetLine(&aLine, nLine);

The local object aLine with specific info will be passed into function "rBox.SetLine" to clone.

Comment 6 ChaoHuang 2012-06-20 05:17:11 UTC

(In reply to comment #4)
> BTW: The patch has almost no risk and if it saves a significant amount of
> memory then this issue should become a AOO 3.4.1 candidate.
> ChaoHuang: can you give a rough estimate how much memory will be saved in a
> common scenario?

The amount of memory in leaking depends on the number of line object in document. There will be 4*sizeof(SvxBorderLine)=4*12byte=48byte for each line object.

Comment 7 hdu@apache.org 2012-06-20 15:42:35 UTC

I'd say the fix is great to have for the next minor release then but it is not a release blocker for the next micro release.

Comment 8 Yan Ji 2012-11-30 04:47:26 UTC

Since last SVT(r1400866) shows there is no memory leak, so close this defect as resolved.