Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Blindly pkilling sox is a bad idea #8

Closed
megabytesofrem opened this issue Aug 13, 2020 · 1 comment · Fixed by #24
Closed

Blindly pkilling sox is a bad idea #8

megabytesofrem opened this issue Aug 13, 2020 · 1 comment · Fixed by #24
Labels
enhancement New feature or request feature to implement Fix/feature code is ready and waiting to be implemented

Comments

@megabytesofrem
Copy link
Collaborator

Blindly pkilling sox is a bad idea. Especially since it's possible that a user could have multiple instances of sox running, and when he/she closes Lyrebird then all of the other instances of sox get killed too. An alternative would be to look into terminating the subprocess properly

@megabytesofrem megabytesofrem added the enhancement New feature or request label Aug 13, 2020
@hunternet93
Copy link

When you run subprocess.Popen, it returns a Popen object on which you can call kill() to kill its attached process. Just store a reference in your MainWindow class and add a function to run kill:

class MainWindow:
  def whatever_makes_sox(self):
    self.sox_proc = subprocess.Popen(whatever)

  def cleanup(self):
    if self.sox_prox:
      self.sox_proc.kill()

Then add a global try/except handler wrapping your Gtk.main(), for example:

try:
  Gtk.main()
except BaseException e:
  win.cleanup()

(Catching BaseException means it'll also catch KeyboardInterrupt, SystemExit, and a few others that don't inherit from Exception)

I'm not on my dev machine right now so can't make a PR, but I think that should do it.

@ghost ghost added the feature to implement Fix/feature code is ready and waiting to be implemented label Aug 14, 2020
@ghost ghost closed this as completed in #24 Aug 14, 2020
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request feature to implement Fix/feature code is ready and waiting to be implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants