Re: Anyone interested in fining a mem leak for hire?
James Burgess <[email protected]> Thu, 28 Sep 2017 16:43:44 -0700
| Newsgroups | gmane.comp.video.openexr.devel |
|---|---|
| Message-ID | <[email protected]> |
--===============6522772734522268652==
Content-Type: multipart/alternative; boundary="Apple-Mail=_4B734F2A-D0F8-4C35-A885-E02194CACF04"
--Apple-Mail=_4B734F2A-D0F8-4C35-A885-E02194CACF04
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
charset=utf-8
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 =3D 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=E2=80=99th read because EOF is found.=20
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=E2=80=99re not then =E2=80=9Cthe file is probably incomplete=E2=80=9D=
. If so it takes some corrective action and sets its =E2=80=9Ccomplete=E2=80=
=9D flag to false. So it seems like the author meant to handle your =
error case. I don=E2=80=99t know the code well enough to know should it =
try to catch the InputExc exception right here?=20
Also =E2=80=9Ccorrective action=E2=80=9D 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=E2=80=99ve 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 =E2=80=9C-1=E2=80=9D 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=E2=80=99t mean =
to? Any way, if someone would like to comment on what the =E2=80=9Cright =
thing=E2=80=9D to do here at a high level I=E2=80=99d submit a patch.
Cheers,
- James
> On Sep 28, 2017, at 9:37 AM, Schoenberger <[email protected]> wrote:
>=20
> Hi
> =20
> As the topic says, is anyone interested in fixing a memory leak in =
openEXR?
> =20
> The issue can be reproduced by running this command
> file =3D new Imf::RgbaInputFile(filename);
> =20
> with this unfinished file:
> www.binaryalchemy.de/ftp_file/Exr-memleak.zip =
<http://www.binaryalchemy.de/ftp_file/Exr-memleak.zip>
> =20
> =20
> regards,
> Holger Sch=C3=B6nberger
> technical director
> The day has 24 hours, if that does not suffice, I will take the night
> _______________________________________________
> Openexr-devel mailing list
> [email protected] <mailto:[email protected]>
> https://lists.nongnu.org/mailman/listinfo/openexr-devel =
<https://lists.nongnu.org/mailman/listinfo/openexr-devel>
--Apple-Mail=_4B734F2A-D0F8-4C35-A885-E02194CACF04
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html;
charset=utf-8
<html><head><meta http-equiv=3D"Content-Type" content=3D"text/html =
charset=3Dutf-8"></head><body style=3D"word-wrap: break-word; =
-webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" =
class=3D"">The problem is that (this is using the 2.2 source) that =
InputFile::initialize() (ImfInputFile.cpp:533) does a new on a =
ScanLineInputFile:<div class=3D""><br class=3D""></div><div =
class=3D""> _data->sFile =3D =
new ScanLineInputFile (_data->header,<br class=3D""> =
=
=
_data->_streamData->is,<br class=3D""> =
=
=
_data->numThreads);</div><div =
class=3D""><br class=3D""></div><div class=3D"">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.</div><div class=3D""><br class=3D""></div><div class=3D"">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=E2=80=99th read because EOF is found. </div><div class=3D""><br =
class=3D""></div><div class=3D"">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=E2=80=99re not then =E2=80=9Cthe =
file is probably incomplete=E2=80=9D. If so it takes some corrective =
action and sets its =E2=80=9Ccomplete=E2=80=9D flag to false. So it =
seems like the author meant to handle your error case. I don=E2=80=99t =
know the code well enough to know should it try to catch the InputExc =
exception right here? </div><div class=3D""><br class=3D""></div><div=
class=3D"">Also =E2=80=9Ccorrective action=E2=80=9D 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=E2=80=99ve 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 =E2=80=9C-1=E2=80=9D the call to =
seekg() would throw.</div><div class=3D""><br class=3D""></div><div =
class=3D"">Given this state of things it feels like something bad =
happened to this chunk of code. Maybe someone took something out they =
didn=E2=80=99t mean to? Any way, if someone would like to comment on =
what the =E2=80=9Cright thing=E2=80=9D to do here at a high level I=E2=80=99=
d submit a patch.</div><div class=3D""><br class=3D""></div><div =
class=3D"">Cheers,</div><div class=3D"">- James</div><div class=3D""><br =
class=3D""></div><div class=3D""><br class=3D""></div><div class=3D""><br =
class=3D""></div><div class=3D""><div><blockquote type=3D"cite" =
class=3D""><div class=3D"">On Sep 28, 2017, at 9:37 AM, Schoenberger =
<<a href=3D"mailto:[email protected]" =
class=3D"">[email protected]</a>> wrote:</div><br =
class=3D"Apple-interchange-newline"><div class=3D""><div =
class=3D"WordSection1" style=3D"page: WordSection1; font-family: =
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: =
normal; font-weight: normal; letter-spacing: normal; orphans: auto; =
text-align: start; text-indent: 0px; text-transform: none; white-space: =
normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: =
0px;"><div style=3D"margin: 0cm 0cm 0.0001pt; font-size: 11pt; =
font-family: Calibri, sans-serif;" class=3D"">Hi<o:p =
class=3D""></o:p></div><div style=3D"margin: 0cm 0cm 0.0001pt; =
font-size: 11pt; font-family: Calibri, sans-serif;" class=3D""><o:p =
class=3D""> </o:p></div><div style=3D"margin: 0cm 0cm 0.0001pt; =
font-size: 11pt; font-family: Calibri, sans-serif;" class=3D"">As the =
topic says, is anyone interested in fixing a memory leak in openEXR?<o:p =
class=3D""></o:p></div><div style=3D"margin: 0cm 0cm 0.0001pt; =
font-size: 11pt; font-family: Calibri, sans-serif;" class=3D""><o:p =
class=3D""> </o:p></div><div style=3D"margin: 0cm 0cm 0.0001pt; =
font-size: 11pt; font-family: Calibri, sans-serif;" class=3D"">The issue =
can be reproduced by running this command<o:p class=3D""></o:p></div><div =
style=3D"margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: =
Calibri, sans-serif;" class=3D"">file =3D new =
Imf::RgbaInputFile(filename);<o:p class=3D""></o:p></div><div =
style=3D"margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: =
Calibri, sans-serif;" class=3D""><o:p class=3D""> </o:p></div><div =
style=3D"margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: =
Calibri, sans-serif;" class=3D"">with this unfinished file:<o:p =
class=3D""></o:p></div><div style=3D"margin: 0cm 0cm 0.0001pt; =
font-size: 11pt; font-family: Calibri, sans-serif;" class=3D""><a =
href=3D"http://www.binaryalchemy.de/ftp_file/Exr-memleak.zip" =
style=3D"color: rgb(149, 79, 114); text-decoration: underline;" =
class=3D"">www.binaryalchemy.de/ftp_file/Exr-memleak.zip</a><o:p =
class=3D""></o:p></div><div style=3D"margin: 0cm 0cm 0.0001pt; =
font-size: 11pt; font-family: Calibri, sans-serif;" class=3D""><span =
lang=3D"EN-US" class=3D""><o:p class=3D""> </o:p></span></div><div =
style=3D"margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: =
Calibri, sans-serif;" class=3D""><span lang=3D"EN-US" class=3D""><o:p =
class=3D""> </o:p></span></div><div style=3D"margin: 0cm 0cm =
0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" =
class=3D""><span lang=3D"EN-US" class=3D"">regards,<o:p =
class=3D""></o:p></span></div><div style=3D"margin: 0cm 0cm 0.0001pt; =
font-size: 11pt; font-family: Calibri, sans-serif;" class=3D""><span =
lang=3D"EN-US" style=3D"font-size: 10pt; font-family: Arial, =
sans-serif;" class=3D"">Holger Sch=C3=B6nberger<br class=3D""></span><span=
lang=3D"EN-US" style=3D"font-size: 7.5pt; font-family: Arial, =
sans-serif;" class=3D"">technical director<br class=3D"">The day has 24 =
hours, if that does not suffice, I will take the night</span><span =
lang=3D"EN-US" class=3D""><o:p class=3D""></o:p></span></div></div><span =
style=3D"font-family: Helvetica; font-size: 12px; font-style: normal; =
font-variant-caps: normal; font-weight: normal; letter-spacing: normal; =
orphans: auto; text-align: start; text-indent: 0px; text-transform: =
none; white-space: normal; widows: auto; word-spacing: 0px; =
-webkit-text-stroke-width: 0px; float: none; display: inline =
!important;" =
class=3D"">_______________________________________________</span><br =
style=3D"font-family: Helvetica; font-size: 12px; font-style: normal; =
font-variant-caps: normal; font-weight: normal; letter-spacing: normal; =
orphans: auto; text-align: start; text-indent: 0px; text-transform: =
none; white-space: normal; widows: auto; word-spacing: 0px; =
-webkit-text-stroke-width: 0px;" class=3D""><span style=3D"font-family: =
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: =
normal; font-weight: normal; letter-spacing: normal; orphans: auto; =
text-align: start; text-indent: 0px; text-transform: none; white-space: =
normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; =
float: none; display: inline !important;" class=3D"">Openexr-devel =
mailing list</span><br style=3D"font-family: Helvetica; font-size: 12px; =
font-style: normal; font-variant-caps: normal; font-weight: normal; =
letter-spacing: normal; orphans: auto; text-align: start; text-indent: =
0px; text-transform: none; white-space: normal; widows: auto; =
word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=3D""><a =
href=3D"mailto:[email protected]" style=3D"color: rgb(149, 79, =
114); text-decoration: underline; font-family: Helvetica; font-size: =
12px; font-style: normal; font-variant-caps: normal; font-weight: =
normal; letter-spacing: normal; orphans: auto; text-align: start; =
text-indent: 0px; text-transform: none; white-space: normal; widows: =
auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" =
class=3D"">[email protected]</a><br style=3D"font-family: =
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: =
normal; font-weight: normal; letter-spacing: normal; orphans: auto; =
text-align: start; text-indent: 0px; text-transform: none; white-space: =
normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: =
0px;" class=3D""><a =
href=3D"https://lists.nongnu.org/mailman/listinfo/openexr-devel" =
style=3D"color: rgb(149, 79, 114); text-decoration: underline; =
font-family: Helvetica; font-size: 12px; font-style: normal; =
font-variant-caps: normal; font-weight: normal; letter-spacing: normal; =
orphans: auto; text-align: start; text-indent: 0px; text-transform: =
none; white-space: normal; widows: auto; word-spacing: 0px; =
-webkit-text-stroke-width: 0px;" =
class=3D"">https://lists.nongnu.org/mailman/listinfo/openexr-devel</a></di=
v></blockquote></div><br class=3D""></div></body></html>=
--Apple-Mail=_4B734F2A-D0F8-4C35-A885-E02194CACF04--
--===============6522772734522268652==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
Openexr-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/openexr-devel
--===============6522772734522268652==--