microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.07k stars 29.21k forks source link

Regression: build task does not appear to terminate until user hits return in terminal #37846

Closed gwk closed 6 years ago

gwk commented 6 years ago

Steps to Reproduce:

  1. Create new project directory, with .vscode/tasks.json:
    {
    "version": "2.0.0",
    "echoCommand": false,
    "type": "process",
    "presentation": {},
    "tasks": [
    {
      "label": "build",
      "command": "./build.sh",
      "args": ["${relativeFile}"],
      "group": {"kind": "build", "isDefault": true}
    }
    ]
    }
  2. Create build.sh with executable permissions and contents:
    
    #!/bin/sh

echo $@ sleep 2 echo "DONE."


3. Run the build command on any file (running it on build.sh itself works fine).

The task will print done and presumably terminate. However the VSCode terminal window will not show it as terminated, and if I press the "build" keybinding again I get "The task 'typecheck-current (vscode-task-unterminated)' is already active. To terminate it use `Terminate Task...` from the Tasks menu." However if I press return in the integrated terminal then I will immediately get the expected "Terminal will be reused by tasks, press any key to close it."

If you remove the `sleep 2` then the problem becomes very hard to reproduce. I have occasionally had difficulty reproducing it with the sleep, but by changing the sleep duration to a larger value I can usually trigger the bug. This makes me think that the problem is due to a change in the subprocess polling or something similar.

This regression occurred in the last few days. It is very annoying because now I have to click on the terminal and hit return after every type check.

Reproduces without extensions: Yes
dbaeumer commented 6 years ago

@Tyriar the task system doesn't do anything special here. I listen for corresponding events on the terminal. Does this ring a bell for you? Could there be a timing issue with listeners in the terminal so that the task runner never receives the event.

dbaeumer commented 6 years ago

@gwk when this happens are the corresponding processes still alive (e.g. the bash executing the build.sh). And the tasks.json file you are using looks a little wired as will since you are actually executing a shell command. It should look like this:

{
  "version": "2.0.0",
  "tasks": [
    {
      "label": "build",
      "type": "shell",
      "command": "./build.sh ${relativeFile}",
      "group": {"kind": "build", "isDefault": true}
    }
  ]
}
dbaeumer commented 6 years ago

@Tyriar this sounds very similar to https://github.com/Microsoft/vscode/issues/32276#issuecomment-326044055. Can you please have a look?

gwk commented 6 years ago

@dbaeumer I just verified that the task process does terminate according to ps ax but the task window does not notice until I hit return.

dbaeumer commented 6 years ago

@gwk thanks for the info.

floooh commented 6 years ago

I'm running into exactly same problem since moving my build system wrapper over to Tasks 2.0, this is also on the current MacOS Beta (Windows works fine, Linux so far untested).

The tasks invoke a python script, but VSCode doesn't detect that the task has finished (ps aux shows no running python instances though). Only focusing the Terminal output panel and hitting a key (doesn't have to be Enter) finishes the task properly.

Here's what I'm seeing when a task is done (at this point, no python process is running, but VSCode thinks the task is still running):

screen shot 2017-11-11 at 13 59 29

Then hitting a key, this message appears, only at this point, VSCode thinks the task has finished:

screen shot 2017-11-11 at 14 01 12

