FileResource sometimes triggers incorrect contents change events

[Hanksha]

Bug Description:

The FileResource sometimes triggers incorrect contents change events because the resource is somehow out of sync during file saving.

I initially found about it in our own Theia based application which has custom editors using the Theia filesystem API (FileResource, FileService etc.) during end to end tests and some user reports, I was then able to reproduce it by running Theia itself in a playwright test since it takes a bunch of iterations to trigger it.

Steps to Reproduce:

1. Disable auto save.
2. Run this test (you can add it to theia-application-shell.test.ts)

test('should open, modified and close a text editor many times', async () => {
    await app.page.pause();
    for (let i = 0; i < 200; i++) {
        const explorer = await app.openView(TheiaExplorerView);

        const fileStatNode = await explorer.getFileStatNodeByLabel('sample.txt');
        const contextMenu = await fileStatNode.openContextMenu();
        await contextMenu.clickMenuItem('Open');

        const textEditor = new TheiaTextEditor('sample.txt', app);
        await textEditor.waitForVisible();

        await textEditor.addTextToNewLineAfterLineByLineNumber(1, 'a');

        await textEditor.save();

        await textEditor.close();
    }
});

4. Once it's paused add a breakpoint to file-resource.ts:302.
5. Resume the test execution.
6. Wait for the breakpoint to trigger because the file resource is not synced anymore.

I've recording a GIF of that issue being reproduced:

Additional Information

• Operating System: Windows
• Theia Version: 1.52.0

I tried to run the test with those changes:

It seems to have fixed the problem, could it be a race condition? Maybe the change event is received by the FileResource before the fileService.update or fileService.write resolves so that the version (which includes mtime) is updated.

[tsmaeder]

Hi @Hanksha. Thanks for catching this. Could you describe a bit what you think is going wrong and how your changes fix the problem?

[Hanksha]

Hi @tsmaeder

Writing/updating a file via FileResource goes as follow:

1. doWrite is called, or doSaveContentChanges if the file is meant to be updated.
2. this.fileService.write or this.fileService.update is called, they both return a FileStatWithMetadata asynchronously.
3. The FileStatWithMetadata is then used to set the _version of the FileResource and update the etag, encoding and the mtime.
4. At the end of this process the FileResource is in sync because the mtime of the version is the same as on the file system.

The problem is that between step 2 and 3, the FileResource can be out of sync since the file is modified on the file system but the mtime is not updated in FileResource. It becomes a problem because it seems that during that process an event from FileService.onDidFilesChange can be triggered between step 2 and 3. This causes the FileResource to check if it's in sync which won't be the case until the step 3 is over. In order to check if it's out of sync, the FileResource simply resolves the URI via this.fileService.resolve and compares the resolved mtime to the stored mtime.

That is why adding a lock fixes the issue. It prevents the FileResource from checking if it's in sync between step 2 and 3. It will wait for the write/update operation to be fully completed before to check if it's still in sync.

[tsmaeder]

So if I understand correctly, this does not require an interaction with a third party system (compiler, etc.), but with only Theia itself handling the file if the timing/ordering is right, correct?

[Hanksha]

Yes that's correct.

[tsmaeder]

I'll add this one to #13192 as a reliability issue. I don't see this problem, so I assumed it's low frequency, right? I would therefore classify it as low prio, though.

[tsmaeder]

And of course, feel free to open a PR if you feel so inclined.

[Hanksha]

Yes it can be low frequency, I guess it depends on the machine, we've had a few users experienced it regularly though.

I'll contribute a PR for this.