Skip to content

Commit cc1a49c

Browse files
re-fix configuration provider initialization (#1386)
1 parent 1bc7009 commit cc1a49c

File tree

2 files changed

+38
-21
lines changed

2 files changed

+38
-21
lines changed

packages/langium/src/lsp/language-server.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,13 +199,16 @@ export class DefaultLanguageServer implements LanguageServer {
199199
const configurationParams = connection ? <ConfigurationInitializedParams>{
200200
...params,
201201
register: params => connection.client.register(DidChangeConfigurationNotification.type, params),
202-
getConfiguration: params => connection.workspace.getConfiguration(params)
202+
fetchConfiguration: params => connection.workspace.getConfiguration(params)
203203
} : params;
204204

205-
this.services.workspace.WorkspaceManager.initialized(params)
206-
.catch(err => console.error('Error in WorkspaceManager initialization:', err));
205+
// do not await the promises of the following calls, as they must not block the initialization process!
206+
// otherwise, there is the danger of out-of-order processing of subsequent incoming messages from the language client
207+
// however, awaiting should be possible in general, e.g. in unit test scenarios
207208
this.services.workspace.ConfigurationProvider.initialized(configurationParams)
208209
.catch(err => console.error('Error in ConfigurationProvider initialization:', err));
210+
this.services.workspace.WorkspaceManager.initialized(params)
211+
.catch(err => console.error('Error in WorkspaceManager initialization:', err));
209212
}
210213
}
211214

packages/langium/src/workspace/configuration.ts

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,7 @@
44
* terms of the MIT License, which is available in the project root.
55
******************************************************************************/
66

7-
import type {
8-
ConfigurationItem, DidChangeConfigurationParams, DidChangeConfigurationRegistrationOptions, InitializeParams,
9-
InitializedParams
10-
} from 'vscode-languageserver-protocol';
7+
import type { ConfigurationItem, DidChangeConfigurationParams, DidChangeConfigurationRegistrationOptions, InitializeParams, InitializedParams } from 'vscode-languageserver-protocol';
118
import type { ServiceRegistry } from '../service-registry.js';
129
import type { LangiumSharedCoreServices } from '../services.js';
1310

@@ -46,7 +43,7 @@ export interface ConfigurationProvider {
4643

4744
export interface ConfigurationInitializedParams extends InitializedParams {
4845
register?: (params: DidChangeConfigurationRegistrationOptions) => void,
49-
getConfiguration?: (configuration: ConfigurationItem[]) => Promise<any>
46+
fetchConfiguration?: (configuration: ConfigurationItem[]) => Promise<any>
5047
}
5148

5249
/**
@@ -57,6 +54,7 @@ export class DefaultConfigurationProvider implements ConfigurationProvider {
5754
protected readonly serviceRegistry: ServiceRegistry;
5855
protected settings: Record<string, Record<string, any>> = {};
5956
protected workspaceConfig = false;
57+
protected fetchConfiguration: ((configurations: ConfigurationItem[]) => Promise<any>) | undefined;
6058

6159
constructor(services: LangiumSharedCoreServices) {
6260
this.serviceRegistry = services.ServiceRegistry;
@@ -79,20 +77,34 @@ export class DefaultConfigurationProvider implements ConfigurationProvider {
7977
});
8078
}
8179

82-
if (params.getConfiguration) {
83-
// params.getConfiguration(...) is a function to be provided by the calling language server for the sake of
84-
// decoupling this implementation from the concrete LSP implementations, specifically the LSP Connection
80+
// params.fetchConfiguration(...) is a function to be provided by the calling language server for the sake of
81+
// decoupling this implementation from the concrete LSP implementations, specifically the LSP Connection
82+
this.fetchConfiguration = params.fetchConfiguration;
8583

86-
const configToUpdate = this.serviceRegistry.all.map(lang => <ConfigurationItem>{
87-
// Fetch the configuration changes for all languages
88-
section: this.toSectionName(lang.LanguageMetaData.languageId)
89-
});
90-
// get workspace configurations (default scope URI)
91-
const configs = await params.getConfiguration(configToUpdate);
92-
configToUpdate.forEach((conf, idx) => {
93-
this.updateSectionConfiguration(conf.section!, configs[idx]);
94-
});
95-
}
84+
// awaiting the fetch of the initial configuration data must not happen here, because it would block the
85+
// initialization of the language server, and may lead to out-of-order processing of subsequent messages from the language client;
86+
// fetching the initial configuration in a non-blocking manner might cause a read-before-write problem
87+
// in case the workspace initialization relies on that configuration data (which is the case for the grammar language server);
88+
// consequently, we fetch the initial configuration data on blocking manner on-demand, when the first configuration value is read,
89+
// see below in #getConfiguration(...);
90+
}
91+
}
92+
93+
protected async initializeConfiguration(): Promise<void> {
94+
if (this.fetchConfiguration) {
95+
const configToUpdate = this.serviceRegistry.all.map(lang => <ConfigurationItem>{
96+
// Fetch the configuration changes for all languages
97+
section: this.toSectionName(lang.LanguageMetaData.languageId)
98+
});
99+
100+
// get workspace configurations (default scope URI)
101+
const configs = await this.fetchConfiguration(configToUpdate);
102+
configToUpdate.forEach((conf, idx) => {
103+
this.updateSectionConfiguration(conf.section!, configs[idx]);
104+
});
105+
106+
// reset the 'fetchConfiguration' to 'undefined' again in order to prevent further 'fetch' attempts
107+
this.fetchConfiguration = undefined;
96108
}
97109
}
98110

@@ -122,6 +134,8 @@ export class DefaultConfigurationProvider implements ConfigurationProvider {
122134
* @param configuration Configuration name
123135
*/
124136
async getConfiguration(language: string, configuration: string): Promise<any> {
137+
await this.initializeConfiguration();
138+
125139
const sectionName = this.toSectionName(language);
126140
if (this.settings[sectionName]) {
127141
return this.settings[sectionName][configuration];

0 commit comments

Comments
 (0)