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

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

Closed
DanTup opened this issue Mar 12, 2019 · 10 comments
Closed
Assignees
Labels
tasks Task system issues under-discussion Issue is under discussion for relevance, priority, approach

Comments

@DanTup
Copy link
Contributor

DanTup commented Mar 12, 2019

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).

@DanTup DanTup changed the title Race condition with "runOn": "folderOpen" Race condition with "runOn": "folderOpen" if extension not already activated Mar 12, 2019
@vscodebot vscodebot bot added the tasks Task system issues label Mar 12, 2019
@alexr00
Copy link
Member

alexr00 commented Mar 22, 2019

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.

@alexr00 alexr00 added the bug Issue identified by VS Code Team member as probable bug label Mar 22, 2019
@alexr00 alexr00 added this to the On Deck milestone Mar 22, 2019
@DanTup
Copy link
Contributor Author

DanTup commented Mar 22, 2019

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.

@alexr00
Copy link
Member

alexr00 commented Nov 15, 2021

This should have been fixed by 9c12df8

@alexr00 alexr00 closed this as completed Nov 15, 2021
@alexr00 alexr00 removed this from the On Deck milestone Nov 15, 2021
@DanTup
Copy link
Contributor Author

DanTup commented Nov 15, 2021

@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.

@DanTup
Copy link
Contributor Author

DanTup commented Nov 15, 2021

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.

@alexr00
Copy link
Member

alexr00 commented Nov 16, 2021

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

@DanTup
Copy link
Contributor Author

DanTup commented Nov 16, 2021

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.

@alexr00 alexr00 added under-discussion Issue is under discussion for relevance, priority, approach and removed bug Issue identified by VS Code Team member as probable bug labels Nov 19, 2021
@alexr00 alexr00 added this to the November 2021 milestone Nov 19, 2021
@alexr00
Copy link
Member

alexr00 commented Nov 19, 2021

Reopening to discuss next week.

@DanTup
Copy link
Contributor Author

DanTup commented Nov 19, 2021

@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 :-)

@alexr00 alexr00 reopened this Nov 22, 2021
@alexr00 alexr00 modified the milestones: November 2021, December 2021 Nov 24, 2021
@alexr00
Copy link
Member

alexr00 commented Dec 8, 2021

Closing as we now have an onTaskType activation event.

@alexr00 alexr00 closed this as completed Dec 8, 2021
@alexr00 alexr00 removed this from the December 2021 milestone Dec 8, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
tasks Task system issues under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

2 participants