sylikc / pyexiftool

PyExifTool (active PyPI project) - A Python library to communicate with an instance of Phil Harvey's ExifTool command-line application. Runs one process with special -stay_open flag, and pipes data to/from. Much more efficient than running a subprocess for each command!
Other
161 stars 21 forks source link

ExifToolHelper Hangs in pytest, in wine #63

Open OdyX opened 1 year ago

OdyX commented 1 year ago

Trying to debug a pyexiftool issue on Windows, to which I don't have access, I'm basically trying to run this, in tobix/pywine:


def test_exiftool_works_on_all_images():
    with ExifToolHelper(executable="./exiftool.exe") as et:
        for d in et.get_metadata("./test.tiff"):
             assert len(d) > 0

    # Also test ExifTool.execute_json
    with ExifTool(executable="./exiftool.exe") as eftl:
        d = eftl.execute_json("./test.tiff")
        assert len(d) > 0

I run this in the following command:

xvfb-run sh -c "wine pytest"

With various prints and pytest options, I can see that the get_metadata call works, and that assert len(d) > 0 passes, but the code never goes past the end of the with block. In other words, it hangs in .terminate(), and never goes to test ExifTool.

Now. If I add a import pdb; pdb.set_trace() as first line in the function, and hit c when prompted, it works. What am I doing wrong?

sylikc commented 1 year ago

Ok. I also have an issue with running in pytest on actual Windows. I think this might be related to the bug I reported to CPython more than a year ago. It's still open... https://github.com/python/cpython/issues/87950

You can try putting print statements in terminate() and see if it happens at self._process.communicate()

OdyX commented 1 year ago

Let's see if I understand the code in terminate() correctly.

  1. At the end of a with ExifTool(): block, first, __exit__() is called, which calls terminate(_del=False) (_del defaults to False); hence through exiftool.py#L839-L851 I can confirm it blocks at line 850. There, on Windows, this seems it can be rewritten in a simpler way, as we want to kill the process in all cases;
    try:
        self._process.communicate(input=b"-stay_open\nFalse\n", timeout=timeout)  # this is a constant sequence specified by PH's exiftool
    except subprocess.TimeoutExpired:  # this is new in Python 3.3 (for python 2.x, use the PyPI subprocess32 module)
        pass
    # Kill it in all cases
    self._process.kill()
  2. Then self._flag_running_false() runs, unsets self._running, and terminates
  3. Finally, __del__() runs, but as self._running has been unset, doesn't call terminate() at all.

In my tests, with the above simple patch, I see no lurking processes in Linux nor Windows.

sylikc commented 1 year ago

I'll try this next week. But from what I remember, there bug manifests itself during del or something. I had some tests cases that caused PyExifTool to hang. I think they're still in the test code.