Skip to content

Race condition with "runOn": "folderOpen" if extension not already activated #70303

Closed

Description

@DanTup

I had an issue raised at Dart-Code/Dart-Code#1516 that seems to be a race condition when tasks are set to "runOn": "folderOpen". The issue is that the task is invoked before the extension activates resulting in "The pub task detection didn't contribute a task" (on Windows, at least - I can repro there, but not on macOS...).

In the task provider sample, it activates with "onCommand:workbench.action.tasks.runTask" which sidesteps this issue by ensuring the extension is activated when any task is run.

It doesn't make sense for all extensions that have tasks to activate when any task at all is run. It's bad for VS Code's performance, and since we don't have an activation reason we don't know when this happens. VS Code knows the type of the commands it's trying to run, and it knows which extensions may be involved (from their manifests).

Ideally, I think VS Code could activate the extensions it knows may be required when it's executing a command. Otherwise, I think we should at least be able to activate on execution of a specific task type (onTaskType:foo for ex).

Activity

  1. changed the title [-]Race condition with "runOn": "folderOpen"[/-] [+]Race condition with "runOn": "folderOpen" if extension not already activated[/+] on Mar 12, 2019
  2. alexr00 commented on Mar 22, 2019

    @alexr00
    Member

    This is unfortunately tricky to do. There isn't a good way to get at the task type that a provider provides until it has actually provided some tasks.

  3. added
    bugIssue identified by VS Code Team member as probable bug
    on Mar 22, 2019
  4. added this to the On Deck milestone on Mar 22, 2019
  5. DanTup commented on Mar 22, 2019

    @DanTup
    ContributorAuthor

    Could you use the types from the manifest? Or have an onTaskType:foo activation event? Or give us an activation reason and allow us to deactivate after providing the tasks?

    I appreciate it may be tricky (most useful things are :-)), but having every extension that contributes tasks immediately launched when anything tries to run a task doesn't scale/perform well. I'm entirely happy to additional work in my extension to prevent it. It bugs me when extensions are unnecessarily activated (and it leads to confusing user experience, like this where Node forces itself into the debug list for every project) and I don't want to push similar issues onto my extensions users.

  6. alexr00 commented on Nov 15, 2021

    @alexr00
    Member

    This should have been fixed by 9c12df8

  7. removed this from the On Deck milestone on Nov 15, 2021
  8. DanTup commented on Nov 15, 2021

    @DanTup
    ContributorAuthor

    Alex Ross (@alexr00) can you clarify the intended behaviour with that change? The code looks like it waits for extensions that activate on "onCommand:workbench.action.tasks.runTask" but the description above specifically describes not wanting to do this because it just eagerly activates the extensions when any task is run.

  9. DanTup commented on Nov 15, 2021

    @DanTup
    ContributorAuthor

    Oh, I didn't scroll along... perhaps whenInstalledExtensionsRegistered handles waiting for any in-progress activations to complete? I couldn't find a good description for that function.

  10. alexr00 commented on Nov 16, 2021

    @alexr00
    Member

    Danny Tuppeny (@DanTup) the recommendation for extension that provide tasks is to always activate "onCommand:workbench.action.tasks.runTask".

  11. DanTup commented on Nov 16, 2021

    @DanTup
    ContributorAuthor

    the recommendation for extension that provide tasks is to always activate "onCommand:workbench.action.tasks.runTask".

    This recommendation doesn't seem very consistent with other functionality that is designed to prevent VS Code activating extensions for projects where they're not relevant. This comes up quite a bit and I'm forever asking for some guidelines around this, but haven't really had any.

    Right now, when my language extension activates, it starts a language server. It does this unconditionally because VS Code does not give the reason for activation (#44711). The alternative would be to scan the workspace to check if we really should activate, but in many cases this ends up duplicating a search that VS Code has already done and bails out of in large workspaces (#73656).

    The issue at #88617 (comment) was specifically re-opened to deal with a case exactly like this, where extensions are being eagerly activated in contexts where they're not required. It doesn't seem necessary for every extension that uses tasks to be activated when any single task is run.

    It's a little frustrating that despite repeated requests, there have never been any guidelines published for language authors about handling activation like this. Should we assume VS Code is trying not to activate us when not required, or should we assume it may eagerly activate is frequently and we should handle our own "real" activation inside after running in a lower-impact mode?

    I have no problem doing either, but it would be nice if VS Code could provide guidelines or at least be consistent about this. It would not be worth the effort of making my own activation handling to avoid spawning a language server in cases like this if VS Code is generally trying to avoid it.

  12. added
    under-discussionIssue is under discussion for relevance, priority, approach
    and removed
    bugIssue identified by VS Code Team member as probable bug
    on Nov 19, 2021
  13. alexr00 commented on Nov 19, 2021

    @alexr00
    Member

    Reopening to discuss next week.

  14. DanTup commented on Nov 19, 2021

    @DanTup
    ContributorAuthor

    Alex Ross (@alexr00) Thanks! (though I think you forgot to re-open :)). FWIW there are some related comments at #73656 (comment).

    I'm ok if the outcome is that this remains as-is, but I'd really like to see some proper guidance so that everyone is on the same page. Right now for ever issue where I think that's a good idea, there's another that makes me think it's worthwhile :-)

  15. alexr00 commented on Dec 8, 2021

    @alexr00
    Member

    Closing as we now have an onTaskType activation event.

  16. removed this from the December 2021 milestone on Dec 8, 2021
  17. locked and limited conversation to collaborators on Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

tasksTask system issuesunder-discussionIssue is under discussion for relevance, priority, approach

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions