From e2d5d9e793718686a3652ce975a0cd6e676bc67e Mon Sep 17 00:00:00 2001 From: Ryan Tharp Date: Wed, 10 Jun 2020 15:21:28 -0700 Subject: [PATCH 1/2] put lock around buildNewOnionPaths since it's called multiple times --- js/modules/loki_snode_api.js | 122 +++++++++++++++++------------------ 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/js/modules/loki_snode_api.js b/js/modules/loki_snode_api.js index de0c8bf24..f8a1cd03e 100644 --- a/js/modules/loki_snode_api.js +++ b/js/modules/loki_snode_api.js @@ -416,87 +416,87 @@ class LokiSnodeAPI { }); } - // FIXME: need a lock because it is being called multiple times in parallel async buildNewOnionPaths() { - // Note: this function may be called concurrently, so - // might consider blocking the other calls + // Note: this function may be called concurrently - const _ = window.Lodash; - - log.info('LokiSnodeAPI::buildNewOnionPaths - building new onion paths'); + return primitives.allowOnlyOneAtATime('buildNewOnionPaths', async () => { + const _ = window.Lodash; - const allNodes = await this.getRandomSnodePool(); + log.info('LokiSnodeAPI::buildNewOnionPaths - building new onion paths'); - if (this.guardNodes.length === 0) { - // Not cached, load from DB - const nodes = await window.libloki.storage.getGuardNodes(); + const allNodes = await this.getRandomSnodePool(); - if (nodes.length === 0) { - log.warn( - 'LokiSnodeAPI::buildNewOnionPaths - no guard nodes in DB. Will be selecting new guards nodes...' - ); - } else { - // We only store the nodes' keys, need to find full entries: - const edKeys = nodes.map(x => x.ed25519PubKey); - this.guardNodes = allNodes.filter( - x => edKeys.indexOf(x.pubkey_ed25519) !== -1 - ); + if (this.guardNodes.length === 0) { + // Not cached, load from DB + const nodes = await window.libloki.storage.getGuardNodes(); - if (this.guardNodes.length < edKeys.length) { + if (nodes.length === 0) { log.warn( - `LokiSnodeAPI::buildNewOnionPaths - could not find some guard nodes: ${ - this.guardNodes.length - }/${edKeys.length} left` + 'LokiSnodeAPI::buildNewOnionPaths - no guard nodes in DB. Will be selecting new guards nodes...' + ); + } else { + // We only store the nodes' keys, need to find full entries: + const edKeys = nodes.map(x => x.ed25519PubKey); + this.guardNodes = allNodes.filter( + x => edKeys.indexOf(x.pubkey_ed25519) !== -1 ); + + if (this.guardNodes.length < edKeys.length) { + log.warn( + `LokiSnodeAPI::buildNewOnionPaths - could not find some guard nodes: ${ + this.guardNodes.length + }/${edKeys.length} left` + ); + } } - } - // If guard nodes is still empty (the old nodes are now invalid), select new ones: - if (this.guardNodes.length === 0) { - this.guardNodes = await this.selectGuardNodes(); + // If guard nodes is still empty (the old nodes are now invalid), select new ones: + if (this.guardNodes.length === 0) { + this.guardNodes = await this.selectGuardNodes(); + } } - } - // TODO: select one guard node and 2 other nodes randomly - let otherNodes = _.difference(allNodes, this.guardNodes); + // TODO: select one guard node and 2 other nodes randomly + let otherNodes = _.difference(allNodes, this.guardNodes); - if (otherNodes.length < 2) { - log.warn( - 'LokiSnodeAPI::buildNewOnionPaths - Too few nodes to build an onion path! Refreshing pool and retrying' - ); - await this.refreshRandomPool(); - await this.buildNewOnionPaths(); - return; - } + if (otherNodes.length < 2) { + log.warn( + 'LokiSnodeAPI::buildNewOnionPaths - Too few nodes to build an onion path! Refreshing pool and retrying' + ); + await this.refreshRandomPool(); + await this.buildNewOnionPaths(); + return; + } - otherNodes = _.shuffle(otherNodes); - const guards = _.shuffle(this.guardNodes); + otherNodes = _.shuffle(otherNodes); + const guards = _.shuffle(this.guardNodes); - // Create path for every guard node: - const nodesNeededPerPaths = window.lokiFeatureFlags.onionRequestHops - 1; + // Create path for every guard node: + const nodesNeededPerPaths = window.lokiFeatureFlags.onionRequestHops - 1; - // Each path needs X (nodesNeededPerPaths) nodes in addition to the guard node: - const maxPath = Math.floor( - Math.min( - guards.length, - nodesNeededPerPaths - ? otherNodes.length / nodesNeededPerPaths - : otherNodes.length - ) - ); + // Each path needs X (nodesNeededPerPaths) nodes in addition to the guard node: + const maxPath = Math.floor( + Math.min( + guards.length, + nodesNeededPerPaths + ? otherNodes.length / nodesNeededPerPaths + : otherNodes.length + ) + ); - // TODO: might want to keep some of the existing paths - this.onionPaths = []; + // TODO: might want to keep some of the existing paths + this.onionPaths = []; - for (let i = 0; i < maxPath; i += 1) { - const path = [guards[i]]; - for (let j = 0; j < nodesNeededPerPaths; j += 1) { - path.push(otherNodes[i * nodesNeededPerPaths + j]); + for (let i = 0; i < maxPath; i += 1) { + const path = [guards[i]]; + for (let j = 0; j < nodesNeededPerPaths; j += 1) { + path.push(otherNodes[i * nodesNeededPerPaths + j]); + } + this.onionPaths.push({ path, bad: false }); } - this.onionPaths.push({ path, bad: false }); - } - log.info(`Built ${this.onionPaths.length} onion paths`, this.onionPaths); + log.info(`Built ${this.onionPaths.length} onion paths`, this.onionPaths); + }); } async getRandomSnodeAddress() { From a905703cb4be3139fca83c9e4d0cde05b090277b Mon Sep 17 00:00:00 2001 From: Ryan Tharp Date: Thu, 18 Jun 2020 14:55:47 -0700 Subject: [PATCH 2/2] move internal buildNewOnionPaths function into buildNewOnionPathsWorker per review --- js/modules/loki_snode_api.js | 127 ++++++++++++++++++----------------- 1 file changed, 64 insertions(+), 63 deletions(-) diff --git a/js/modules/loki_snode_api.js b/js/modules/loki_snode_api.js index ebdc681ba..6d04da9c8 100644 --- a/js/modules/loki_snode_api.js +++ b/js/modules/loki_snode_api.js @@ -408,88 +408,89 @@ class LokiSnodeAPI { }); } - async buildNewOnionPaths() { - // Note: this function may be called concurrently + async buildNewOnionPathsWorker() { + const _ = window.Lodash; - return primitives.allowOnlyOneAtATime('buildNewOnionPaths', async () => { - const _ = window.Lodash; + log.info('LokiSnodeAPI::buildNewOnionPaths - building new onion paths'); - log.info('LokiSnodeAPI::buildNewOnionPaths - building new onion paths'); + const allNodes = await this.getRandomSnodePool(); - const allNodes = await this.getRandomSnodePool(); + if (this.guardNodes.length === 0) { + // Not cached, load from DB + const nodes = await window.libloki.storage.getGuardNodes(); - if (this.guardNodes.length === 0) { - // Not cached, load from DB - const nodes = await window.libloki.storage.getGuardNodes(); + if (nodes.length === 0) { + log.warn( + 'LokiSnodeAPI::buildNewOnionPaths - no guard nodes in DB. Will be selecting new guards nodes...' + ); + } else { + // We only store the nodes' keys, need to find full entries: + const edKeys = nodes.map(x => x.ed25519PubKey); + this.guardNodes = allNodes.filter( + x => edKeys.indexOf(x.pubkey_ed25519) !== -1 + ); - if (nodes.length === 0) { + if (this.guardNodes.length < edKeys.length) { log.warn( - 'LokiSnodeAPI::buildNewOnionPaths - no guard nodes in DB. Will be selecting new guards nodes...' - ); - } else { - // We only store the nodes' keys, need to find full entries: - const edKeys = nodes.map(x => x.ed25519PubKey); - this.guardNodes = allNodes.filter( - x => edKeys.indexOf(x.pubkey_ed25519) !== -1 + `LokiSnodeAPI::buildNewOnionPaths - could not find some guard nodes: ${ + this.guardNodes.length + }/${edKeys.length} left` ); - - if (this.guardNodes.length < edKeys.length) { - log.warn( - `LokiSnodeAPI::buildNewOnionPaths - could not find some guard nodes: ${ - this.guardNodes.length - }/${edKeys.length} left` - ); - } } + } - // If guard nodes is still empty (the old nodes are now invalid), select new ones: - if (this.guardNodes.length < MIN_GUARD_COUNT) { - // TODO: don't throw away potentially good guard nodes - this.guardNodes = await this.selectGuardNodes(); - } + // If guard nodes is still empty (the old nodes are now invalid), select new ones: + if (this.guardNodes.length < MIN_GUARD_COUNT) { + // TODO: don't throw away potentially good guard nodes + this.guardNodes = await this.selectGuardNodes(); } + } - // TODO: select one guard node and 2 other nodes randomly - let otherNodes = _.difference(allNodes, this.guardNodes); + // TODO: select one guard node and 2 other nodes randomly + let otherNodes = _.difference(allNodes, this.guardNodes); - if (otherNodes.length < 2) { - log.warn( - 'LokiSnodeAPI::buildNewOnionPaths - Too few nodes to build an onion path! Refreshing pool and retrying' - ); - await this.refreshRandomPool(); - await this.buildNewOnionPaths(); - return; - } + if (otherNodes.length < 2) { + log.warn( + 'LokiSnodeAPI::buildNewOnionPaths - Too few nodes to build an onion path! Refreshing pool and retrying' + ); + await this.refreshRandomPool(); + await this.buildNewOnionPaths(); + return; + } - otherNodes = _.shuffle(otherNodes); - const guards = _.shuffle(this.guardNodes); + otherNodes = _.shuffle(otherNodes); + const guards = _.shuffle(this.guardNodes); - // Create path for every guard node: - const nodesNeededPerPaths = window.lokiFeatureFlags.onionRequestHops - 1; + // Create path for every guard node: + const nodesNeededPerPaths = window.lokiFeatureFlags.onionRequestHops - 1; - // Each path needs X (nodesNeededPerPaths) nodes in addition to the guard node: - const maxPath = Math.floor( - Math.min( - guards.length, - nodesNeededPerPaths - ? otherNodes.length / nodesNeededPerPaths - : otherNodes.length - ) - ); + // Each path needs X (nodesNeededPerPaths) nodes in addition to the guard node: + const maxPath = Math.floor( + Math.min( + guards.length, + nodesNeededPerPaths + ? otherNodes.length / nodesNeededPerPaths + : otherNodes.length + ) + ); - // TODO: might want to keep some of the existing paths - this.onionPaths = []; + // TODO: might want to keep some of the existing paths + this.onionPaths = []; - for (let i = 0; i < maxPath; i += 1) { - const path = [guards[i]]; - for (let j = 0; j < nodesNeededPerPaths; j += 1) { - path.push(otherNodes[i * nodesNeededPerPaths + j]); - } - this.onionPaths.push({ path, bad: false }); + for (let i = 0; i < maxPath; i += 1) { + const path = [guards[i]]; + for (let j = 0; j < nodesNeededPerPaths; j += 1) { + path.push(otherNodes[i * nodesNeededPerPaths + j]); } + this.onionPaths.push({ path, bad: false }); + } - log.info(`Built ${this.onionPaths.length} onion paths`, this.onionPaths); - }); + log.info(`Built ${this.onionPaths.length} onion paths`, this.onionPaths); + } + + async buildNewOnionPaths() { + // this function may be called concurrently make sure we only have one inflight + return primitives.allowOnlyOneAtATime('buildNewOnionPaths', this.buildNewOnionPathsWorker); } async getRandomSnodeAddress() {