Opened 20 years ago

Last modified 17 years ago

#520 closed defect (later)

[OGR] Leak of OGRFeatureDefn in ogrfeature.cpp

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

Description (last modified by warmerdam)

I found a leak of OGRFeatureDefn while working on MITAB #283:
http://www.maptools.org/bugzilla/show_bug.cgi?id=283

It seems that the leak would be caused by OGRFeature::~OGRFeature() which does a
Dereference() but doesn't delete the object if the feature count is zero.  This
works fine as long as the featureDefn is also owned by a currently opened layer
and that the layer is destroyed only after the last feature is destroyed, but in
cases where the featureDefn is solely owned by the OGRFeature then there is a
possibility of a leak.

I have applied the patch locally in the MITAB copy of ogrfeature.cpp but didn't
apply it to the master (in GDAL) because I would like you to review the fix and
make sure it won't have any side-effects with other OGR code that assumed that a
feature never destroyed the featuredefn:

--- ogr/ogrfeature.cpp  24 Jul 2003 04:33:29 -0000      1.2
+++ ogr/ogrfeature.cpp  12 Mar 2004 18:18:52 -0000
@@ -164,8 +164,6 @@
 OGRFeature::~OGRFeature()

 {
-    poDefn->Dereference();
-
     if( poGeometry != NULL )
         delete poGeometry;

@@ -200,6 +198,9 @@

     CPLFree( pauFields );
     CPLFree(m_pszStyleString);
+
+    if (poDefn->Dereference() == 0)
+        delete poDefn;
 }

Change History (2)

comment:1 by warmerdam, 20 years ago

Daniel, 

As we have discussed a few years ago, I should have had layers taking
an explicit reference to features.  Then we could have made your suggested
change safely, but as things stand too much code is implemented the other way
around.  

So, I will defer this bug report in the hopes of revisiting it.  Perhaps when
I go to restructure OGR I could address this as well. 

comment:3 by warmerdam, 17 years ago

Description: modified (diff)
Priority: highnormal

This was fixed in the OGRFeatureDefn reference counting overhaul a year or so ago.

Note: See TracTickets for help on using tickets.