Node process is not killed

[albertosantini]

Node process is not terminated when Vim quits (win7).

I think it is due to this change: https://github.com/marijnh/tern_for_vim/commit/c48a908e53b477542a5ff2f29bfe3d6419d47137#L0L83

[marijnh]

Very likely. @clausreinke I guess that was the reason you used the awkward way of terminating the process? Did you find any background on why .terminate() wouldn't work on win? The Python docs suggest it does.

[clausreinke]

See marijnh/tern#97. There were various mismatches between "API available on windows" and "works on windows", not to mention "works the same way on windows". Convenience libraries for working with win32 don't seem to be installed by default. Then there is the intermediate shell process. As I'm no python expert, I worked around by using taskkill to kill the process and childprocess. Btw, my "ceterum censeo" still applies:-) one way to avoid this mess would be more control queries for the tern server, such as reloading project config and project files or terminating the server by query.

[albertosantini]

I resolved with the following code:

  si = subprocess.STARTUPINFO()
  si.dwFlags = subprocess.STARTF_USESHOWWINDOW
  si.wShowWindow = subprocess.SW_HIDE
  proc = subprocess.Popen(vim.eval("g:tern#command") + vim.eval("g:tern#arguments"),
                          cwd=project.dir, env=env,
                          stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
                          shell=False, startupinfo=si)

With shell=False terminate() works nice, but a window is opened. So I modified the startup info to hide that window.

I suppose there is not need now about win = platform.system() == "Windows".

If you like I can provide a PR.

[clausreinke]

Avoiding the shell=True hack would be nice. When I looked into it, the non-hacky ways relied on undocumented non-future-safe features. I recorded some info links here (Grr, github makes it awfully difficult to find comments if they were attached to commits):

https://github.com/marijnh/tern/commit/784b8018906111de31a337c5e7070f10c2c325f1#commitcomment-2846281

Your variant also isn't documented for python v2.7.1, does seem to appear in the subprocess source, but with some constants in _subprocess instead of subprocess. In any case, it is still windows-specific, and its long-term status is unclear (to me, from trying to find authoritative docs).

[marijnh]

Yes, the STARTUPINFO thing is deprecated in 2.7 and gone in 3.x. We can't rely on it. Of course, they don't seem to provide a working alternative, or, generally, care very much about cross-platform API consistency.

Can you try simply restoring the taskkill hack for the windows platform and see if that solves the issue? (I don't have a complete Windows environment set up, so Windows debugging is painful for me.)

[clausreinke]

Since issues are easier to search, here are two related python bugs:

• subprocess module changed exposed attributes
• Mutualize win32 functions

Looks like subprocess, then _subprocess, then _winapi.

[albertosantini]

Sorry misbutton... +1 to restore taskkill solution.

[clausreinke]

The bug reports suggest some kind of system behind those changes, and the python 3 docs still list STARTUPINFO (with an example that doesn't work in 2.7.1).

So I played around a little and found that wrapping selective import in exception handling appears to work

import subprocess,sys

log = open('c:/javascript/tmp/log.txt','a')
log.write (sys.version+"\n")

try:
  log.write ("subprocess"+"\n")
  from subprocess import STARTF_USESHOWWINDOW,SW_HIDE
except ImportError:
  log.write ("_subprocess"+"\n")
  from _subprocess import STARTF_USESHOWWINDOW,SW_HIDE

si = subprocess.STARTUPINFO()
si.dwFlags = STARTF_USESHOWWINDOW
si.wShowWindow = SW_HIDE

...Popen...

with python 3.3.1 (subprocess), 2.7.1 (_subprocess), and 2.6.4 (subprocess). Since I don't have Vims installed that match the various dlls, I had to jump through some hoops to reproduce the extra-console-window issue, so take this with a grain of salt.

[albertosantini]

@clausreinke do you mean 2.7.4, you don't?

[clausreinke]

@albertosantini 2.7.1

[marijnh]

2.7.4 has neither subprocess.STARTUPINFO nor _subprocess, though.

[marijnh]

Oh, wait, of course it doesn't. I'm not on Windows.

[marijnh]

@clausreinke Do you have time to submit a patch that works for you?

[clausreinke]

Having tried this, there are some disadvantages:

• without the shell, the PATH doesn't include tern install location
• one must not point to the npm-generated tern.cmd, either, or one will get an intermediate shell again
• so one needs to point to ["node","<path to tern>"]
• .terminate() still just calls TerminateProcess(), so tern still won't get a chance to cleanup, right? Not sure any more whether there are advantages...

[marijnh]

On Unix, Tern handles SIGTERM and does clean up. But I'm not sure how that maps to windows (both on the node.js and Python side).

I'm okay with just reverting to running taskkill when on windows (calling .terminate() on other platforms). That seemed to work, at least.

[albertosantini]

In windows we have two options: taskkill or Popen with startupinfo.

For shell=True see the warning under Frequently Used Arguments for details: http://docs.python.org/2.7/library/subprocess.html?highlight=popen#frequently-used-arguments Maybe it is not Tern case because the command string is constructed from internal input.

startupinfo for windows is supported in 2.7.5 and in 3.3.2 (see Popen Helpers section).

Googling about to kill a tree of processes (for instance, cmd + tern process), it seems the simplest way is taskkill on windows.

[marijnh]

Please see patch 16493d3336099ec104f2f5a3808c5a67bd928a49 . Wholly untested, but might just be trivial enough to still work.

[albertosantini]

For me it is ok (win 7 64bit / python 2.7.5 / vim 7.3.46).

I tested the patch without a tern project file and with let g:tern#arguments=["--no-port-file"].

For every javascript file , I have a node process. When I quit Vim, all node processes are terminated.

[clausreinke]

@marijnh

On Unix, Tern handles SIGTERM and does clean up. But I'm not sure how that maps to windows (both on the node.js and Python side).

As far as I can tell, TerminateProcess is the most brutal and uncooperative way to end a process, the final step. Killing a process from the Taskmanager tries something more cooperative first, but not POSIX style. Neither python nor nodejs seem to participate in the cooperative protocols(?), so it is just a "hammer over the head", no clean up.

Again, a shutdown server command would bypass such OS-specific fun.

[AndrewRayCode]

Merging in dupe #28 here #fixme

[marijnh]

27 is the dupe. #28 doesn't have anything to do with this one.

[clausreinke]

I grew tired of tern leaving .tern-port files everywhere, so I made a couple of changes to support soft termination:

• in bin/tern, httpServer, add

if (target.pathname == "/exit") process.exit();

• in autoload/tern.vim, tern_killServer, change

urllib2.urlopen("http://localhost:" + str(project.port) + "/exit", "",
                      float(vim.eval("g:tern_request_timeout")));
# subprocess.call("taskkill /t /f /pid " + str(project.proc.pid), shell=True)

That seems to allow tern to shut down properly while clearing away .tern-port, something the hard terminating taskkill on windows didn't allow. Since the same should work cross platform, one might get rid of some platform-testing there as well.

[marijnh]

Fair enough. I guess the windows situation is crappy enough to warrant such a workaround. Will you submit a pull request?

[clausreinke]

I found an even easier way (soft exit tern when its stdin ends). Pull requests: marijnh/tern#242 and marijnh/tern_for_vim#40

[marijnh]

@clausreinke's improvement has been merged and appears to work on both Linux and Windows (and thus most likely also OS X). Closing.

[AndrewRayCode]

I am using tern for vim and I have the merged change:

ack 'stdin=subprocess.PIPE, stdout=subprocess.PIPE' autoload/tern.vim 87: stdin=subprocess.PIPE, stdout=subprocess.PIPE,

however I have over 100 node procs open right now. This is definitely not fixed. Please reopen this issue.

[clausreinke]

@DelvarWorld are you using the matching tern release?

[AndrewRayCode]

I just tried installing the latest version (cloned this repo into bundle, ran npm install inside of it)

Right now I have 9 node procs open. Not sure if it's rising, will report back. But vim is already slowing down.

[AndrewRayCode]

11 node procs open

me@~/.vim/bundle/tern_for_vim (master) $ npm list tern_for_vim@0.4.0 /Users/me/configs/.vim/bundle/tern_for_vim └─┬ tern@0.5.0

Ugh. Does anyone actually use tern for vim successfully? Are they on OSX?

[AndrewRayCode]

Definitely still happening. Please reopen this ticket.

[clausreinke]

@DelvarWorld can't really help you, since this doesn't happen for me and I don't have a Mac. Since closing a process on end of stdin is standard behavior, there is something really odd about your configuration.

On the other hand, that increases your chances of finding the root cause yourself: just start with a small test case, then build up complexity until you either have a working plugin or a small test case that does not work as expected.

Here is a minimal file to test the behavior the patch in question is making use of:

$ cat stdin.js

process.stdin.on("end", function() { process.exit(); });
process.stdin.resume();

setInterval(function(){console.log(process.uptime())},500);

$ echo | node stdin.js

$ cat | node stdin.js
0.6640379000018584
1.1820676000061212
1.7000971999950707

The process prints timings until its stdin ends (piping in a finite input, or typing EOF (CTRL-D, usually) into cat.

If that works as expected, add a python script that starts this process via Popen, like the vim plugin does.

[vincentwoo]

This is actually biting me when trying to run tern dockerized - I think Kubernetes ends stdin to the container, which causes tern to kill itself. I'm not sure if this should be a concern, but it took me quite a long time to debug.