Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#3045 closed defect (fixed)

Memory leak when exporting to MapInfo

Reported by: ryanrino Owned by: chaitanya
Priority: normal Milestone:
Component: OGR_SF Version: unspecified
Severity: normal Keywords: mitab
Cc: warmerdam, Daniel Morissette

Description

This memory leak cause an out of memory error in my application, which exports millions of polygons. I have attached the file containing the error. The fix is included. Briefly, in \ogr\ogrsf_frmts\mitab_feature.cpp the pointer poStylePart below is not deleted if i>0.

for(i=0; i<numParts; i++) {

poStylePart = poStyleMgr->GetPart?(i);

if(poStylePart->GetType?() == OGRSTC...) {

break;

}

}

also

if(i >= numParts)

return;

causes exits without deleting poStylePart and poStyleMgr

Attachments (1)

mitab_feature.cpp (320.7 KB) - added by ryanrino 11 years ago.
This a modified version of the file at the tip, as the file existed when last changed at revision 15756

Download all attachments as: .zip

Change History (7)

Changed 11 years ago by ryanrino

Attachment: mitab_feature.cpp added

This a modified version of the file at the tip, as the file existed when last changed at revision 15756

comment:1 Changed 11 years ago by warmerdam

Cc: warmerdam Daniel Morissette added
Keywords: mitab added
Owner: changed from warmerdam to chaitanya

Chaitanya,

Could you review and confirm the memory leak and confirm that the fix is appropriate. Once you are happy with the fix you can bounce it back to me for upstreaming into mitab.

comment:2 Changed 11 years ago by ryanrino

In my fix I forgot to set the pointers to NULL, which could cause a crash:

if(poStylePart->GetType??() == OGRSTC...) {

break;

} else {

delete poStylePart; poStylePart = NULL; otherwise we could get a crash.

}

comment:3 Changed 11 years ago by chaitanya

Status: newassigned

ryanrino,

Can you provide a sample dataset that is causing the problem? I couldn't find any problem with anything I tested until now.

comment:4 Changed 10 years ago by chaitanya

Resolution: fixed
Status: assignedclosed

The patch shouldn't cause any problems. Since this is untested, I am limiting this to the trunk. (r20164)

comment:5 Changed 10 years ago by aboudreault

chaitanya, ryanrino, I've nothing right now to test... but I'm looking at the code and it seems to me that all the "delete poStylePart;" statements inside the "if(i >= numParts)" blocks are unnescessary and could cause problems. If we enter in that if block, that means we passed through all the style parts and nothing was found. The else inside the for deletes the poStylePart every loop if it's not the one we are looking for. Is my understanding ok or I'm missing something?

comment:6 Changed 10 years ago by aboudreault

After a quick look... although the instruction delete ptr;, which ptr is NULL is considered "safe". I've removed it from the patch still it was not necessary.

Note: See TracTickets for help on using tickets.