Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#4002 closed defect (fixed)

crashes in mrsid driver

Reported by: Even Rouault Owned by: kirk
Priority: normal Milestone:
Component: GDAL_Raster Version: svn-trunk
Severity: normal Keywords:
Cc: Even Rouault, kirk, tamas

Description

Kirk,

r21974 causes autotest/gdrivers/mrsid.py to crash. (note that I've also added a missing variable initialization in r21976)

See following Valgrind traces :

  TEST: mrsid_6 ... ==22387== Invalid read of size 8
==22387==    at 0xD7E7CE0: LizardTech::Jpeg2000ReaderStreamSource::~Jpeg2000ReaderStreamSource() (in /home/even/MRSID8/Raster_DSDK/lib/libltidsdk.so.8)
==22387==    by 0xD7E0E88: LizardTech::J2KImageReader::~J2KImageReader() (in /home/even/MRSID8/Raster_DSDK/lib/libltidsdk.so.8)
==22387==    by 0x72B4C07: MrSIDDataset::~MrSIDDataset() (mrsiddataset.cpp:792)
==22387==    by 0x745249A: GDALClose (gdaldataset.cpp:2388)
==22387==    by 0x6A6C44F: _wrap_delete_Dataset (gdal_wrap.cpp:3841)
==22387==    by 0x4239CF: PyObject_CallFunctionObjArgs (in /usr/bin/python2.6)
==22387==    by 0x6A61634: SwigPyObject_dealloc (gdal_wrap.cpp:1584)
==22387==    by 0x44FC52: ??? (in /usr/bin/python2.6)
==22387==    by 0x46D326: ??? (in /usr/bin/python2.6)
==22387==    by 0x535D12: ??? (in /usr/bin/python2.6)
==22387==    by 0x4A92E6: PyEval_EvalCodeEx (in /usr/bin/python2.6)
==22387==    by 0x4A7808: PyEval_EvalFrameEx (in /usr/bin/python2.6)
==22387==  Address 0x64504f0 is 0 bytes inside a block of size 64 free'd
==22387==    at 0x4C2719F: operator delete(void*) (vg_replace_malloc.c:387)
==22387==    by 0x72B4AF5: MrSIDDataset::~MrSIDDataset() (mrsiddataset.cpp:782)
==22387==    by 0x745249A: GDALClose (gdaldataset.cpp:2388)
==22387==    by 0x6A6C44F: _wrap_delete_Dataset (gdal_wrap.cpp:3841)
==22387==    by 0x4239CF: PyObject_CallFunctionObjArgs (in /usr/bin/python2.6)
==22387==    by 0x6A61634: SwigPyObject_dealloc (gdal_wrap.cpp:1584)
==22387==    by 0x44FC52: ??? (in /usr/bin/python2.6)
==22387==    by 0x46D326: ??? (in /usr/bin/python2.6)
==22387==    by 0x535D12: ??? (in /usr/bin/python2.6)
==22387==    by 0x4A92E6: PyEval_EvalCodeEx (in /usr/bin/python2.6)
==22387==    by 0x4A7808: PyEval_EvalFrameEx (in /usr/bin/python2.6)
==22387==    by 0x4A854F: PyEval_EvalFrameEx (in /usr/bin/python2.6)
==22387== 
pure virtual method called
terminate called without an active exception

Change History (5)

comment:1 by kirk, 13 years ago

Thanks Even for the report and the init correction. That was sloppy of me; I was trying to squeeze it in before leaving the sprint last week.

Based on your valgrind output I have checked in an obvious fix (r22005), but am unable to test it at the moment. Can you try it again?

comment:2 by kirk, 13 years ago

Cc: Even Rouault added

comment:3 by Even Rouault, 13 years ago

Resolution: fixed
Status: newclosed

It worked now for mrsid.py, but when testing more with stuff that involved virtual IO, it crashed in eStat = reader->initialize( poDS->poStream, NULL ). The reason was that the error status for poDS->poStream->open() wasn't tested and this failed. So I test it now just after ltiofs->initialize() and if it fails, it goes through LTIVSIStream().

Fixed in r22006 with tests to test virtual IO. Tested with Geo_DSDK-7.0.0.2167 and GeoDSDKV8 on Linux 64bit.

I'm curious for the reason for the initial change: perhaps because I/O through LTIOFileStream is faster than through LTIVSIStream ?

comment:4 by kirk, 13 years ago

Cc: kirk added

Yes, the reason is that LTIOFileStream uses cached I/O (fopen() et al.), whereas VSI uses... Windows API? The MrSID reading pattern benefits greatly from the cached I/O, so after talking w/ Frank at the sprint we decided the best thing would be to see if the LTIOFileStream could open the string as a file first and then fall back to VSI for non-files (Frank said he did something similar for the ECW driver).

Thanks again for the testing and the additional fixes.

comment:5 by kirk, 13 years ago

Cc: tamas added

Tamas reported 64-bit testing caused a crash. I suspect the old DLL heap issue. I have moved the stream objects to be member variables instead of being allocated from the heap. (r22052)

Note: See TracTickets for help on using tickets.