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

There is a race starting dependent tasks #180578

Closed
markw65 opened this issue Apr 21, 2023 · 1 comment · Fixed by #180617
Closed

There is a race starting dependent tasks #180578

markw65 opened this issue Apr 21, 2023 · 1 comment · Fixed by #180617
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders tasks Task system issues verified Verification succeeded
Milestone

Comments

@markw65
Copy link
Contributor

markw65 commented Apr 21, 2023

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.77.3
  • OS Version: MacOS 13.3.1

Steps to Reproduce:

Run the following shell commands to create a project with a suitable set of dependent tasks

mkdir -p test/.vscode
cd test
cat <<"EOF" > .vscode/tasks.json
{
  "version": "2.0.0",
  "tasks": [
    {
      "type": "shell",
      "command": "sleep 2",
      "label": "sleeper",
      "problemMatcher": [],
      "presentation": {
        "reveal": "always",
        "group": "sleepers",
        "close": false
      }
    },
    {
      "type": "shell",
      "command": "echo task1",
      "dependsOn": ["sleeper"],
      "label": "task1",
      "problemMatcher": [],
      "presentation": {
        "reveal": "always",
        "group": "sleepers",
        "close": false
      }
    },
    {
      "type": "shell",
      "command": "echo task2",
      "dependsOn": ["task1"],
      "label": "task2",
      "problemMatcher": [],
      "presentation": {
        "reveal": "always",
        "group": "sleepers",
        "close": false
      }
    },
    {
      "type": "shell",
      "command": "echo task3",
      "dependsOn": ["task1"],
      "label": "task3",
      "problemMatcher": [],
      "presentation": {
        "reveal": "always",
        "group": "sleepers",
        "close": false
      }
    },
    {
      "type": "shell",
      "command": "echo task4... && sleep 3 && echo ...done",
      "dependsOn": ["task2", "task3"],
      "label": "task4",
      "problemMatcher": [],
      "presentation": {
        "reveal": "always",
        "group": "sleepers",
        "close": false
      },
      "group": {
        "kind": "build",
        "isDefault": true
      }
    }
  ]
}
EOF
code .

The pattern here is that task4 depends on task3 and task2, task3 and task2 each depend on task1, and task1 depends on sleeper.

Running task4 should launch one copy of sleeper, then one instance of task1, then one instance each of task2 and task3 (in parallel) and finally task4.

Instead it runs sleeper, task1 then task2, then reports there's a dependency cycle involving task1 and stops (the message appears in the output pane, rather than the debug console).

The issue is similar to the one in #180541. task2 launches task1 via _executeDependentTask which calls _executeTask, which returns before adding task1 to _activeTasks because that can't happen until sleeper finishes. Then task3 thinks that task1 isn't running yet (because it's not in _activeTasks) and tries to launch it again; and now it sees the false dependency cycle. What we want is for task3 to attach to the already launched task1.

markw65 pushed a commit to markw65/vscode that referenced this issue Apr 21, 2023
Fixes microsoft#180578

This fixes the issue by keeping a map of tasks that have been passed
to _executeTask, but which haven't yet been added to _activeTasks.

Code that needs to synchronise with _activeTasks waits on the corresponding
_activatingTasks promise, which will resolve when the task is actually added to
_activeTasks (or when an error causes it to not be started).
markw65 pushed a commit to markw65/vscode that referenced this issue Apr 22, 2023
Fixes microsoft#180578

The problem was that after a call to _executeTask there is a delay (at least
until the next tick, but potentially considerably longer) before the task is
added to _activeTasks. During this period a second instance of the task could
be kicked off, causing problems.

This commit fixes the issue by adding an entry (without a terminal) to
_activeTasks before returning from _executeTask. For the most part though, it
preserves the existing behavior - so for example `getActiveTasks` only returns
the tasks that have terminals attached, just as it did before; and there is no
change to when `TaskEvent` events are fired for the task.
markw65 pushed a commit to markw65/vscode that referenced this issue Apr 22, 2023
Fixes microsoft#180578

The problem was that after a call to _executeTask there is a delay (at least
until the next tick, but potentially considerably longer) before the task is
added to _activeTasks. During this period a second instance of the task could
be kicked off, causing problems.

This commit fixes the issue by adding an entry (without a terminal) to
_activeTasks before returning from _executeTask. For the most part though, it
preserves the existing behavior - so for example `getActiveTasks` only returns
the tasks that have terminals attached, just as it did before; and there is no
change to when `TaskEvent` events are fired for the task.
@meganrogge meganrogge added bug Issue identified by VS Code Team member as probable bug tasks Task system issues labels Apr 24, 2023
@meganrogge meganrogge added this to the May 2023 milestone Apr 24, 2023
markw65 pushed a commit to markw65/vscode that referenced this issue Apr 24, 2023
Fixes microsoft#180578

The problem was that after a call to _executeTask there is a delay (at least
until the next tick, but potentially considerably longer) before the task is
added to _activeTasks. During this period a second instance of the task could
be kicked off, causing problems.

This commit fixes the issue by adding an entry (without a terminal) to
_activeTasks before returning from _executeTask. For the most part though, it
preserves the existing behavior - so for example `getActiveTasks` only returns
the tasks that have terminals attached, just as it did before; and there is no
change to when `TaskEvent` events are fired for the task.
markw65 pushed a commit to markw65/vscode that referenced this issue Apr 25, 2023
Fixes microsoft#180578

The problem was that after a call to _executeTask there is a delay (at least
until the next tick, but potentially considerably longer) before the task is
added to _activeTasks. During this period a second instance of the task could
be kicked off, causing problems.

This commit fixes the issue by adding an entry (without a terminal) to
_activeTasks before returning from _executeTask. For the most part though, it
preserves the existing behavior - so for example `getActiveTasks` only returns
the tasks that have terminals attached, just as it did before; and there is no
change to when `TaskEvent` events are fired for the task.
@meganrogge
Copy link
Contributor

Verify what is described above now works and also kill one of the dependent tasks, rerun the original task, and confirm that it gets created.

@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels May 4, 2023
@rzhao271 rzhao271 added the verified Verification succeeded label May 31, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders tasks Task system issues verified Verification succeeded
Projects
None yet
4 participants