tasks.onDidStartTaskProcess gives the wrong processId if there is a "dependsOn" task

[ejizba]

Issue Type: Bug

1. Add this code to a simple "Hello World" VS Code extension in the activate method (and add "*" as an activationEvent to make sure it activates):

   vscode.tasks.onDidStartTaskProcess(e => {
       vscode.window.showInformationMessage(`Task Name: "${e.execution.task.name}" Process ID: "${e.processId}"`);
   });

2. Start debugging the extension
3. In the "Extension Development Host", add this tasks.json to your workspace

   {
       "version": "2.0.0",
       "tasks": [
           {
               "label": "taskToDependOn",
               "type": "shell",
               "command": "sleep 3",
               "problemMatcher": []
           },
           {
               "label": "Run this task",
               "type": "shell",
               "command": "sleep 3",
               "problemMatcher": [],
               "dependsOn": "taskToDependOn"
           }
       ]
   }

4. Do "Run Task" from the command palette and select "Run this task" as the task
   

Expected Result:

I will see two info messages, with different pid's.

Actual Result:

I see two info messages, with the same pid - I'm guessing it's the pid from "taskToDependOn":

Important Notes:

1. This consistently happens the first time I run the task for that session of VS Code. If I run the task again, sometimes it has the same problem and sometimes it works
2. This issue does not repro for me on v1.53 of VS Code. It must've been introduced in v1.54
3. This also repro-ed for me on Windows - I used Start-Sleep -Seconds 3 to get the task to work in PowerShell

VS Code version: Code 1.54.1 (f30a9b7, 2021-03-04T22:42:18.719Z)
OS version: Darwin x64 20.3.0

System Info

Item	Value
CPUs	Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz (12 x 2600)
GPU Status	2d_canvas: enabled gpu_compositing: enabled metal: disabled_off multiple_raster_threads: enabled_on oop_rasterization: enabled opengl: enabled_on protected_video_decode: unavailable_off rasterization: enabled skia_renderer: disabled_off_ok video_decode: enabled webgl: enabled webgl2: enabled
Load (avg)	3, 3, 3
Memory (System)	16.00GB (0.06GB free)
Process Argv	--crash-reporter-id e2c60431-fb89-41a2-b9ed-11e53cee83ac
Screen Reader	no
VM	0%

[alexr00]

@Tyriar for this process ID tasks has always just used the processId on the ITerminalInstance. When re-using terminals as tasks does, would you expect them to have the same process id, but only sometimes?

[ejizba]

You can tell this isn't related to re-using terminals by changing the sleep command above to something like sleep 10 & ps | grep sleep

Here is the process ids we're given:

But here is the task output:

It started a new process and the old process (43931) is no longer running.

[alexr00]

Looks like something is out of sequence in the integrated terminal. Tasks just uses the process ID from terminal.
I've created a branch to illustrate this: alexr00/incorrectTerminalPid
See where the logs are in that branch: 1ed71d3

I would expect that the pid is set in the terminal before the terminal event that tasks uses fires. However, it's clear that's not happening.

To repro, checkout my branch and run from source.
Open a folder and add the tasks listed in the original issue:

{
    "version": "2.0.0",
    "tasks": [
        {
            "label": "taskToDependOn",
            "type": "shell",
            "command": "sleep 3",
            "problemMatcher": []
        },
        {
            "label": "Run this task",
            "type": "shell",
            "command": "sleep 3",
            "problemMatcher": [],
            "dependsOn": "taskToDependOn"
        }
    ]
}

Run the "Run this task" task.

Console output only the first time the task is run:

[ejizba]

To help with your investigation, this was first reported to us on February 8th "with the most recent update to the Insiders version" (although we didn't realize at the time what was going on). Something changed between v1.53 and that date to cause this regression

[ejizba]

@fiveisprime @chrisdias @alexr00 @Tyriar @meganrogge what are the chances this will make it out in a recovery build for VS Code? Matt implied he was trying to get this included with v1.54.2, but that obviously didn't happen. This was a regression in the v1.54 release that completely broke C# debugging for the Azure Functions extension, the same week we announced .NET 5 support in conjunction with the platform.

We shipped a workaround last week that seems to fix the issue on our end, but let's just say I'm not super confident in it. Basically, we check if the processId we got from onDidStartTaskProcess is running and use vscode.window.activeTerminal.processId if it isn't. In v1.53, the processId is running and we're good, but in v1.54 we usually fall back to activeTerminal.processId and I feel like that's not guaranteed to be the right id. Y'all are welcome to comment on if that workaround is good enough for now, but I'd still feel much better if VS Code just fixed the root cause regression ASAP.

[Tyriar]

@ejizba sorry about this regression, must have slipped through with the major refactors that happened to bring in persistent terminal processes. I'll investigate tomorrow morning.

The workaround seems like it should work, the only concern is that window.activeTerminal is the active terminal which may not be the one of the task? If you can use any signals to verify that it would be more solid, verifying the name of the terminal should be an easy way to do this I think.

[ejizba]

This is fixed for me on the latest insiders, thanks!

Just to confirm - this missed the recovery releases and will be going out with 1.55?

[Tyriar]

@ejizba good to hear 🙂 yes it'll be going in 1.55, it sounds like the workaround is solid enough in the meantime. Again sorry about the hassle. When you remove the workaround for 1.55 you'll also want to bump the vscode engine version in package.json so people who haven't updated VS Code won't hit issues.

[alexr00]

Verified that this issue is fixed, but now I get some extra console errors:

  ERR Could not find pty with id "5": Error: Could not find pty with id "5"
    at PtyService._throwIfNoPty (d:\repos\Microsoft\vscode\out\vs\platform\terminal\node\ptyService.js:180:23)
    at PtyService.shutdown (d:\repos\Microsoft\vscode\out\vs\platform\terminal\node\ptyService.js:101:25)
    at Object.call (d:\repos\Microsoft\vscode\out\vs\base\parts\ipc\common\ipc.js:813:39)
    at Server.onPromise (d:\repos\Microsoft\vscode\out\vs\base\parts\ipc\common\ipc.js:253:35)
    at Server.onRawMessage (d:\repos\Microsoft\vscode\out\vs\base\parts\ipc\common\ipc.js:226:33)
    at d:\repos\Microsoft\vscode\out\vs\base\parts\ipc\common\ipc.js:171:73
    at Emitter.fire (d:\repos\Microsoft\vscode\out\vs\base\common\event.js:479:38)
    at process.fn (d:\repos\Microsoft\vscode\out\vs\base\common\event.js:257:44)
    at process.emit (events.js:315:20)
    at emit (internal/child_process.js:876:12)
    at processTicksAndRejections (internal/process/task_queues.js:85:21)