Opened 17 years ago

Closed 16 years ago

#1696 closed defect (fixed)

Memory leaks in MITAB driver

Reported by: rmatsu Owned by: Daniel Morissette
Priority: normal Milestone: 1.5.0
Component: OGR_SF Version: svn-trunk
Severity: critical Keywords: MITAB
Cc: warmerdam

Description

Using C# wrapper, when creating new tab files in a loop, memory usage climbs continuously.

Change History (3)

comment:1 by warmerdam, 17 years ago

Cc: warmerdam added
Milestone: 1.5.0
Owner: changed from warmerdam to Daniel Morissette

Richard (by email) also suggested this patch:

Index: /ogr/ogrsf_frmts/mitab/mitab_feature.cpp
===================================================================
--- /ogr/ogrsf_frmts/mitab/mitab_feature.cpp (revision 11729)
+++ /ogr/ogrsf_frmts/mitab/mitab_feature.cpp (working copy)
@@ -7813,6 +7813,12 @@
         }
     }
 
+ if (poStyleMgr)
+  delete poStyleMgr;
+
+ if (poStylePart)
+  delete poStylePart;
+
     return;
 }
 
@@ -7997,7 +8003,13 @@
         SetBrushFGColor((GInt32)nBrushColor);
     }
 
-     return;
+ if (poStyleMgr)
+  delete poStyleMgr;
+
+ if (poStylePart)
+  delete poStylePart;
+
+    return;
    
 } 
 
@@ -8281,6 +8293,14 @@
         nSymbolColor = strtol(pszSymbolColor, NULL, 16);
         SetSymbolColor((GInt32)nSymbolColor);
     }
+
+ if (poStyleMgr)
+ {
+  delete poStyleMgr;
+ }
+
+ if (poStylePart)
+  delete poStylePart;
    
     return;
    
    
Index: /ogr/ogrsf_frmts/mitab/mitab_imapinfofile.cpp
===================================================================
--- /ogr/ogrsf_frmts/mitab/mitab_imapinfofile.cpp (revision 11728)
+++ /ogr/ogrsf_frmts/mitab/mitab_imapinfofile.cpp (working copy)
@@ -281,6 +281,7 @@
     TABPoint *poTABPointFeature = NULL;
     TABRegion *poTABRegionFeature = NULL;
     TABPolyline *poTABPolylineFeature = NULL;
+ const char *styleString = NULL;
 
     /*-----------------------------------------------------------------
      * MITAB won't accept new features unless they are in a type derived
@@ -300,11 +301,11 @@
        *------------------------------------------------------------*/
       case wkbPoint:
         poTABFeature = new TABPoint(poFeature->GetDefnRef());
-        if(poFeature->GetStyleString())
+  styleString = poFeature->GetStyleString();
+        if(styleString)
         {
             poTABPointFeature = (TABPoint*)poTABFeature;
-            poTABPointFeature->SetSymbolFromStyleString(
-                poFeature->GetStyleString());
+            poTABPointFeature->SetSymbolFromStyleString(styleString);
         }
         break;
       /*-------------------------------------------------------------
@@ -313,14 +314,13 @@
       case wkbPolygon:
       case wkbMultiPolygon:
         poTABFeature = new TABRegion(poFeature->GetDefnRef());
-        if(poFeature->GetStyleString())
+  styleString = poFeature->GetStyleString();
+        if(styleString)
         {
             poTABRegionFeature = (TABRegion*)poTABFeature;
-            poTABRegionFeature->SetPenFromStyleString(
-                poFeature->GetStyleString());
+            poTABRegionFeature->SetPenFromStyleString(styleString);
 
-            poTABRegionFeature->SetBrushFromStyleString(
-                poFeature->GetStyleString());
+            poTABRegionFeature->SetBrushFromStyleString(styleString);
         }
         break;
       /*-------------------------------------------------------------
@@ -329,11 +329,11 @@
       case wkbLineString:
       case wkbMultiLineString:
         poTABFeature = new TABPolyline(poFeature->GetDefnRef());
-        if(poFeature->GetStyleString())
+  styleString = poFeature->GetStyleString();
+        if(styleString)
         {
             poTABPolylineFeature = (TABPolyline*)poTABFeature;
-            poTABPolylineFeature->SetPenFromStyleString(
-                poFeature->GetStyleString());
+            poTABPolylineFeature->SetPenFromStyleString(styleString);
         }
         break;
       /*-------------------------------------------------------------
@@ -381,6 +381,8 @@
         eErr = OGRERR_FAILURE;
 
     delete poTABFeature;
+ if (styleString != NULL)
+  CPLFree((void *)styleString);
    
     return eErr;
 }

comment:2 by Daniel Morissette, 16 years ago

Status: newassigned

I have applied the mitab_feature.cpp patch upstream in MITAB (1.6.4) and will try to backport in time for the GDAL 1.5.0 release.

With respect to the mitab_imapinfofile.cpp patch, I am not convinced that it is correct. The poFeature->GetStyleString() returns a reference to an internal buffer that is freed when the OGRFeature is destroyed, so freeing it again is invalid. Unless you can provide a testcase to reproduce this leak then I am going to drop the mitab_imapinfofile.cpp patch.

comment:3 by Daniel Morissette, 16 years ago

Resolution: fixed
Status: assignedclosed

The upgrade of the MITAB driver in r13312 (GDAL 1.5.0-beta2) included the patch for mitab_feature.cpp.

Marking this ticket fixed.

Note: See TracTickets for help on using tickets.