Skip to content

Commit

Permalink
fix memory leak from constructor exceptions (#653)
Browse files Browse the repository at this point in the history 
* fix another memory leak from constructor exceptions

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>
  • Loading branch information
@peterhillman
peterhillman committed Feb 6, 2020
1 parent 88246d9 commit 51bd0ff
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 90 deletions.
78 changes: 52 additions & 26 deletions OpenEXR/IlmImf/ImfScanLineInputFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1107,8 +1107,6 @@ newLineBufferTask (TaskGroup *group,

void ScanLineInputFile::initialize(const Header& header)
{
try
{
_data->header = header;

_data->lineOrder = _data->header.lineOrder();
Expand Down Expand Up @@ -1159,15 +1157,6 @@ void ScanLineInputFile::initialize(const Header& header)
_data->linesInBuffer) / _data->linesInBuffer;

_data->lineOffsets.resize (lineOffsetSize);
}
catch (...)
{
if (_data->partNumber == -1)
delete _streamData;
delete _data;
_data=NULL;
throw;
}
}


Expand All @@ -1182,8 +1171,24 @@ ScanLineInputFile::ScanLineInputFile(InputPartData* part)

_data->version = part->version;

initialize(part->header);

try
{
initialize(part->header);
}
catch(...)
{
if (!_data->memoryMapped)
{
for (size_t i = 0; i < _data->lineBuffers.size(); i++)
{
EXRFreeAligned(_data->lineBuffers[i]->buffer);
_data->lineBuffers[i]->buffer=nullptr;
}
}

delete _data;
throw;
}
_data->lineOffsets = part->chunkOffsets;

_data->partNumber = part->partNumber;
Expand All @@ -1206,19 +1211,40 @@ ScanLineInputFile::ScanLineInputFile
_streamData->is = is;
_data->memoryMapped = is->isMemoryMapped();

initialize(header);

//
// (TODO) this is nasty - we need a better way of working out what type of file has been used.
// in any case I believe this constructor only gets used with single part files
// and 'version' currently only tracks multipart state, so setting to 0 (not multipart) works for us
//

_data->version=0;
readLineOffsets (*_streamData->is,
_data->lineOrder,
_data->lineOffsets,
_data->fileIsComplete);
try
{

initialize(header);

//
// (TODO) this is nasty - we need a better way of working out what type of file has been used.
// in any case I believe this constructor only gets used with single part files
// and 'version' currently only tracks multipart state, so setting to 0 (not multipart) works for us
//

_data->version=0;
readLineOffsets (*_streamData->is,
_data->lineOrder,
_data->lineOffsets,
_data->fileIsComplete);
}
catch(...)
{
if(_data)
{
if (!_data->memoryMapped)
{
for (size_t i = 0; i < _data->lineBuffers.size(); i++)
{
EXRFreeAligned(_data->lineBuffers[i]->buffer);
_data->lineBuffers[i]->buffer=nullptr;
}
}
}
delete _streamData;
delete _data;
throw;
}
}


