Opened 17 years ago
Closed 14 years ago
#1540 closed defect (fixed)
Minor memory leaks in gmlreader.cpp
Reported by: | rogerjames99 | Owned by: | chaitanya |
---|---|---|---|
Priority: | normal | Milestone: | 1.7.0 |
Component: | OGR_SF | Version: | 1.4.0 |
Severity: | normal | Keywords: | ogr gml xerces |
Cc: | warmerdam, epifanio |
Description (last modified by )
I have fixed some minor memory leaks in gmlreader.cpp. The xerces runtime was not being cleaned up properly on exit so I have added a call to XMLPlatformUtils::Terminate(); in the GMLReader class destructor. Also some strings in GMLReader::SetupParser were not being freed, so I have enclosed them in std::auto_ptr(s). The relevant files modded against the 1.4.0 release are attached. These leaks are trivial but can bite someone making protracted use of the library without reloading.
Attachments (7)
Change History (17)
by , 17 years ago
Attachment: | gmlreader.cpp added |
---|
by , 17 years ago
Attachment: | gmlreaderp.h added |
---|
comment:2 by , 17 years ago
Cc: | added |
---|---|
Description: | modified (diff) |
Milestone: | → 1.5.0 |
Owner: | changed from | to
Priority: | lowest → low |
Version: | unspecified → 1.4.0 |
comment:3 by , 16 years ago
Status: | new → assigned |
---|
by , 16 years ago
Attachment: | gmlreader-minor-leak-fix.patch added |
---|
This is regular patch including fixes proposed in attached big gmlreaderp.h and gmlreader.cpp files. This patch is to help to review proposed changes.
by , 16 years ago
Attachment: | gml-leaks-new-fix-mloskot.patch added |
---|
This is new improved version of proposed patch. The original version is buggy, because it's forbidden to use std::auto_ptr with buffers allocated by memory manager used in XMLString due to delete / delete[] mismatch.
by , 16 years ago
Simple test presenting the proposed patch in action. Running this test result in segmentation fault because internal state of the reader is broken. As Frank predicted, the solution is to track number of XML readers. This test uses road.gml copied to road1.gml and road2.gml. Attached Makefile is used to compile the test.
by , 16 years ago
Sample GML file. Second copy 'road2.gml' is needed by the test.cpp program.
comment:4 by , 16 years ago
It's important to note that only first run of ./test program throws segmentation fault. Next, .gfs files are available and further executions do not crash.
comment:5 by , 16 years ago
Keywords: | xerces added |
---|
comment:6 by , 16 years ago
Milestone: | 1.5.0 → 1.5.1 |
---|
comment:7 by , 16 years ago
Summary: | Minor memory leaks in gmlreader.cpp (fixed) → Minor memory leaks in gmlreader.cpp |
---|
comment:8 by , 14 years ago
Milestone: | 1.5.4 → 1.7.0 |
---|---|
Owner: | changed from | to
Priority: | low → normal |
Status: | assigned → new |
Chaitanya,
Please review proposed changes, and whether the driver has any memory leaks (via valgrind) before 1.7 release.
comment:9 by , 14 years ago
Cc: | added |
---|
Chaitanya,
your commit in r18188 broke compilation when Xerces is not available.
libtool: compile: g++ -g -O2 -Wall -I.. -I../.. -I/Users/sasha/gdal/port -I/Users/sasha/gdal/gcore -I/Users/sasha/gdal/alg -I/Users/sasha/gdal/ogr -I/Users/sasha/gdal/ogr/ogrsf_frmts -I/usr/local/include -DOGR_ENABLED -I/Users/sasha/gdal/port -I/Users/Shared/source/epylib/local/include -I/Library/Frameworks/UnixImageIO.framework/unix -I/Library/Frameworks/UnixImageIO.framework/unix/include -I/Users/Shared/source/epylib/local/include -I/Users/Shared/source/epylib/local/include -I -I/include -DHAVE_XERCES=0 -DHAVE_EXPAT -c gmlreader.cpp -fno-common -DPIC -o ../o/.libs/gmlreader.o gmlreader.cpp: In destructor 'virtual GMLReader::~GMLReader()': gmlreader.cpp:140: error: 'XMLPlatformUtils' has not been declared make[3]: *** [../o/gmlreader.lo] Error 1 make[2]: *** [gml-target] Error 2 make[1]: *** [sublibs] Error 2 make: *** [ogr-target] Error 2 MacBook-Pro-15-di-Massimo-Di-Stefano:gdal sasha$
XMLPlatformUtils::Terminate(); should be protected by #if HAVE_XERCES == 1
comment:11 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Further testing on valgrind using the python interpreter with the autotest scripts ogr_gml_read.py and ogr_gml_geom.py and the attached test.cpp showed no memory leaks.
Mateusz,
I'd like you to review these changes and apply them if they seem appropriate.
Note, please ensure that the XMLPlatformUtils::Terminate() does not interfere with other GMLReader's that may be active at the same time. So try tests where two GML datastores are open at the same time. Close one, and then verify you can still read from the other.
I suspect the patch will have to be changed to keep track of how many GMLReader's are active, and only call the Terminate() when the last one is destroyed. Presumably that logic should also set m_bXercesInitialized to FALSE.
Since this is low priority and a bit risky as a change, I'd like to hold it to trunk, not the stable branch.