andyvand / pefile

Automatically exported from code.google.com/p/pefile
Other
0 stars 0 forks source link

mmap.mmap() object is not closed; file handle is leaked. #26

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. mmap not closed and file handle to the parsed file is retained.

What is the expected output? What do you see instead?

Not to leak file handles.  I solved it by have a close() api I would call that 
would close the handle if the binary was mmaped:

  def __init__(self, name=None, data=None, fast_load=None):

        [...]

        self.__mmapped = not data

    def close(self):
        if self.__mmapped:
            self.__data__.close()

See attached patch.

What version of the product are you using? On what operating system?

1.10.2-102 on windows 7.

Please provide any additional information below.

Original issue reported on code.google.com by jandre@gmail.com on 18 Apr 2011 at 9:33

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by ero.carr...@gmail.com on 20 Apr 2011 at 7:34

GoogleCodeExporter commented 9 years ago
Fixed in revision 105. The fix is slightly different to the suggested one but 
achieves the same result

    def close(self):

        if isinstance(self.__data__, mmap.mmap):
            self.__data__.close()

Original comment by ero.carr...@gmail.com on 20 Apr 2011 at 5:29

GoogleCodeExporter commented 9 years ago
thanks for the fixed. there is a related issue i think, if the constructor 
fails due to a parsing issue (e.g., 
    raise PEFormatError('Data length less than expected header length.')) then the object fails to construct so i can't call close() on it.  however, the mmap file handle still lives :(

I think constructor should close any file handles on exception, although not 
sure if there any potential side effects. e.g., in __init__:

        try:
            self.__parse__(name, data, fast_load)
        except:
            self.close()
            raise

thanks.

Original comment by jandre@gmail.com on 23 May 2011 at 11:55

GoogleCodeExporter commented 9 years ago
Thanks for pointing this out. I will take a look.

Original comment by ero.carr...@gmail.com on 24 May 2011 at 8:01

GoogleCodeExporter commented 9 years ago
Fixed in revision 111

Original comment by ero.carr...@gmail.com on 1 Aug 2011 at 7:18