Skip to content

Commit

Permalink
fix: createDevice race condition
Browse files Browse the repository at this point in the history
Creating a second device would terminate the second device correctly, then delete the old one from the map, causing it to be leaked and go missing

Co-authored-by: Johan Nyman <[email protected]>
  • Loading branch information
Julusian and nytamin committed Oct 5, 2023
1 parent 65a73c4 commit d2004df
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions packages/timeline-state-resolver/src/conductor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,8 @@ export class Conductor extends EventEmitter<ConductorEvents> {
options?: { signal?: AbortSignal }
): Promise<DeviceContainer<DeviceOptionsBase<any>>> {
let newDevice: DeviceContainer<DeviceOptionsBase<any>> | undefined
const throwIfAborted = () => this.throwIfAborted(options?.signal, deviceId, 'creation')
try {
const throwIfAborted = () => this.throwIfAborted(options?.signal, deviceId, 'creation')
if (this.devices.has(deviceId)) {
throw new Error(`Device "${deviceId}" already exists when creating device`)
}
Expand All @@ -441,7 +441,7 @@ export class Conductor extends EventEmitter<ConductorEvents> {

if (!newDevicePromise) {
const type: any = deviceOptions.type
return Promise.reject(`No matching device type for "${type}" ("${DeviceType[type]}") found in conductor`)
throw new Error(`No matching device type for "${type}" ("${DeviceType[type]}") found in conductor`)
}

newDevice = await makeImmediatelyAbortable(async () => {
Expand All @@ -466,15 +466,16 @@ export class Conductor extends EventEmitter<ConductorEvents> {
throw new Error(`Device "${deviceId}" already exists when creating device`)
}
throwIfAborted()
this.devices.set(deviceId, newDevice)

return newDevice
} catch (e) {
await this.terminateUnwantedDevice(newDevice)
this.devices.delete(deviceId)

this.emit('error', 'conductor.createDevice', e)
return Promise.reject(e)
throw e
}

this.devices.set(deviceId, newDevice)

return newDevice
}

private throwIfAborted(signal: AbortSignal | undefined, deviceId: string, action: string) {
Expand Down

0 comments on commit d2004df

Please sign in to comment.