From cc3f98b20acdf71a285adcaebe8a0e84221231a9 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Fri, 1 Feb 2019 10:34:27 +1100 Subject: [PATCH 1/5] Move the incoming hash filtering logic into sql.js to prevent data races --- app/sql.js | 8 ++++++-- js/modules/data.js | 2 +- libtextsecure/http-resources.js | 11 ++++------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/app/sql.js b/app/sql.js index b64c4feac..95aa17e82 100644 --- a/app/sql.js +++ b/app/sql.js @@ -1534,18 +1534,22 @@ async function saveMessage(data, { forceSave } = {}) { return toCreate.id; } -async function saveSeenMessageHashes(arrayOfHashes) { +async function saveSeenMessageHashes(incomingHashes) { let promise; + const hashList = incomingHashes.map(h => h.hash); + const dupHashes = await getSeenMessagesByHashList(hashList); + const newHashes = incomingHashes.filter(h => !dupHashes.includes(h.hash)); db.serialize(() => { promise = Promise.all([ db.run('BEGIN TRANSACTION;'), - ...map(arrayOfHashes, hashData => saveSeenMessageHash(hashData)), + ...map(newHashes, hashData => saveSeenMessageHash(hashData)), db.run('COMMIT TRANSACTION;'), ]); }); await promise; + return newHashes; } async function saveSeenMessageHash(data) { diff --git a/js/modules/data.js b/js/modules/data.js index e850c723d..622750aff 100644 --- a/js/modules/data.js +++ b/js/modules/data.js @@ -788,7 +788,7 @@ async function cleanSeenMessages() { } async function saveSeenMessageHashes(data) { - await channels.saveSeenMessageHashes(_cleanData(data)); + return channels.saveSeenMessageHashes(_cleanData(data)); } async function saveSeenMessageHash(data) { diff --git a/libtextsecure/http-resources.js b/libtextsecure/http-resources.js index bded6ef25..a506a31fe 100644 --- a/libtextsecure/http-resources.js +++ b/libtextsecure/http-resources.js @@ -45,16 +45,13 @@ const filterIncomingMessages = async function filterIncomingMessages( messages ) { - const incomingHashes = messages.map(m => m.hash); - const dupHashes = await window.Signal.Data.getSeenMessagesByHashList( - incomingHashes - ); - const newMessages = messages.filter(m => !dupHashes.includes(m.hash)); - const newHashes = newMessages.map(m => ({ + const incomingHashes = messages.map(m => ({ expiresAt: m.expiration, hash: m.hash, })); - await window.Signal.Data.saveSeenMessageHashes(newHashes); + let newHashes = await window.Signal.Data.saveSeenMessageHashes(incomingHashes); + newHashes = newHashes.map(h => h.hash); + const newMessages = messages.filter(m => newHashes.includes(m.hash)); return newMessages; }; From 25383458b1fef936730e81415800d28f3b42e7a7 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Fri, 1 Feb 2019 11:30:15 +1100 Subject: [PATCH 2/5] Fixed bug with our swarm nodes being removed from memory after a successful request because of returning a reference --- js/modules/loki_message_api.js | 2 +- js/modules/loki_snode_api.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/js/modules/loki_message_api.js b/js/modules/loki_message_api.js index 8c05e7c39..e802dab13 100644 --- a/js/modules/loki_message_api.js +++ b/js/modules/loki_message_api.js @@ -282,7 +282,7 @@ class LokiMessageAPI { await Promise.all( Object.entries(ourSwarmNodes) .splice(0, remainingRequests) - .map(([nodeUrl, lastHash]) => doRequest(nodeUrl, lastHash)) + .map(([nodeUrl, nodeData]) => doRequest(nodeUrl, nodeData)) ); } } diff --git a/js/modules/loki_snode_api.js b/js/modules/loki_snode_api.js index 0a6265334..74fea4762 100644 --- a/js/modules/loki_snode_api.js +++ b/js/modules/loki_snode_api.js @@ -103,7 +103,7 @@ class LokiSnodeAPI { }; }); } - return this.ourSwarmNodes; + return {...this.ourSwarmNodes}; } async refreshSwarmNodesForPubKey(pubKey) { From b2f456031f043e5f1edccd29f6700a89a4846723 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Fri, 1 Feb 2019 14:48:25 +1100 Subject: [PATCH 3/5] Forgot to lint --- js/modules/loki_snode_api.js | 2 +- libtextsecure/http-resources.js | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/js/modules/loki_snode_api.js b/js/modules/loki_snode_api.js index 74fea4762..2fe719633 100644 --- a/js/modules/loki_snode_api.js +++ b/js/modules/loki_snode_api.js @@ -103,7 +103,7 @@ class LokiSnodeAPI { }; }); } - return {...this.ourSwarmNodes}; + return { ...this.ourSwarmNodes }; } async refreshSwarmNodesForPubKey(pubKey) { diff --git a/libtextsecure/http-resources.js b/libtextsecure/http-resources.js index a506a31fe..995655181 100644 --- a/libtextsecure/http-resources.js +++ b/libtextsecure/http-resources.js @@ -49,7 +49,9 @@ expiresAt: m.expiration, hash: m.hash, })); - let newHashes = await window.Signal.Data.saveSeenMessageHashes(incomingHashes); + let newHashes = await window.Signal.Data.saveSeenMessageHashes( + incomingHashes + ); newHashes = newHashes.map(h => h.hash); const newMessages = messages.filter(m => newHashes.includes(m.hash)); return newMessages; From fac8e72861abe5c2b29ff9b84edc92e528a2fca3 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Mon, 4 Feb 2019 11:14:31 +1100 Subject: [PATCH 4/5] Added queue manager to properly fix the race condition bug and reset the changes I made in other commit --- app/sql.js | 8 ++------ background.html | 1 + js/modules/data.js | 2 +- js/queue_manager.js | 28 ++++++++++++++++++++++++++++ libtextsecure/http-resources.js | 18 +++++++++++------- 5 files changed, 43 insertions(+), 14 deletions(-) create mode 100644 js/queue_manager.js diff --git a/app/sql.js b/app/sql.js index 95aa17e82..b64c4feac 100644 --- a/app/sql.js +++ b/app/sql.js @@ -1534,22 +1534,18 @@ async function saveMessage(data, { forceSave } = {}) { return toCreate.id; } -async function saveSeenMessageHashes(incomingHashes) { +async function saveSeenMessageHashes(arrayOfHashes) { let promise; - const hashList = incomingHashes.map(h => h.hash); - const dupHashes = await getSeenMessagesByHashList(hashList); - const newHashes = incomingHashes.filter(h => !dupHashes.includes(h.hash)); db.serialize(() => { promise = Promise.all([ db.run('BEGIN TRANSACTION;'), - ...map(newHashes, hashData => saveSeenMessageHash(hashData)), + ...map(arrayOfHashes, hashData => saveSeenMessageHash(hashData)), db.run('COMMIT TRANSACTION;'), ]); }); await promise; - return newHashes; } async function saveSeenMessageHash(data) { diff --git a/background.html b/background.html index f2a11cda3..7fa85684e 100644 --- a/background.html +++ b/background.html @@ -723,6 +723,7 @@ + diff --git a/js/modules/data.js b/js/modules/data.js index 622750aff..e850c723d 100644 --- a/js/modules/data.js +++ b/js/modules/data.js @@ -788,7 +788,7 @@ async function cleanSeenMessages() { } async function saveSeenMessageHashes(data) { - return channels.saveSeenMessageHashes(_cleanData(data)); + await channels.saveSeenMessageHashes(_cleanData(data)); } async function saveSeenMessageHash(data) { diff --git a/js/queue_manager.js b/js/queue_manager.js new file mode 100644 index 000000000..7202c83dc --- /dev/null +++ b/js/queue_manager.js @@ -0,0 +1,28 @@ +/* eslint-disable more/no-then */ + +// eslint-disable-next-line func-names +(function() { + 'use strict' + + class JobQueue { + constructor() { + this.pending = Promise.resolve(); + } + + add(job) { + const previous = this.pending || Promise.resolve(); + this.pending = previous.then(job, job); + const current = this.pending; + + current.then(() => { + if (this.pending === current) { + delete this.pending; + } + }); + + return current; + } + } + + window.JobQueue = JobQueue; +})(); diff --git a/libtextsecure/http-resources.js b/libtextsecure/http-resources.js index 995655181..4ac3ee38c 100644 --- a/libtextsecure/http-resources.js +++ b/libtextsecure/http-resources.js @@ -45,15 +45,16 @@ const filterIncomingMessages = async function filterIncomingMessages( messages ) { - const incomingHashes = messages.map(m => ({ + const incomingHashes = messages.map(m => m.hash); + const dupHashes = await window.Signal.Data.getSeenMessagesByHashList( + incomingHashes + ); + const newMessages = messages.filter(m => !dupHashes.includes(m.hash)); + const newHashes = newMessages.map(m => ({ expiresAt: m.expiration, hash: m.hash, })); - let newHashes = await window.Signal.Data.saveSeenMessageHashes( - incomingHashes - ); - newHashes = newHashes.map(h => h.hash); - const newMessages = messages.filter(m => newHashes.includes(m.hash)); + await window.Signal.Data.saveSeenMessageHashes(newHashes); return newMessages; }; @@ -64,9 +65,12 @@ handleRequest = request => request.respond(404, 'Not found'); } let connected = false; + const jobQueue = new window.JobQueue(); const processMessages = async messages => { - const newMessages = await filterIncomingMessages(messages); + const newMessages = await jobQueue.add( + () => filterIncomingMessages(messages) + ); newMessages.forEach(async message => { const { data } = message; this.handleMessage(data); From b2e95932f14139f6b4ddb925b06a24a2beac67c2 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Mon, 4 Feb 2019 11:18:59 +1100 Subject: [PATCH 5/5] Lint --- js/queue_manager.js | 2 +- libtextsecure/http-resources.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/js/queue_manager.js b/js/queue_manager.js index 7202c83dc..348d0868a 100644 --- a/js/queue_manager.js +++ b/js/queue_manager.js @@ -2,7 +2,7 @@ // eslint-disable-next-line func-names (function() { - 'use strict' + 'use strict'; class JobQueue { constructor() { diff --git a/libtextsecure/http-resources.js b/libtextsecure/http-resources.js index 4ac3ee38c..05b5d76e6 100644 --- a/libtextsecure/http-resources.js +++ b/libtextsecure/http-resources.js @@ -68,8 +68,8 @@ const jobQueue = new window.JobQueue(); const processMessages = async messages => { - const newMessages = await jobQueue.add( - () => filterIncomingMessages(messages) + const newMessages = await jobQueue.add(() => + filterIncomingMessages(messages) ); newMessages.forEach(async message => { const { data } = message;