Issue 120028 - [From Symphony] There is a memory leak in function SwWW8ImplReader::MatchSdrBoxIntoFlyBoxItem
Summary: [From Symphony] There is a memory leak in function SwWW8ImplReader::MatchSdrB...
Status: CLOSED FIXED
Alias: None
Product: performance
Classification: Code
Component: code (show other issues)
Version: AOO 3.4.0
Hardware: All All
: P3 Normal (vote)
Target Milestone: 4.0.0
Assignee: AOO issues mailing list
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 120975 121366
  Show dependency tree
 
Reported: 2012-06-19 05:32 UTC by ChaoHuang
Modified: 2013-02-16 09:18 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
for file "main/sw/source/filter/ww8/ww8graf.cxx" (559 bytes, patch)
2012-06-19 05:41 UTC, ChaoHuang
chao.dev.h: review?
Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
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.