From 5cea367399657386a9c8ebd3eba23f2ff208adee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABtan=20Renaudeau?= Date: Tue, 3 Jul 2018 15:35:55 +0200 Subject: [PATCH] Refactor deviceAccess without semaphore using a simple queue --- package.json | 1 - src/helpers/deviceAccess.js | 80 +++++++++++-------------------------- yarn.lock | 4 -- 3 files changed, 23 insertions(+), 62 deletions(-) diff --git a/package.json b/package.json index 95599c61..a143110f 100644 --- a/package.json +++ b/package.json @@ -96,7 +96,6 @@ "rxjs": "^6.2.1", "rxjs-compat": "^6.2.1", "secp256k1": "3.3.1", - "semaphore": "^1.1.0", "semver": "^5.5.0", "smoothscroll-polyfill": "^0.4.3", "source-map": "0.7.3", diff --git a/src/helpers/deviceAccess.js b/src/helpers/deviceAccess.js index f6fbc478..590fdddc 100644 --- a/src/helpers/deviceAccess.js +++ b/src/helpers/deviceAccess.js @@ -1,5 +1,4 @@ // @flow -import createSemaphore from 'semaphore' import type Transport from '@ledgerhq/hw-transport' import TransportNodeHid from '@ledgerhq/hw-transport-node-hid' import { DEBUG_DEVICE } from 'config/constants' @@ -10,67 +9,34 @@ import { createCustomErrorClass } from './errors' // and guarantee we do one device access at a time. It also will handle the .close() // NOTE optim: in the future we can debounce the close & reuse the same transport instance. -type WithDevice = (devicePath: string) => (job: (Transport<*>) => Promise) => Promise - -const semaphorePerDevice = {} +type WithDevice = (devicePath: string) => (job: (Transport<*>) => Promise<*>) => Promise const DisconnectedDevice = createCustomErrorClass('DisconnectedDevice') -const remapError = (p: Promise): Promise => - p.catch(e => { - if (e && e.message && e.message.indexOf('HID') >= 0) { - throw new DisconnectedDevice(e.message) - } - throw e - }) +const mapError = e => { + if (e && e.message && e.message.indexOf('HID') >= 0) { + throw new DisconnectedDevice(e.message) + } + throw e +} -export const withDevice: WithDevice = devicePath => { - const sem = - semaphorePerDevice[devicePath] || (semaphorePerDevice[devicePath] = createSemaphore(1)) +let queue = Promise.resolve() - return job => - takeSemaphorePromise(sem, devicePath, async () => { - const t = await retry(() => TransportNodeHid.open(devicePath), { maxRetry: 1 }) +export const withDevice: WithDevice = devicePath => job => { + const p = queue.then(async () => { + const t = await retry(() => TransportNodeHid.open(devicePath), { maxRetry: 1 }) + if (DEBUG_DEVICE) { + t.setDebugMode(true) + } + try { + const res = await job(t).catch(mapError) + return res + } finally { + await t.close() + } + }) - if (DEBUG_DEVICE) t.setDebugMode(true) - try { - const res = await remapError(job(t)) - // $FlowFixMe - return res - } finally { - await t.close() - } - }) -} + queue = p.catch(() => null) -function takeSemaphorePromise(sem, devicePath, f: () => Promise): Promise { - return new Promise((resolve, reject) => { - sem.take(() => { - process.send({ - type: 'setDeviceBusy', - busy: true, - devicePath, - }) - f().then( - r => { - sem.leave() - resolve(r) - process.send({ - type: 'setDeviceBusy', - busy: false, - devicePath, - }) - }, - e => { - sem.leave() - reject(e) - process.send({ - type: 'setDeviceBusy', - busy: false, - devicePath, - }) - }, - ) - }) - }) + return p } diff --git a/yarn.lock b/yarn.lock index bdc99385..2be06201 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12810,10 +12810,6 @@ selfsigned@^1.9.1: dependencies: node-forge "0.7.5" -semaphore@^1.1.0: - version "1.1.0" - resolved "https://registry.yarnpkg.com/semaphore/-/semaphore-1.1.0.tgz#aaad8b86b20fe8e9b32b16dc2ee682a8cd26a8aa" - semver-diff@^2.0.0: version "2.1.0" resolved "https://registry.yarnpkg.com/semver-diff/-/semver-diff-2.1.0.tgz#4bbb8437c8d37e4b0cf1a68fd726ec6d645d6d36"