Skip to content

Commit

Permalink
stop reading before writing stores (#12717)
Browse files Browse the repository at this point in the history
The merging of on disk values using `deepmerge` is bogus.

Instead we'll completely overwrite the file. This should be fine as the
`PluginsKeyValueStorage` singleton is shared across all plugin host
processes, meaning there shouldn't be a risk for a race condition
to corrupt the file being written on disk.

Note that there is still a race condition risk when multiple Theia
backends are running on the same host, but odds should be low.
  • Loading branch information
paul-marechal authored Jul 26, 2023
1 parent a75dd66 commit c8fb5ad
Showing 1 changed file with 3 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
// *****************************************************************************

import { deepmerge } from '@theia/core/shared/@theia/application-package';
import { injectable, inject, postConstruct } from '@theia/core/shared/inversify';
import { FileSystemLocking } from '@theia/core/lib/node';
import * as fs from '@theia/core/shared/fs-extra';
Expand Down Expand Up @@ -111,11 +110,9 @@ export class PluginsKeyValueStorage {

private syncStores(): void {
this.syncStoresTimeout = setTimeout(async () => {
await Promise.all(Array.from(this.storesToSync, async store => {
await this.fsLocking.lockPath(store.fsPath, async storePath => {
const valuesOnDisk = await this.readFromFile(storePath);
store.values = deepmerge(valuesOnDisk, store.values);
await this.writeToFile(storePath, store.values);
await Promise.all(Array.from(this.storesToSync, async ({ fsPath, values }) => {
await this.fsLocking.lockPath(fsPath, async storePath => {
await this.writeToFile(storePath, values);
});
}));
this.storesToSync.clear();
Expand Down

0 comments on commit c8fb5ad

Please sign in to comment.