vstinner / hachoir

Hachoir is a Python library to view and edit a binary stream field by field
http://hachoir.readthedocs.io/
GNU General Public License v2.0
613 stars 70 forks source link

guess.py:createParser() failure leaves FileInputStream open (Version 3.1.2) #71

Closed patricklcam closed 2 years ago

patricklcam commented 2 years ago

Calling guess.py:createParser() with an invalid file type leaves open FileInputSteam object Code is:

     stream = FileInputStream(filename, real_filename, tags=tags)
     return guessParser(stream)

Probably should be something like:

    stream = FileInputStream(filename, real_filename, tags=tags)
    guess = guessParser(stream)
    if not guess:
        stream.close()
    return guess
vstinner commented 2 years ago

Do you want to propose a PR?

patricklcam commented 2 years ago

I don't really have any github experience. Let me investigate how to create a PR and link to this issue. I will get back to you in a day or so.

patricklcam commented 2 years ago

Poked around for a bit and created a PR from a forked branch.

ticketsvl commented 1 year ago

I had a file permission bug on Windows (linux is good) and found guess.py is responsible.

PermissionError: [WinError 32] The process cannot access the file because it is being used by another process:

I didn't find how to close file stream (if guess is valid), so I patched exact same part of code to return stream and close it in my main code. That fixed permission bug for me. return guess, stream

parser_data, parser_stream = createParser(mypath)
metadata = extractMetadata(parser_data)
sec = metadata.get('duration').seconds
width = metadata.get('width')
height = metadata.get('height')
parser_stream.close()

Not sure how to fix this and keep back cap, so @vstinner should to decide. If is there a better way to close stream without patching guess.py return, let me know. Regards.

vstinner commented 1 year ago

Did you see the commit https://github.com/vstinner/hachoir/commit/f536652459c3741da1acf6535517fd69ab9922aa ? It's part of Hachoir version 3.1.3.

ticketsvl commented 1 year ago

Yes, I saw it. And I had an error with this commit and all updates by pip. I could make a new bugreport, but it looks like, it's the same bug as in topic. Commit hadn't fixed it for all cases.

See my code above. Originally it had one return value

parser_data = createParser(mypath)
metadata = extractMetadata(parser_data)

File stream remains open and this code will cause PermissionError in Windows(!) py interpreter, if later code part will try to access same file.

vstinner commented 1 year ago

parser_data = createParser(mypath) metadata = extractMetadata(parser_data)

I don't see guessParser() being called explicitly. Please open a new issue with more details.