Why subprocess can't successfully kill the old running process?

Question

I run a program test.py. Since it collapses frequently, I import subprocess to restart it when it stops. Sometimes I found subprocess can't successfully restart it. Hence, I force the program to restart every 60 minutes. But I find that there sometimes two test.py processing running simutanously. What's wrong with my code and how to fix it? I use windows 7 OS. Plz check the following codes and thanks in advance:

import subprocess
import time
from datetime import datetime

p = subprocess.Popen(['python.exe', r'D:\test.py'], shell=True)
minutes = 1
total_time = 0
while True:
    now = datetime.now()

    #periodly restart
    total_time += 1
    if total_time % 100 == 0:
        try:
            p.kill()
        except Exception as e:
            terminated = True
        finally:
            p = subprocess.Popen(['python.exe', r'D:\test.py'], shell=True)

    #check and restart if it stops
    try:
        terminated = p.poll()
    except Exception as e:
        terminated = True
    if terminated:
        p = subprocess.Popen(['python.exe', r'D:\test.py'], shell=True)
    time.sleep(minutes * 60)

Answer 1

While I don't agree at all with your design, the specific problem is here:

except Exception as e:
        terminated = True
finally:
        p = subprocess.Popen(['python.exe', r'D:\test.py'], shell=True)

In the case that an Exception was thrown, you're setting terminated to true, but then immediately restarting the subprocess. Then, later, you check:

if terminated:
    p = subprocess.Popen(['python.exe', r'D:\test.py'], shell=True)

At this point, terminated is true, so it starts a new subprocess. However, it already had done that in the finally block.

Really, what you should do is simply not bother restarting it during the kill attempt:

    try:
        p.kill()
    except Exception:
        # We don't care, just means it was already dead
        pass
    finally:
        # Either the process is dead, or we just killed it. Either way, need to restart
        terminated = True

Then your if terminated clause will properly restart the process and you won't have a duplicate.