Issue 120105

Summary:

[From Symphony] There is a memory leak in function ScTableSheetObj::PrintAreaUndo_Impl

Product:

performance

Reporter:

ChaoHuang <chao.dev.h>

Component:

code

Assignee:

AOO issues mailing list <issues>

Status:

CLOSED FIXED

QA Contact:

Severity:

Normal

Priority:

CC:

Armin.Le.Grand, 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/sc/source/ui/unoobj/cellsuno.cxx"	chao.dev.h: review?
Extended patch	none

Description ChaoHuang 2012-06-27 13:57:18 UTC

Steps to reproduce the defect:
1) Launch Aoo3.4
2) New a ods file
3) Set "Print Ranges" through menu "Format / Print Ranges / Edit", choose "- user defined -", then give a range
4) Save the ods file, close it
5) Reopen the ods file, close it again

Defect : There is a memory leak in function ScTableSheetObj::PrintAreaUndo_Impl

Comment 1 ChaoHuang 2012-06-27 14:19:06 UTC

The function "ScTableSheetObj::PrintAreaUndo_Impl" is called by these fuctions:
  1)ScTableSheetObj::setPrintAreas
  2)ScTableSheetObj::setPrintTitleColumns
  3)ScTableSheetObj::setTitleColumns
  4)ScTableSheetObj::setPrintTitleRows
  5)ScTableSheetObj::setTitleRows

The argument passed to function "ScTableSheetObj::PrintAreaUndo_Impl" is the same in these five functions. Here is the code snippet.
    ScPrintRangeSaver* pOldRanges = pDoc->CreatePrintRangeSaver();

A new obj will be created on heap in function "ScDocument::CreatePrintRangeSaver". But there is no code to release pOldRanges in these five functions.

In the body of function "ScTableSheetObj::PrintAreaUndo_Impl", there are two places to free obj "pOldRanges".
  1) the "else" branch of "if ( pDocSh )"
     >>> to call "delete pOldRanges" directly
  2) the branch of "if ( pDocSh ) && if (bUndo)"
     >>> to save it in the member data in class ScUndoPrintRange, to free it in the destructor of class ScUndoPrintRange.

So there will be a memory leak about obj "pOldRanges" in the "else" branch of "if (bUndo)". The situation is the same for obj "pNewRanges" in function "ScTableSheetObj::PrintAreaUndo_Impl". The sulution is to free them explicitly.

Comment 2 ChaoHuang 2012-06-27 14:55:50 UTC

Created attachment 78501 [details]
for file "main/sc/source/ui/unoobj/cellsuno.cxx"

Comment 3 Armin Le Grand 2012-06-27 16:19:54 UTC

Created attachment 78504 [details]
Extended patch

ALG: Good catch! No need to create pNewRanges when not in undo, possible to use pOldRanges as flag if used or not and cleanup on central place. Added patch to show this.
It would be even better to use boost::shared_ptr for ScPrintRangeSaver and to derive it from boost::noncopyable (it contains a pointer, when implicitely copied nothing good will happen). Old code, *sigh*.

Comment 4 ChaoHuang 2012-06-27 16:26:00 UTC

(In reply to comment #3)
> Created attachment 78504 [details]
> Extended patch
> 
> ALG: Good catch! No need to create pNewRanges when not in undo, possible to
> use pOldRanges as flag if used or not and cleanup on central place. Added
> patch to show this.
> It would be even better to use boost::shared_ptr for ScPrintRangeSaver and
> to derive it from boost::noncopyable (it contains a pointer, when
> implicitely copied nothing good will happen). Old code, *sigh*.

Thanks for the extended patch ! 
Armin, I learn a lot from you. Thanks!

Comment 5 Armin Le Grand 2012-06-28 09:00:06 UTC

ALG->ChaoHuang: Any more quesitons, else I will commit. Thanks!

Comment 6 ChaoHuang 2012-06-28 22:00:30 UTC

(In reply to comment #5)
> ALG->ChaoHuang: Any more quesitons, else I will commit. Thanks!

hi, Armin
Please go ahead to commit the extended. I tested the code. It's OK. Thanks!

Comment 7 Armin Le Grand 2012-06-29 08:55:47 UTC

ALG->ChaoHuang: Thanks for testing and taking a look. Preparing commit.

Comment 8 Armin Le Grand 2012-06-29 09:20:25 UTC

ALG: Comitted as r1355277. Thanks for the patch!

Comment 9 ChaoHuang 2012-10-17 08:16:30 UTC

Suggest to put it into AOO 3.5.0 release

Comment 10 Yan Ji 2012-11-30 04:46:43 UTC

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