Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zombie process either by process.kill or pty.destroy #58

Open
hayeah opened this issue Jan 10, 2014 · 5 comments
Open

zombie process either by process.kill or pty.destroy #58

hayeah opened this issue Jan 10, 2014 · 5 comments

Comments

@hayeah
Copy link

hayeah commented Jan 10, 2014

This is the same problem as seen in #8 (which is closed).

The following spawns a bash shell every 3 seconds, and destroys each one after 500ms. It leaves behind a bunch of zombie processes.

PTY = require("pty.js")

console.log "Process: #{process.pid}"

bash = ->
  pty = PTY.spawn("bash",[],{
    name: 'xterm-color'
    cwd: process.cwd()
    env: process.env
  })

  setTimeout (->
    console.log "kill", pty.pid
    # Both of the following leave zombie processes
    # process.kill(pty.pid,"SIGTERM")
    # process.kill(pty.pid,"SIGHUP")
    pty.destroy()
  ), 500

setInterval (-> bash()), 3000
@hhuuggoo
Copy link

I think the problem is chjj/tty.js#111, basically you need to signal(SIGCHLD, SIG_INT), though I have no idea how to do that in node.js, process.on seems to require a function as the second parameter.

the following code works, however it seems to be a bit overkill

Terminal.prototype.kill = function(sig) {
  try {
    process.kill(this.pid, sig || 'SIGTERM');
    waitpid(this.pid);
  } catch(e) {
    ;
  }
};

where waitpid is https://www.npmjs.org/package/waitpid

@chjj
Copy link
Owner

chjj commented Feb 21, 2014

Yes, it's most likely due to the non-handling of SIGCHLD (which is what a zombie is: a child process whos exit never got read by waitpid after a sigchld). Adding our own SIGCHLD handler may conflict with the libuv SIGCHLD handler. It's still in the works. I may just merge it to master and use it for a while to see if there are any major problems.

SIGCHLD discussion: #37

@moul
Copy link

moul commented Oct 6, 2014

Any update on this ?

@EloB
Copy link

EloB commented Oct 15, 2014

+1

@howardroark
Copy link

👍 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants