Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#3954 closed enhancement (fixed)

GDALDestroyDriverManager() - dataset cleanup

Reported by: Even Rouault Owned by: warmerdam
Priority: normal Milestone: 1.9.0
Component: GDAL_Raster Version: unspecified
Severity: normal Keywords:
Cc:

Description

Quoting gdal-dev

Folks,

I have modified GDALDriverManager::~GDALDriverManager() so that when
it is destroyed it will first close all still-open GDAL datasets.  A debug
message will be issued for each dataset force-closed.

This is part of an effort to ensure a clean shutdown and to make it easier
to detect memory leaks in some situations.

This is not a completely safe operation.  In particular, if there are VRT
files referencing other GDALDataset's the referenced dataset may be cleaned
up before the referencing dataset which would be bad.

I need this primarily for cleanup in MapServer where GDAL datasets are now
normally left open (CLOSE_CONNECTION=DEFER).

If this automatic dataset description causes problems we can disable or
remove it before the GDAL 1.9 release.  But it seems relatively harmless
to incorporate in trunk for the time being.

   http://trac.osgeo.org/gdal/changeset/21662

Best regards,

This is indeed unsafe (a trivial code that opens a VRT and doesn't close it shows it under Valgrind), not only in VRT, but also for NITF (with JPEG compression), TIL, RS2, ERS and BIGGIF drivers (I established the list by finding the drivers that call GDALClose())

I have implemented a safer way by adding a new virtual method, GDALDataset::CloseDependantDatasets(), which is called by GDALDriverManager::~GDALDriverManager() on the list of still opened datasets while there are datasets that still declare dependant datasets. Once all remaining datasets are "stand-alone" they can safely be destroyed.

Change History (7)

comment:1 by Even Rouault, 13 years ago

r21664 /trunk/gdal/ (13 files in 7 dirs): Add a GDALDataset::CloseDependantDatasets() that can be used by GDALDriverManager::~GDALDriverManager() to safely close remaining opened datasets; Use it in VRT, NITF, TIL, RS2, ERS and GIF drivers (#3954)

comment:2 by Even Rouault, 13 years ago

r21665 /trunk/gdal/frmts/ (dimap/dimapdataset.cpp pds/pdsdataset.cpp): PDS and DIMAP drivers also need to implement CloseDependentDatasets(); by the way fix the DIMAP driver so that it doesn't directly attach the raster bands of its dependant images to itself but use a ProxyRasterBand itself (#3954)

comment:3 by Even Rouault, 13 years ago

r21668 /trunk/gdal/frmts/vrt/vrtdataset.cpp: VRT: call FlushCache() in CloseDependentDatasets() to avoid the sources to be removed from the serizalized VRT (#3955)

r21669 /trunk/gdal/gcore/ (gdaldrivermanager.cpp gdalproxypool.cpp): Fix crash that can occur sometimes when ~GDALDriverManager() is called on a VRT referencing a VRT (#3954)

comment:4 by Even Rouault, 13 years ago

Resolution: fixed
Status: newclosed

There were also issues with internal and external overviews/mask bands...

r21674 /trunk/gdal/ (13 files in 10 dirs): Implement CloseDependentDatasets() for GDALDefaultOverviews and GTiffDataset; In VRT, NITF, TIL, RS2, ERS, GIF, PDS and DIMAP, call superclass CloseDependentDatasets() implementation (#3954)

r21677 /trunk/autotest/cpp/ (Makefile testclosedondestroydm.c): Test most corner cases of the CloseDependentDatasets() mechanism (#3954)

Hopefully the issue is now completely addressed...

comment:5 by Even Rouault, 13 years ago

r21688 /trunk/ (2 files in 2 dirs): GDALDestroyDriverManager(): force closing of datasets with reference count > 1 (#3954)

comment:6 by Even Rouault, 13 years ago

r21721 /trunk/ (2 files in 2 dirs): Implement CloseDependentDatasets() on MrSID (#3954)

comment:7 by Even Rouault, 13 years ago

r21723 /trunk/ (3 files in 2 dirs): Implement CloseDependentDatasets() on Rasterlite (#3954)

Note: See TracTickets for help on using tickets.