An example task (fips is the name of the python script, or on Windows fips.cmd a batch wrapper which calls the python script, calling the command as "python fips ..." doesn't make a difference):

  {
   "problemMatcher":[
    {
     "owner":"cpp",
     "pattern":{
      "severity":4,
      "column":3,
      "file":1,
      "regexp":"^(.*):(\\d+):(\\d+):\\s+(warning|error):\\s+(.*)$",
      "line":2,
      "message":5
     },
     "fileLocation":"absolute"
    }
   ],
   "group":"build",
   "windows":{
    "command":"fips make DDSCubeMap"
   },
   "type":"shell",
   "label":"DDSCubeMap",
   "command":"./fips make DDSCubeMap",
   "presentation":{
    "reveal":"always"
   }
  },
floooh commented 6 years ago

...PS: I also have the impression that I'm only seeing this problem since very recently, but I am not entirely sure.

May be it's related to the latest MacOS Beta? (I only updated about 2 days ago)

dbaeumer commented 6 years ago

The terminal system for some reason thinks the process didn't finish correctly. Not sure why hitting a key makes the terminal move to its final state.

@Tyriar The message Terminal will be reused by tasks, press any key to close it is printed by the terminal service (tasks set this as a onExit value). So the terminal system doesn't detect the exit correctly.

Can you please investigate and see why the terminal doesn't work as expected.

floooh commented 6 years ago

Btw, I just noticed that the official Microsoft C# extension has the same problem on MacOS with the automatically created tasks.json. When running a build with Cmd+Shift+B, VSCode does not detect that the build has finished until a key is pressed. This is on the latest MacOS beta (10.13.2 Beta 17C76a).

chrmarti commented 6 years ago

@dbaeumer I can't reproduce this with the tasks.jsons and build.sh provided and invoking it on Cmd+Shift+B. Is there anything in addition I need to do / set up?

floooh commented 6 years ago

@gwk and I are both on the MacOS 10.13.2 Beta, may be that's the culprit? Since it also happens with .NET Core development and the official C# plugin on the normal 1.18.0 VSCode version I would expect many more bug reports otherwise...

I just tested with Typescript and get the same behaviour, even without a task.json file (just a tsconfig.json). On Cmd+Shift+B I get the dropdown with tsc: build - tsconfig.json and tsc: watch - tsconfig.json.

I select "build", and only this line is printed in the terminal panel:

> Executing task: tsc -p "/Users/floh/projects/altai/tsconfig.json" <

Starting another build gives me the dropdown that the task is already active.

Pressing a key in the terminal panel prints this:

Terminal will be reused by tasks, press any key to close it.

...and the task is detected as finished only then.

chrmarti commented 6 years ago

Thanks, I can reproduce it on MacOS 10.13.2 Beta (17C76a).

In the failure case: node-pty is waiting for 'close' on the terminal object, but that only comes after a key press triggered an 'error' event on the socket.

In the success case: node-pty also waits for 'close' at the same location and in this case that is triggered through the socket's 'close'.

Not sure what could cause this.

dbaeumer commented 6 years ago

@chrmarti thanks for the investigation. Given that this only happens on a Beta of the Mac OS I would not do anything for a recovery build given the above analysis.

gwk commented 6 years ago

Curious, have we verified that this does not happen anywhere except the recent betas? I don't have that ability at the moment.

dbaeumer commented 6 years ago

Yes, @chrmarti was not able to reproduce it on a non beta release but was on the beta 10.13.2. Since we only got reports on the Beta it is plausible to assume it doesn't exist on non beta version.

gwk commented 6 years ago

OK. Just checked and the problem remains for macOS beta 4 (17C79a).

chrmarti commented 6 years ago

@roblourens Checked on 10.13.1 and it did not reproduce. I just checked with VS Code 1.17.2 on MacOS 10.13.2 Beta (17C76a) and it did reproduce.

dbaeumer commented 6 years ago

@Tyriar please see https://github.com/Microsoft/vscode/issues/38824. There the reported claims he sees it on 10.13.0

g-arjones commented 6 years ago

Did anyone have the chance to test this on the beta that was just released yesterday (10.13.2 beta 5)?

gwk commented 6 years ago

Problem persists on 10.13.2 Beta 5 (17C83a).

sstaub commented 6 years ago

Tried with the release of macOS 10.13.2, but the problem persists.

deisner commented 6 years ago

I'm experiencing the same problem, macOS 10.13.2 (now released), VS Code 1.18.1.

dbaeumer commented 6 years ago

@Tyriar can you please have a look. @chrmarti did quite some investigation already

SolanumVexus commented 6 years ago

I can confirm and localize the issue to macOS 10.13.2. Upon installing that update I immediately began to experience this issue.

Tyriar commented 6 years ago

Aaaand I'm back! Starting to look into it now, I upgraded to 10.13.2 and can reproduce.

ZZYSonny commented 6 years ago

I got the same problem on 10.13.2.

Tyriar commented 6 years ago

While I can consistent repro in VS Code, I'm having a bit of trouble reproducing in just node-pty. I had it happening a few times in a row (socket not closing), but then it just started working consistently again 😕

Tyriar commented 6 years ago

Got debugging on the terminal process working, when the keystroke happens the socket is perfectly fine according to nodejs:

screen shot 2017-12-08 at 11 30 41 am

So far it looks like this is either an issue with NodeJS or macOS.

Tyriar commented 6 years ago

Here's the error when enter is pressed:

screen shot 2017-12-08 at 11 37 26 am
Tyriar commented 6 years ago

Here's a summary of what's happening. libuv fires this callback when the process has an exit code, the callback is set here. That callback is called onexit and looks like this:

https://github.com/chjj/pty.js/blob/fe63a412574f45ee6bb6d8fab4a5c102107b5201/lib/pty.js#L108-L117

The problem occurs because it goes into the !self._emittedClose condition as the socket isn't closed yet and waits for it to fire before proceeding with killing the terminal. The problem is, under the latest macOS this just doesn't fire sometimes. I haven't been able to nail down exactly when it happens as it seems very inconsistent.

It's also interesting to note that often (not always) when the socket is closed properly it will close before the onexit callback is called.

My proposed solution is to add a 1 second timeout for the close event to fire, if it doesn't happen then assume something at a lower level went wrong and proceed with killing the terminal.

Tyriar commented 6 years ago

As reported in https://github.com/Microsoft/vscode/issues/39883 it can also happen when running exit from your shell, at which point the terminal's title becomes "kernal_task":

screen shot 2017-12-08 at 1 53 59 pm
Tyriar commented 6 years ago

Upstream issue: https://github.com/Tyriar/node-pty/issues/163 Upstream PR: https://github.com/Tyriar/node-pty/pull/164

Tyriar commented 6 years ago

Cherry picked to release/1.19

dbaeumer commented 6 years ago

@Tyriar thanks for looking into this!

floooh commented 6 years ago

The fix appears to work in the current insider build, many thanks :)

sstaub commented 6 years ago

Also for me. Thank's