greyblake / vim-preview

Vim plugin for previewing markup files(markdown,rdoc,textile,html)
GNU General Public License v2.0
209 stars 29 forks source link

No zombies #4

Closed guns closed 13 years ago

guns commented 13 years ago

Hi, I switched to your preview plugin from another markdown preview plugin, and I'm having a great time.

Attached is a patch that reaps zombies created by Kernel#fork and uses more standard approaches for shell escaping. The details are in the patch header.

Btw, if you do alot of shelling out from ruby, I suggest you take a look at the shellwords module and the new 1.9 features for system, exec, and spawn. Most of the time though, you can just feed shell params to system and friends as separate params (and not a single string) and be fine.

Cheers Sung

guns commented 13 years ago

Oops, patch should work properly now. I didn't realize Vim changes the values of $stdout and $stderr. In this case, redirecting the STDOUT, STDERR constants is the right thing to do.

greyblake commented 13 years ago

Hi, Sung! Let me thank you for your effort! You are right that zombies should be handled appropriate way. But is it work for you? In my case it doesn't help. A zombie still exists after closing a browser.I spent some time trying to understand why so. As ruby doc says, Process.detach creates new thread and waits until forked process finishes. But main thread finished earlier than browser is closed. So the thread what was created by Process.detach wouldn't handle a zombie, because it will be aborted when the main thread is closed

You can do a simple test in your Vim editor(type this, save a file and do :so %):

ruby <<RUBY_END
Thread.new do
  sleep 1
  system "echo SOMETHING > /tmp/vim_rb.log";
end
RUBY_END

In this case you'll never see tmp/vim_rb.log file. And if you try this: ruby <<RUBY_END Thread.new do sleep 1 system "echo SOMETHING > /tmp/vim_rb.log"; end sleep 2 RUBY_END /tmp/vim_rb.log will be created.

Please let me know, if I'm wrong or if your changes really work for you. By the way my environment is: OS kernel: Linux 2.6.32-5-amd64 Shell: Bash 4.1.5 Vim: 7.3 (with ruby 1.8.7 patch 302).

Thanks. Sergey.

guns commented 13 years ago

But main thread finished earlier than browser is closed. So the thread what was created by Process.detach wouldn't handle a zombie, because it will be aborted when the main thread is closed

Ah, it seems there's a bit I don't understand about Vim's ruby integration. You're right, zombies were still being created. This would not be happening if vim didn't kill the thread running the ruby interpreter, or if it ran ruby commands in a subprocess (where we could just tell it to wait for children to die with Process.wait). I will look into it further and try to get it right!

(I accidentally pushed a commit; please ignore)

greyblake commented 13 years ago

However, thanks:)

guns commented 13 years ago

Okay, one solution: just do what Process#detach would have done by hand. Double fork so that the browser process is no longer a child of vim, and let the first child set up the reaping thread.

Since the first child just forks and detaches the browser process, it will return very quickly and we can feel free to block and reap it via Process#wait.

Tell me what you think!

greyblake commented 13 years ago

closed