1

I am running into problems when I attempt to terminate a run a long running process running on a separate thread.

The below is the program. WorkOne creates a subprocess and runs a long running process "adb logcat" that generates log lines. I start WorkOne in main(), wait for 5 sec and attempt to stop it. Multiple runs gives multiple outputs

import threading
import time
import subprocess
import sys

class WorkOne(threading.Thread):

    def __init__(self):
        threading.Thread.__init__(self)
        self.event = threading.Event()  
        self.process = subprocess.Popen(['adb','logcat'], stdout=subprocess.PIPE, stderr=sys.stdout.fileno())      

    def run(self):   
        for line in iter(self.process.stdout.readline,''):            
            #print line
            if self.event.is_set():
                self.process.terminate()
                self.process.kill()
                break;
        print 'exited For'

    def stop(self):
        self.event.set()

def main():

    print 'starting worker1'
    worker1 = WorkOne()
    worker1.start()
    print 'number of threads: ' + str(threading.active_count())
    time.sleep(5)
    worker1.stop()
    worker1.join(5)
    print 'number of threads: ' + str(threading.active_count())

if __name__ == '__main__':
    main()

Sometimes I get [A]:

starting worker1
number of threads: 2
number of threads: 2
exited For

Sometimes I get [B]:

starting worker1
number of threads: 2
number of threads: 1
exited For

Sometimes I get [C]:

starting worker1
number of threads: 2
number of threads: 2

I think I should expect to get [B] all the time. What is going wrong here?

Harkish
  • 2,262
  • 3
  • 22
  • 31
  • 1
    Since you set a timeout in the `join()` call, it is possible to have the thread alive even if it has been stopped (e.g. it is waiting for a new line in the `for` loop). – A. Rodas Mar 18 '13 at 01:57
  • Looks like that was the problem, moved the terminate and kill calls from within the for loop to the stop method and now I get the consistent output that I expect – Harkish Mar 18 '13 at 02:26

2 Answers2

0

Change

       if self.event.is_set():
            self.process.terminate()
            self.process.kill()
            break;

to

        if self.event.is_set():
            self.process.terminate()
            self.process.wait()
            break

The semicolon is a dead giveaway that there is a problem here.

I am guessing that without the wait() the thread sometimes unblocks the work1.join(5) too soon. In those cases, threading.active_count() returns 2.

And, as @A.Rodas says, work1.join(5) should be work1.join() to ensure the join does not unblock until work1 is done.


By the way, I am not sure why you'd ever want to call terminate then kill in succession. On Unix, kill is a more severe form of terminate. On Windows, they are identical. So if you are going to call kill, there is no need to call terminate.

Since you know the program called by subprocess, you should also know if terminate suffices to stop it.

Therefore, you should only need one: either self.process.terminate() or self.process.kill().

unutbu
  • 842,883
  • 184
  • 1,785
  • 1,677
0

I think [B] is only possible if the subprocess takes less than 10 seconds: The main thread sleeps 5 seconds, and after that worker finishes within the 5 seconds timeout of join().

For 10 seconds or more, worker can be alive even after the join() call since it has a timeout argument, which may happen or not. Then you can get [A] (subprocess finishes a few seconds later) or [C] (subprocess finishes much later).

To get always [B], remove the timeout argument of join() so the main thread waits until worker finishes (or make sure you kill the process within 10 seconds by placing the kill call outside of the loop).

A. Rodas
  • 20,171
  • 8
  • 62
  • 72