Properly synchronise terminalTaskSystem._activeTasks #180581
Conversation
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).
|
I'll just note that while I'm pretty sure this works, and closes the race, I think a cleaner approach would repurpose _activeTasks instead of creating the new _activatingTasks map. Basically _executeTask would add an entry to _activeTasks before starting to look at its dependents. This would mean that the terminal in IActiveTerminalData would have to be optional (because we don't have a terminal for it yet). And then the terminal would be added once the terminal is determined. I started down that route, but there are many uses of _activeTasks where I struggled (due to lack of familiarity with the code) to determine what the correct behavior would be if there was no terminal. So I went with this instead. I'd be happy to try again if that seems like a better approach. |
|
I thought about this some more, and #180617 seems like a much better solution |
Fixes #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).