-1

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)
Deep_fox
  • 405
  • 2
  • 6
  • 14
  • drop `shell=True` -- it creates unnecessary here `cmd.exe` process (`p.kill()` kills that process). – jfs Feb 13 '15 at 21:37
  • related: [subprocess can't successfully restart the targeted python file](http://stackoverflow.com/q/28302081/4279) – jfs Feb 13 '15 at 21:37

1 Answers1

0

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.

aruisdante
  • 8,875
  • 2
  • 30
  • 37
  • Can't agree with you more. Thanks so much. – Deep_fox Feb 13 '15 at 04:00
  • Your mentioned that "At this point, terminated is true, so it starts a new subprocess. ". But since I restart it in the 'finally' clause, should 'terminated==None' in the following 'try' clause? Do you mean that the period is too short to let the program run again such that at that point 'terminated==True'? – Deep_fox Feb 13 '15 at 04:20
  • You aren't restarting the script making those calls, you're restarting its child process, `p`. Anyway, The second `try` block has no bearing on the evaluation of the `if terminated` statement, that is always checked every iteration of the `while` loop. – aruisdante Feb 13 '15 at 04:38