Expand Down
154 changes: 90 additions & 64 deletions OpenEXR/IlmImf/ImfTiledInputFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -713,44 +713,47 @@ TiledInputFile::TiledInputFile (const char fileName[], int numThreads):
IStream* is = 0;
try
{
is = new StdIFStream (fileName);
readMagicNumberAndVersionField(*is, _data->version);
try
{
is = new StdIFStream (fileName);
readMagicNumberAndVersionField(*is, _data->version);

//
// Backward compatibility to read multpart file.
//
if (isMultiPart(_data->version))
{
compatibilityInitialize(*is);
return;
}
//
// Backward compatibility to read multpart file.
//
if (isMultiPart(_data->version))
{
compatibilityInitialize(*is);
return;
}

_data->_streamData = new InputStreamMutex();
_data->_streamData->is = is;
_data->header.readFrom (*_data->_streamData->is, _data->version);
initialize();
//read tile offsets - we are not multipart or deep
_data->tileOffsets.readFrom (*(_data->_streamData->is), _data->fileIsComplete,false,false);
_data->_streamData->currentPosition = _data->_streamData->is->tellg();
}
catch (IEX_NAMESPACE::BaseExc &e)
{
if (_data->_streamData)
_data->_streamData = new InputStreamMutex();
_data->_streamData->is = is;
_data->header.readFrom (*_data->_streamData->is, _data->version);
initialize();
//read tile offsets - we are not multipart or deep
_data->tileOffsets.readFrom (*(_data->_streamData->is), _data->fileIsComplete,false,false);
_data->_streamData->currentPosition = _data->_streamData->is->tellg();
}
catch (IEX_NAMESPACE::BaseExc &e)
{
delete _data->_streamData->is;
_data->_streamData->is = is = 0;
delete _data->_streamData;
REPLACE_EXC (e, "Cannot open image file "
"\"" << fileName << "\". " << e.what());
throw;
}
delete _data;
if (is != 0)
delete is;

REPLACE_EXC (e, "Cannot open image file "
"\"" << fileName << "\". " << e.what());
throw;
}
catch (...)
{
if (!_data->memoryMapped)
{
for (size_t i = 0; i < _data->tileBuffers.size(); i++)
{
if(_data->tileBuffers[i])
{
delete [] _data->tileBuffers[i]->buffer;
}
}
}
if ( _data->_streamData != 0)
{
delete _data->_streamData->is;
Expand Down Expand Up @@ -778,38 +781,45 @@ TiledInputFile::TiledInputFile (OPENEXR_IMF_INTERNAL_NAMESPACE::IStream &is, int

try
{
readMagicNumberAndVersionField(is, _data->version);

//
// Backward compatibility to read multpart file.
//
if (isMultiPart(_data->version))
try
{
compatibilityInitialize(is);
return;
}
readMagicNumberAndVersionField(is, _data->version);

streamDataCreated = true;
_data->_streamData = new InputStreamMutex();
_data->_streamData->is = &is;
_data->header.readFrom (*_data->_streamData->is, _data->version);
initialize();
// file is guaranteed to be single part, regular image
_data->tileOffsets.readFrom (*(_data->_streamData->is), _data->fileIsComplete,false,false);
_data->memoryMapped = _data->_streamData->is->isMemoryMapped();
_data->_streamData->currentPosition = _data->_streamData->is->tellg();
}
catch (IEX_NAMESPACE::BaseExc &e)
{
if (streamDataCreated) delete _data->_streamData;
delete _data;
//
// Backward compatibility to read multpart file.
//
if (isMultiPart(_data->version))
{
compatibilityInitialize(is);
return;
}

REPLACE_EXC (e, "Cannot open image file "
"\"" << is.fileName() << "\". " << e.what());
throw;
streamDataCreated = true;
_data->_streamData = new InputStreamMutex();
_data->_streamData->is = &is;
_data->header.readFrom (*_data->_streamData->is, _data->version);
initialize();
// file is guaranteed to be single part, regular image
_data->tileOffsets.readFrom (*(_data->_streamData->is), _data->fileIsComplete,false,false);
_data->memoryMapped = _data->_streamData->is->isMemoryMapped();
_data->_streamData->currentPosition = _data->_streamData->is->tellg();
}
catch (IEX_NAMESPACE::BaseExc &e)
{
REPLACE_EXC (e, "Cannot open image file "
"\"" << is.fileName() << "\". " << e.what());
throw;
}
}
catch (...)
{
if (!_data->memoryMapped)
{
for (size_t i = 0; i < _data->tileBuffers.size(); i++)
{
delete [] _data->tileBuffers[i]->buffer;
}
}
if (streamDataCreated) delete _data->_streamData;
delete _data;
throw;
Expand All @@ -833,13 +843,29 @@ TiledInputFile::TiledInputFile (const Header &header,
// we have somehow got the header.
//

_data->_streamData->is = is;
_data->header = header;
_data->version = version;
initialize();
_data->tileOffsets.readFrom (*(_data->_streamData->is),_data->fileIsComplete,false,false);
_data->memoryMapped = is->isMemoryMapped();
_data->_streamData->currentPosition = _data->_streamData->is->tellg();
try
{
_data->_streamData->is = is;
_data->header = header;
_data->version = version;
initialize();
_data->tileOffsets.readFrom (*(_data->_streamData->is),_data->fileIsComplete,false,false);
_data->memoryMapped = is->isMemoryMapped();
_data->_streamData->currentPosition = _data->_streamData->is->tellg();
}
catch(...)
{
if (!_data->memoryMapped)
{
for (size_t i = 0; i < _data->tileBuffers.size(); i++)
{
delete [] _data->tileBuffers[i]->buffer;
}
}
delete _data->_streamData;
delete _data;
throw;
}
}


Expand Down

0 comments on commit 51bd0ff

Please sign in to comment.