OGRSQLiteLayer::Finalize() - checking for NULL before all CPLFree

[mj10777]

Due a problem, that in the end I caused myself, I was getting the horrible

Invalid free() / delete / delete[] / realloc()

errors.

I this case it was 'pszFIDColumn' had been freed and not set to NULL;

the crash came at the end of ogr_spatialite_8, where as warned, OGRSQLiteLayer::Finalize can run more than once.

==31043== Invalid free() / delete / delete[] / realloc()
==31043==    at 0x4C2BE10: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31043==    by 0x8053421: VSIFree (cpl_vsisimple.cpp:673)
==31043==    by 0x8375F60: OGRSQLiteLayer::Finalize() (ogrsqlitelayer.cpp:116)
==31043==    by 0x8375E03: OGRSQLiteLayer::~OGRSQLiteLayer() (ogrsqlitelayer.cpp:84)
==31043==    by 0x83877A7: OGRSQLiteVectorLayer::~OGRSQLiteVectorLayer() (ogrsqlitevectorlayer.cpp:89)
==31043==    by 0x8387350: OGRSQLiteTableLayer::~OGRSQLiteTableLayer() (ogrsqlitetablelayer.cpp:62)
==31043==    by 0x838737F: OGRSQLiteTableLayer::~OGRSQLiteTableLayer() (ogrsqlitetablelayer.cpp:65)
==31043==    by 0x8366B1E: OGRSQLiteDataSource::~OGRSQLiteDataSource() (ogrsqlitedatasource.cpp:251)
==31043==    by 0x8366C4F: OGRSQLiteDataSource::~OGRSQLiteDataSource() (ogrsqlitedatasource.cpp:262)
==31043==    by 0x7F9806C: GDALClose (gdaldataset.cpp:2757)
==31043==    by 0x835A44F: OGRReleaseDataSource (ogrsfdriverregistrar.cpp:161)
==31043==    by 0x1541A60A: delete_OGRDataSourceShadow (ogr_wrap.cpp:3448)
==31043==    by 0x1541A60A: _wrap_delete_DataSource (ogr_wrap.cpp:6458)

after adding to OGRSQLiteLayer::Finalize:

    if( pszFIDColumn != NULL )
    {
     CPLFree( pszFIDColumn );
     pszFIDColumn = NULL;
    }
    if( panFieldOrdinals != NULL )
    {
     CPLFree( panFieldOrdinals );
     panFieldOrdinals = NULL;
    }
    if( papszCompressedColumns != NULL )
    {
     CSLDestroy(papszCompressedColumns);
     papszCompressedColumns = NULL;
    }

and setting to NULL (after the unneeded free) resolved this problem.

Also after removing the unneeded free the error did not occur.

Severity=blocker while developing at least.

[Even Rouault]

Your change doesn't make sense to me. CPLFree(NULL) is perfectly fine and is a no-op, so adding a if( not_null ) test before doesn't change anything. I don't get any Valgrind errors when running ogr_sqlite.py on trunk.

[mj10777]

I didn't make sense to me either, but ogr_spatialite_8 was failing because of it.

In Initialize(..) I had a

CPLFree( pszFIDColumn );

and ogr_spatialite_8 was failing with a double free error.

The tests before that ran without error, so why it happened in ogr_spatialite_8 I cannot say.

The valgind reports ogrsqlitelayer.cpp:116 as the cause.

[Even Rouault]

I'm afraid you'll have to dig up more. It must hide something else.

[Even Rouault]

Removing obsolete milestone

[Even Rouault]

Closing. This must be an artifact of code specific to the developer branch