From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Oct 2025 15:03:39 +0000 (+0200) Subject: Fix connector status race condition in UI server cache during startup (#1548) X-Git-Url: https://git.piment-noir.org/?a=commitdiff_plain;h=b7aafd95431f61c160a2dea485a487c8b046629b;p=e-mobility-charging-stations-simulator.git Fix connector status race condition in UI server cache during startup (#1548) * Initial plan * Fix connector status race condition by setting status before sending request Set connector status optimistically before sending the StatusNotification request to ensure the updated event (emitted in the request handler's finally block) contains the current status. Revert status on error. This prevents the UI cache from being populated with stale connector status during startup when multiple status notifications are sent concurrently. Fixes race condition where: 1. StatusNotification sent 2. Response received 3. 'updated' event emitted with OLD status (in finally block) 4. Connector status set to NEW status 5. UI cache now has outdated data Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com> * refactor: avoid non-null assertions in sendAndSetConnectorStatus Replace non-null assertions (!) with null checks and early return to comply with updated coding guidelines. Add error logging when connector status is null. Changes: - Check connectorStatus for null before accessing properties - Store connectorStatus reference to avoid repeated calls - Add error log and early return for non-existing connector - Remove all eslint-disable comments for no-non-null-assertion Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com> * revert: remove incorrect fix to sendAndSetConnectorStatus The previous fix addressed OCPP server communication, but the issue is about UI server cache updates via worker messages. Reverting to investigate the correct location for the fix. The race condition is in how worker messages are sent to the main thread, not in the OCPP StatusNotification timing. Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com> * fix: prevent race condition in UI cache during connector initialization Add skipUpdatedEvent parameter to RequestParams to suppress automatic updated events during connector status initialization loop. Emit single updated event after all connectors are initialized. This fixes the race condition where multiple partial ChargingStationData messages were sent to the main thread during startup, potentially causing the UI server cache to contain stale or incomplete connector status data. Changes: - Add skipUpdatedEvent to RequestParams interface - Update OCPPRequestService to check skipUpdatedEvent before emitting - Add params parameter to sendAndSetConnectorStatus - Use skipUpdatedEvent in startMessageSequence for all connector inits - Emit single updated event after initialization loop completes The fix ensures UI cache receives complete, consistent charging station data after all connectors are properly initialized, eliminating the race condition caused by out-of-order or partial state updates. Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com> * revert: remove incorrect fix that broke status notification lifecycle Previous fix suppressed updated events during initialization, which prevented the correct chronological sending of status notifications to OCPP server. The actual issue is in how the UI server cache handles out-of-order worker messages, not in when updated events are emitted. Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com> * fix: prevent UI cache corruption from out-of-order worker messages Add timestamp to ChargingStationData and update workerEventUpdated to only update cache if incoming message is newer than cached data. This prevents the UI server cache from being overwritten with stale data when worker messages arrive out of chronological order during startup. Changes: - Add timestamp field to ChargingStationData interface - Set timestamp in buildChargingStationDataPayload using Date.now() - Update Bootstrap.workerEventUpdated to compare timestamps - Only update cache if incoming data is newer or cache is empty The fix ensures the UI cache always contains the most recent charging station state, even when multiple updated messages are sent during initialization and arrive out of order. Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com> * refactor: encapsulate chargingStations Map with getter/setter methods Make chargingStations Map private in AbstractUIServer and add public getter/setter methods to encapsulate access. Move timestamp comparison logic into the setter method for centralized cache update control. Changes: - Make chargingStations Map private in AbstractUIServer - Add getChargingStationData() getter for single entry - Add listChargingStationData() to return all entries as array - Add setChargingStationData() with timestamp comparison logic - Add deleteChargingStationData() for deletion - Add hasChargingStationData() for existence check - Add getChargingStationsCount() for size - Update Bootstrap event handlers to use setter methods - Update AbstractUIService to use getter methods This encapsulation ensures all cache updates go through the timestamp comparison logic, preventing corruption from out-of-order messages. Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com> * [autofix.ci] apply automated fixes --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com> Co-authored-by: Jérôme Benoit Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- diff --git a/src/charging-station/Bootstrap.ts b/src/charging-station/Bootstrap.ts index c60f200b..52abb188 100644 --- a/src/charging-station/Bootstrap.ts +++ b/src/charging-station/Bootstrap.ts @@ -543,7 +543,7 @@ export class Bootstrap extends EventEmitter { } private readonly workerEventAdded = (data: ChargingStationData): void => { - this.uiServer.chargingStations.set(data.stationInfo.hashId, data) + this.uiServer.setChargingStationData(data.stationInfo.hashId, data) logger.info( `${this.logPrefix()} ${moduleName}.workerEventAdded: Charging station ${ // eslint-disable-next-line @typescript-eslint/restrict-template-expressions @@ -553,7 +553,7 @@ export class Bootstrap extends EventEmitter { } private readonly workerEventDeleted = (data: ChargingStationData): void => { - this.uiServer.chargingStations.delete(data.stationInfo.hashId) + this.uiServer.deleteChargingStationData(data.stationInfo.hashId) // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const templateStatistics = this.templateStatistics.get(data.stationInfo.templateName)! --templateStatistics.added @@ -582,7 +582,7 @@ export class Bootstrap extends EventEmitter { } private readonly workerEventStarted = (data: ChargingStationData): void => { - this.uiServer.chargingStations.set(data.stationInfo.hashId, data) + this.uiServer.setChargingStationData(data.stationInfo.hashId, data) // eslint-disable-next-line @typescript-eslint/no-non-null-assertion ++this.templateStatistics.get(data.stationInfo.templateName)!.started logger.info( @@ -594,7 +594,7 @@ export class Bootstrap extends EventEmitter { } private readonly workerEventStopped = (data: ChargingStationData): void => { - this.uiServer.chargingStations.set(data.stationInfo.hashId, data) + this.uiServer.setChargingStationData(data.stationInfo.hashId, data) // eslint-disable-next-line @typescript-eslint/no-non-null-assertion --this.templateStatistics.get(data.stationInfo.templateName)!.started logger.info( @@ -606,6 +606,6 @@ export class Bootstrap extends EventEmitter { } private readonly workerEventUpdated = (data: ChargingStationData): void => { - this.uiServer.chargingStations.set(data.stationInfo.hashId, data) + this.uiServer.setChargingStationData(data.stationInfo.hashId, data) } } diff --git a/src/charging-station/ui-server/AbstractUIServer.ts b/src/charging-station/ui-server/AbstractUIServer.ts index 74bb3ad0..7f0ff345 100644 --- a/src/charging-station/ui-server/AbstractUIServer.ts +++ b/src/charging-station/ui-server/AbstractUIServer.ts @@ -26,10 +26,9 @@ import { getUsernameAndPasswordFromAuthorizationToken } from './UIServerUtils.js const moduleName = 'AbstractUIServer' export abstract class AbstractUIServer { - public readonly chargingStations: Map public readonly chargingStationTemplates: Set - protected readonly httpServer: Http2Server | Server + protected readonly responseHandlers: Map< `${string}-${string}-${string}-${string}-${string}`, ServerResponse | WebSocket @@ -37,6 +36,8 @@ export abstract class AbstractUIServer { protected readonly uiServices: Map + private readonly chargingStations: Map + public constructor (protected readonly uiServerConfiguration: UIServerConfiguration) { this.chargingStations = new Map() this.chargingStationTemplates = new Set() @@ -80,10 +81,30 @@ export abstract class AbstractUIServer { this.chargingStationTemplates.clear() } + public deleteChargingStationData (hashId: string): boolean { + return this.chargingStations.delete(hashId) + } + + public getChargingStationData (hashId: string): ChargingStationData | undefined { + return this.chargingStations.get(hashId) + } + + public getChargingStationsCount (): number { + return this.chargingStations.size + } + + public hasChargingStationData (hashId: string): boolean { + return this.chargingStations.has(hashId) + } + public hasResponseHandler (uuid: `${string}-${string}-${string}-${string}-${string}`): boolean { return this.responseHandlers.has(uuid) } + public listChargingStationData (): ChargingStationData[] { + return [...this.chargingStations.values()] + } + public abstract logPrefix (moduleName?: string, methodName?: string, prefixSuffix?: string): string public async sendInternalRequest (request: ProtocolRequest): Promise { @@ -98,6 +119,13 @@ export abstract class AbstractUIServer { public abstract sendResponse (response: ProtocolResponse): void + public setChargingStationData (hashId: string, data: ChargingStationData): void { + const cachedData = this.chargingStations.get(hashId) + if (cachedData == null || data.timestamp >= cachedData.timestamp) { + this.chargingStations.set(hashId, data) + } + } + public abstract start (): void public stop (): void { diff --git a/src/charging-station/ui-server/ui-services/AbstractUIService.ts b/src/charging-station/ui-server/ui-services/AbstractUIService.ts index 8fe6bf70..789e7456 100644 --- a/src/charging-station/ui-server/ui-services/AbstractUIService.ts +++ b/src/charging-station/ui-server/ui-services/AbstractUIService.ts @@ -282,7 +282,7 @@ export abstract class AbstractUIService { private handleListChargingStations (): ResponsePayload { return { - chargingStations: [...this.uiServer.chargingStations.values()] as JsonType[], + chargingStations: this.uiServer.listChargingStationData() as JsonType[], status: ResponseStatus.SUCCESS, } satisfies ResponsePayload } @@ -370,7 +370,7 @@ export abstract class AbstractUIService { if (isNotEmptyArray(payload.hashIds)) { payload.hashIds = payload.hashIds .map(hashId => { - if (this.uiServer.chargingStations.has(hashId)) { + if (this.uiServer.hasChargingStationData(hashId)) { return hashId } logger.warn( @@ -387,7 +387,7 @@ export abstract class AbstractUIService { } const expectedNumberOfResponses = Array.isArray(payload.hashIds) ? payload.hashIds.length - : this.uiServer.chargingStations.size + : this.uiServer.getChargingStationsCount() if (expectedNumberOfResponses === 0) { throw new BaseError( 'hashIds array in the request payload does not contain any valid charging station hashId' diff --git a/src/types/ChargingStationWorker.ts b/src/types/ChargingStationWorker.ts index 05e3a85f..3bea940f 100644 --- a/src/types/ChargingStationWorker.ts +++ b/src/types/ChargingStationWorker.ts @@ -25,6 +25,7 @@ export interface ChargingStationData extends WorkerData { started: boolean stationInfo: ChargingStationInfo supervisionUrl: string + timestamp: number wsState?: | typeof WebSocket.CLOSED | typeof WebSocket.CLOSING diff --git a/src/utils/MessageChannelUtils.ts b/src/utils/MessageChannelUtils.ts index f1ca57c1..69abece3 100644 --- a/src/utils/MessageChannelUtils.ts +++ b/src/utils/MessageChannelUtils.ts @@ -94,6 +94,7 @@ const buildChargingStationDataPayload = (chargingStation: ChargingStation): Char // eslint-disable-next-line @typescript-eslint/no-non-null-assertion stationInfo: chargingStation.stationInfo!, supervisionUrl: chargingStation.wsConnectionUrl.href, + timestamp: Date.now(), wsState: chargingStation.wsConnection?.readyState, ...(chargingStation.automaticTransactionGenerator != null && { automaticTransactionGenerator: