mottosso / allzpark

Package-based application launcher for VFX and games production
https://allzpark.com
GNU Lesser General Public License v3.0
184 stars 38 forks source link

Fix deadlock when subprocess doesn't end with EOF #119

Closed davidlatwe closed 3 years ago

davidlatwe commented 3 years ago

This PR is for fixing the same problem what #104 have, but with another solution.

Problem

Deadlock occurred when we use stdout.readline to listen to the subprocess that doesn't end with EOF.

I found that my Allzpark will freeze when closing the application that it just launched, IF Allzpark was launched from a Rez resolved context. And the freeze was because the stdout.readline keep sending empty line b"" due to no EOF signal.

So why there has no EOF emitted from subprocess when Allzpark is launched from a resolved context ? I haven't figure it out. :( It seems like cmd.exe in subprocess is not able to send EOF, not sure.

Here's a simple reproducible :

>>> import subprocess
>>> popen = subprocess.Popen(["start", "cmd"], shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
>>> for line in iter(popen.stdout.readline, ""):
...     print(line)
...     # now close the terminal window
...
b''
b''
b''
b''
# endless empty line

Nevertheless, I came up a solution that seems solid. 🤔

I have changed the stderr listening function to act like Popen.communicate(), since I think stderr doesn't need to be streamed.

buffer = []
buffer.append(stderr.read())
# subprocess ended
stderr.close()

(Referenced from Python's subprocess.py)

With that we can know when the subprocess is ended, and send our own EOF flag to the stdout listening thread to make it stop . Problem solved.

mottosso commented 3 years ago

This PR is for fixing the same problem what #104 have, but with another solution.

Oh, did the other solution not work in the end?

So why there has no EOF emitted from subprocess when Allzpark is launched from a resolved context ? I haven't figure it out.

My guess would be it has to do with the resolved environment running in either cmd.exe or powershell.exe. One of those should probably yield a different result, and I bet it isn't an issue on Linux.

davidlatwe commented 3 years ago

Oh, did the other solution not work in the end?

Somehow it still works, but not broadly tested and I still don't understand why it works. For example, in #104

        for line in iter(self.popen.stdout.readline, ""):
            if not line:
                continue
            ...

Still not knowing why continuing the loop when the line is empty can avoid the infinite empty line deadlock. (Update: Might have some clue now)

And this PR, at least I know how it works.

and I bet it isn't an issue on Linux.

But at the time when I opened #104, I was on Mac, launching Allzpark in Rez resolved context. :P (Update: The issue was not related to EOF, see #120)