danielhrisca / asammdf

Fast Python reader and editor for ASAM MDF / MF4 (Measurement Data Format) files
GNU Lesser General Public License v3.0
655 stars 226 forks source link

Asammdf keeps file open when it cannot read it correctly #1061

Closed eblis closed 3 months ago

eblis commented 4 months ago

Python version

Asammdf 7.4.2 Python 3.9 and 3.11

Code

MDF version

4.10

Code snippet

try:
  with MDF(file_path) as mdf_obj:
    # ..
except Exception as ex:
  # mdf_obj.close() # can't call this function as mdf_obj is not defined anymore, and was never assigned a value to begin with
  move_file(file_path, "some/other/folder") # permission error, file_path is still in use

Traceback

I don't have a traceback as I debugged this issue on a colleague's machine, but I can show you the call trail.

Description

Asammdf keeps a file open after it has finished attempting to read from it (and failed with an exception), and we cannot access the file to move it even after asammdf object is out of scope. And since the asammdf object is out of scope we cannot manually try to close the file with mdf_obj.close() as mdf_obj isn't defined anymore.

We have an MDF file with 4.10 version, around 2GB in size which throws an exception in MDF.init() which has 2 issues:

  1. The with statement cannot be used for such files, as it never enters the context manager, since the __init__() function crashes before __enter__ has a chance to be called. Since __enter__ is never called, nor will __exit__ be called so there's no way for MDF object to close itself automatically.
  2. The bigger issue I think, the underlying file is kept open by asammdf even after __init__ has crashed with an exception, preventing anyone else from opening the file for writing or from moving the file to another location.

the with MDF(file_path) as mdf_obj: statement calls are like this:

danielhrisca commented 3 months ago

@eblis please try the development branch code

eblis commented 3 months ago

The move operation is still prevented with permission error. I think you need to close more than just the file.

We tried with the following changes and then it started working. Is there any downside to closing everything, calling close() on the MDF object, not only the _file ?

-                except:
-                    if self._file:
-                        self._file.close()
-                    raise
+                except:
+                    self.close()
+                    raise
danielhrisca commented 3 months ago

new commit

eblis commented 3 months ago

This works