cgoldberg / xvfbwrapper

Manage headless displays with Xvfb (X virtual framebuffer)
Other
295 stars 52 forks source link

Send SIGTERM to all Xvfb processes. #2

Closed kierun closed 8 years ago

kierun commented 11 years ago

There is a dangling process bug in the code as it stands. Try this:   In [1]: from xvfbwrapper import Xvfb   In [2]: vdisplay = Xvfb(width=1280, height=720)   In [3]: vdisplay.start()     => [In another shell, check all Xvfb, you will find there are two. ]   In [4]: vdisplay.stop()     => [Check Xvfb, you will find one left.]

In the code in this commit, there is a group for all processes started by  subprocess.  This group is then send SIGTERM thus killing all the processes started. 

fgimian commented 10 years ago

The current release version doesn't exhibit this problem on my Ubuntu Server 12.04 machine :smile:

Cheers Fotis

cgoldberg commented 8 years ago

This PR is no longer relevant.

you only need to kill a process group if subprocess.Popen is called with shell=true (which it did at the time of your PR). This is because calling terminate() or kill() only terminates the shell, not the actual subprocess spawned. since the code no longer uses shell=True, this is no longer necessary.