openexr-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Openexr-devel] Anyone interested in fining a mem leak for hire?


From: James Burgess
Subject: Re: [Openexr-devel] Anyone interested in fining a mem leak for hire?
Date: Thu, 28 Sep 2017 16:43:44 -0700

The problem is that (this is using the 2.2 source) that InputFile::initialize() (ImfInputFile.cpp:533) does a new on a ScanLineInputFile:

            _data->sFile = new ScanLineInputFile (_data->header,
                                                  _data->_streamData->is,
                                                  _data->numThreads);

This constructor is going to throw because of the incomplete input file, but the catch is going to happen one up from here in the InputFile constructor (ImfInputFile.cpp:385). The catch handler will delete _data but _data->sFile will, of course, be null so the half-constructed ScanLineInputFile is not deleted. The whole ScanLineInputFile is leaked.

Looking at ScanLineInputFile constructor we see it first allocates its buffers (the major source of the leaked byte count) and then calls readLineOffsets(). That routine seems to be looking out for incomplete files but does not try to catch any exceptions. It throws on the 1067’th read because EOF is found. 

The "looking out for incomplete files" that readLineOffsets() does is a loop to check the offsets are sane and non-zero and the comment says if they’re not then “the file is probably incomplete”. If so it takes some corrective action and sets its “complete” flag to false. So it seems like the author meant to handle your error case. I don’t know the code well enough to know should it try to catch the InputExc exception right here? 

Also “corrective action” routine reconstructLineOffsets would fail and throw itself if it were called. It tries to get the current file position (via tellg()) but this will return -1 since we’ve already read past the end of the file. Then continues to try and re-read the offsets from the current file position which we already know to be at the end so that would also fail. Finally, it would actually cause an exception because it tries to restore the file position, since it was “-1” the call to seekg() would throw.

Given this state of things it feels like something bad happened to this chunk of code. Maybe someone took something out they didn’t mean to? Any way, if someone would like to comment on what the “right thing” to do here at a high level I’d submit a patch.

Cheers,
- James



On Sep 28, 2017, at 9:37 AM, Schoenberger <address@hidden> wrote:

Hi
 
As the topic says, is anyone interested in fixing a memory leak in openEXR?
 
The issue can be reproduced by running this command
file = new Imf::RgbaInputFile(filename);
 
with this unfinished file:
 
 
regards,
Holger Schönberger
technical director
The day has 24 hours, if that does not suffice, I will take the night
_______________________________________________
Openexr-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/openexr-devel


reply via email to

[Prev in Thread] Current Thread [Next in Thread]