WorkspaceService not Concurrency-Safe

[tsmaeder]

When invoking WorkspaceService.spliceRoots() multiple times concurrently (for example via the vs code api workspace.updateWorkspaceRoots(), there can be cases where we run into a "out of sync" condition on the workspace file. Splice roots will do something like:

readWorkspaceFromFile();
update workspace
writeWorkspaceToFile();

it can happen that the both invocations read the file before any invocation writes the file. When the second invocation then tries to write the file, the "out of sync" check fails and we get a dialog asking whether to overwrite the file.

There is also a discussion on Spectrum: https://spectrum.chat/theia/dev/mutual-exclusion-in-api-calls~9bd83415-ffda-4eb2-8b58-3d35427c701e

[paul-marechal]

I "played" around with a naive decorator based API for this, as well as providing a generic operation queue implementation. See the following commit for usage.

Not sure what it's worth, I was thinking that decorator based API are usually kind of sweet.

[akosyakov]

APIs should not be changed while fixing it. WorkspaceService is used a lot.

[tsmaeder]

There's also a problem in that we're relying on the file watcher to call "updateRoots": when we have two "spliceRoots" calls in a row, the second may use the "_roots" variables before it has been updated.

[tsmaeder]

A simple test case would consist of a plugin that calls "vscode.workspace.updateWorkspaceRoots" twice in a row: you'll end up with only one of the changes in the workspace roots.

[slhultgren]

@akosyakov have you thought about this issue more?
As far as I can test your original PR #5828 works for making the spliceRoot mutexed, however there was an additional issue with this code, because spliceRoots relied on the WorkspaceService._roots variable when modifying the roots (not reading from workspace file before modifying). The WorkspaceService._roots is only currently updated by the file watcher which calls WorkspaceService.updateRoots().

I made a PR which triggers an WorkspaceService.updateRoots() when the workspace file is written. #8605.

If we merge both our PRs the code will behave properly for both:

for (let path of paths) {
    await workspaceService.addRoot(path);    // Fixed by my PR
}

and

for (let path of paths) {
    workspaceService.addRoot(path);    // Fixed by your PR https://github.com/eclipse-theia/theia/pull/5828
}

[akosyakov]

We should use Monaco model, it is thread safe already. As we did it for other settings.