From b46ab15a3d32f9952fff3ba0d4f3dad350303aa3 Mon Sep 17 00:00:00 2001 From: iskryzhytskyi Date: Sun, 15 Mar 2020 17:08:34 +0200 Subject: [PATCH 1/5] feat: locking timeout --- test.js | 1 + test/lock-time.js | 18 ++++++++++++++++++ test/steps.js | 8 +------- test/test-utils.js | 10 ++++++++++ test/thread-worker.js | 6 +----- web-locks.js | 21 ++++++++++++++------- 6 files changed, 45 insertions(+), 19 deletions(-) create mode 100644 test/lock-time.js create mode 100644 test/test-utils.js diff --git a/test.js b/test.js index 8f48423..4853b65 100644 --- a/test.js +++ b/test.js @@ -8,6 +8,7 @@ const tests = [ 'deadlock', 'recursive-deadlock', 'thread-main', + 'lock-time', ]; (async () => { diff --git a/test/lock-time.js b/test/lock-time.js new file mode 100644 index 0000000..345cab3 --- /dev/null +++ b/test/lock-time.js @@ -0,0 +1,18 @@ +'use strict'; + +const assert = require('assert').strict; +const { locks } = require('..'); +const { sleep } = require('./test-utils'); + +const TIME_TO_PROCESS = 2000; +const TIME_TO_LOCK = 100; + +module.exports = async () => { + const startTs = Date.now(); + + await locks.request('LockTime', { timeout: TIME_TO_LOCK }, async () => { + await sleep(TIME_TO_PROCESS); + }); + + assert.strictEqual(Date.now() - startTs < TIME_TO_PROCESS, true); +}; diff --git a/test/steps.js b/test/steps.js index 7ceb9b7..71b5642 100644 --- a/test/steps.js +++ b/test/steps.js @@ -2,13 +2,7 @@ const assert = require('assert').strict; const { locks } = require('..'); - -const sleep = msec => - new Promise(resolve => { - setTimeout(() => { - resolve(); - }, msec); - }); +const { sleep } = require('./test-utils'); let counter = 0; diff --git a/test/test-utils.js b/test/test-utils.js new file mode 100644 index 0000000..c1aca09 --- /dev/null +++ b/test/test-utils.js @@ -0,0 +1,10 @@ +'use strict'; + +const sleep = msec => + new Promise(resolve => { + setTimeout(resolve, msec); + }); + +module.exports = { + sleep, +}; diff --git a/test/thread-worker.js b/test/thread-worker.js index 03bb534..b9b9c92 100644 --- a/test/thread-worker.js +++ b/test/thread-worker.js @@ -1,14 +1,10 @@ 'use strict'; const threads = require('worker_threads'); +const { sleep } = require('./test-utils'); const { locks } = require('..'); -const sleep = msec => - new Promise(resolve => { - setTimeout(resolve, msec); - }); - (async () => { if (threads.threadId === 2) { await sleep(10); diff --git a/web-locks.js b/web-locks.js index ae40d86..bdf728d 100644 --- a/web-locks.js +++ b/web-locks.js @@ -11,12 +11,13 @@ const UNLOCKED = 1; let locks = null; // LockManager instance class Lock { - constructor(name, mode = 'exclusive', buffer = null) { + constructor({ name, mode = 'exclusive', buffer = null, timeout = null }) { this.name = name; this.mode = mode; // 'exclusive' or 'shared' this.queue = []; this.owner = false; this.trying = false; + this.timeout = timeout; this.buffer = buffer ? buffer : new SharedArrayBuffer(4); this.flag = new Int32Array(this.buffer, 0, 1); if (!buffer) Atomics.store(this.flag, 0, UNLOCKED); @@ -39,10 +40,16 @@ class Lock { this.owner = true; this.trying = false; const { handler, resolve } = this.queue.shift(); - handler(this).finally(() => { + + const endWork = () => { this.leave(); resolve(); - }); + }; + + if (typeof this.timeout === 'number') { + setTimeout(endWork, this.timeout); + } + handler(this).finally(endWork); } leave() { @@ -89,11 +96,11 @@ class LockManager { handler = options; options = {}; } - const { mode = 'exclusive', signal = null } = options; + const { mode = 'exclusive', signal = null, timeout } = options; let lock = this.collection.get(name); if (!lock) { - lock = new Lock(name, mode); + lock = new Lock({ name, mode, timeout }); this.collection.set(name, lock); const { buffer } = lock; const message = { webLocks: true, kind: 'create', name, mode, buffer }; @@ -147,9 +154,9 @@ class LockManager { receive(message) { if (!message.webLocks) return; - const { kind, name, mode, buffer } = message; + const { kind, name, mode, buffer, timeout } = message; if (kind === 'create') { - const lock = new Lock(name, mode, buffer); + const lock = new Lock({ name, mode, buffer, timeout }); this.collection.set(name, lock); return; } From 07c738793ee6f2ca9fcf9f3640df5c690a8ff10e Mon Sep 17 00:00:00 2001 From: iskryzhytskyi Date: Mon, 16 Mar 2020 09:09:16 +0200 Subject: [PATCH 2/5] fix: remove timeout from Lock and add it to Lock.queue element --- test/lock-time.js | 2 +- web-locks.js | 21 ++++++++++----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/test/lock-time.js b/test/lock-time.js index 345cab3..5ec2017 100644 --- a/test/lock-time.js +++ b/test/lock-time.js @@ -4,7 +4,7 @@ const assert = require('assert').strict; const { locks } = require('..'); const { sleep } = require('./test-utils'); -const TIME_TO_PROCESS = 2000; +const TIME_TO_PROCESS = 200; const TIME_TO_LOCK = 100; module.exports = async () => { diff --git a/web-locks.js b/web-locks.js index bdf728d..eedac64 100644 --- a/web-locks.js +++ b/web-locks.js @@ -11,21 +11,20 @@ const UNLOCKED = 1; let locks = null; // LockManager instance class Lock { - constructor({ name, mode = 'exclusive', buffer = null, timeout = null }) { + constructor(name, mode = 'exclusive', buffer = null) { this.name = name; this.mode = mode; // 'exclusive' or 'shared' this.queue = []; this.owner = false; this.trying = false; - this.timeout = timeout; this.buffer = buffer ? buffer : new SharedArrayBuffer(4); this.flag = new Int32Array(this.buffer, 0, 1); if (!buffer) Atomics.store(this.flag, 0, UNLOCKED); } - enter(handler) { + enter(handler, timeout) { return new Promise(resolve => { - this.queue.push({ handler, resolve }); + this.queue.push({ handler, resolve, timeout }); this.trying = true; setTimeout(() => { this.tryEnter(); @@ -39,15 +38,15 @@ class Lock { if (prev === LOCKED) return; this.owner = true; this.trying = false; - const { handler, resolve } = this.queue.shift(); + const { handler, resolve, timeout } = this.queue.shift(); const endWork = () => { this.leave(); resolve(); }; - if (typeof this.timeout === 'number') { - setTimeout(endWork, this.timeout); + if (timeout) { + setTimeout(endWork, timeout); } handler(this).finally(endWork); } @@ -100,14 +99,14 @@ class LockManager { let lock = this.collection.get(name); if (!lock) { - lock = new Lock({ name, mode, timeout }); + lock = new Lock(name, mode); this.collection.set(name, lock); const { buffer } = lock; const message = { webLocks: true, kind: 'create', name, mode, buffer }; locks.send(message); } - const finished = lock.enter(handler); + const finished = lock.enter(handler, timeout); let aborted = null; if (signal) { aborted = new Promise((resolve, reject) => { @@ -154,9 +153,9 @@ class LockManager { receive(message) { if (!message.webLocks) return; - const { kind, name, mode, buffer, timeout } = message; + const { kind, name, mode, buffer } = message; if (kind === 'create') { - const lock = new Lock({ name, mode, buffer, timeout }); + const lock = new Lock(name, mode, buffer); this.collection.set(name, lock); return; } From f9099d46a9cbe9ef27a934167ec680927d0cd607 Mon Sep 17 00:00:00 2001 From: iskryzhytskyi Date: Mon, 16 Mar 2020 15:17:58 +0200 Subject: [PATCH 3/5] refactor: add the clear timeout logic specify imported file extensions --- test/lock-time.js | 31 ++++++++++++++++++++++--------- test/steps.js | 2 +- test/thread-worker.js | 2 +- web-locks.js | 15 ++++++++++----- 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/test/lock-time.js b/test/lock-time.js index 5ec2017..c9119d8 100644 --- a/test/lock-time.js +++ b/test/lock-time.js @@ -2,17 +2,30 @@ const assert = require('assert').strict; const { locks } = require('..'); -const { sleep } = require('./test-utils'); - -const TIME_TO_PROCESS = 200; -const TIME_TO_LOCK = 100; +const { sleep } = require('./test-utils.js'); module.exports = async () => { - const startTs = Date.now(); + await (async () => { + const caseName = 'Should release a resource after timeout deadline'; + const startTs = Date.now(); + + await locks.request('LockTime', { timeout: 100 }, async () => { + await sleep(200); + }); + + const isCaseSuccess = Date.now() - startTs < 200; + assert.strictEqual(isCaseSuccess, true, caseName); + })(); + + await (async () => { + const caseName = 'Should release a resource after handler complete a task'; + const startTs = Date.now(); - await locks.request('LockTime', { timeout: TIME_TO_LOCK }, async () => { - await sleep(TIME_TO_PROCESS); - }); + await locks.request('LockTime', { timeout: 200 }, async () => { + await sleep(100); + }); - assert.strictEqual(Date.now() - startTs < TIME_TO_PROCESS, true); + const isCaseSuccess = Date.now() - startTs < 200; + assert.strictEqual(isCaseSuccess, true, caseName); + })(); }; diff --git a/test/steps.js b/test/steps.js index 71b5642..e09f3be 100644 --- a/test/steps.js +++ b/test/steps.js @@ -2,7 +2,7 @@ const assert = require('assert').strict; const { locks } = require('..'); -const { sleep } = require('./test-utils'); +const { sleep } = require('./test-utils.js'); let counter = 0; diff --git a/test/thread-worker.js b/test/thread-worker.js index b9b9c92..ab339dd 100644 --- a/test/thread-worker.js +++ b/test/thread-worker.js @@ -1,7 +1,7 @@ 'use strict'; const threads = require('worker_threads'); -const { sleep } = require('./test-utils'); +const { sleep } = require('./test-utils.js'); const { locks } = require('..'); diff --git a/web-locks.js b/web-locks.js index eedac64..aeaa2bf 100644 --- a/web-locks.js +++ b/web-locks.js @@ -38,17 +38,22 @@ class Lock { if (prev === LOCKED) return; this.owner = true; this.trying = false; + let timer; + const { handler, resolve, timeout } = this.queue.shift(); - const endWork = () => { + const finalize = () => { this.leave(); + if (timer !== undefined) { + clearTimeout(timer); + timer = undefined; + } resolve(); }; - if (timeout) { - setTimeout(endWork, timeout); - } - handler(this).finally(endWork); + if (timeout) timer = setTimeout(finalize, timeout); + + handler(this).finally(finalize); } leave() { From 364e6b71e8d8e0e729d1fd2c8e6c29f2c1390a82 Mon Sep 17 00:00:00 2001 From: iskryzhytskyi Date: Mon, 16 Mar 2020 15:50:32 +0200 Subject: [PATCH 4/5] refactor: move errors to separate file --- errors.js | 20 ++++++++++++++++++++ web-locks.js | 8 +------- 2 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 errors.js diff --git a/errors.js b/errors.js new file mode 100644 index 0000000..7afeee7 --- /dev/null +++ b/errors.js @@ -0,0 +1,20 @@ +'use strict'; + +class AbortError extends Error { + constructor(message) { + super(message); + this.name = 'AbortError'; + } +} + +class TimeoutError extends Error { + constructor(message) { + super(message); + this.name = 'TimeoutError'; + } +} + +module.exports = { + AbortError, + TimeoutError, +}; diff --git a/web-locks.js b/web-locks.js index aeaa2bf..24d94c9 100644 --- a/web-locks.js +++ b/web-locks.js @@ -2,6 +2,7 @@ const { EventEmitter } = require('events'); const threads = require('worker_threads'); +const { AbortError, TimeoutError } = require('./errors'); const { isMainThread, parentPort } = threads; const isWorkerThread = !isMainThread; @@ -174,13 +175,6 @@ class LockManager { } } -class AbortError extends Error { - constructor(message) { - super(message); - this.name = 'AbortError'; - } -} - class AbortSignal extends EventEmitter { constructor() { super(); From ed212e40e6a1c4c2c1e3faa787b0ab03f8413786 Mon Sep 17 00:00:00 2001 From: iskryzhytskyi Date: Mon, 16 Mar 2020 15:51:39 +0200 Subject: [PATCH 5/5] feat: clean timeout timer on abort, throw error on timeout --- test/lock-time.js | 23 ++++++++++++++++++++--- web-locks.js | 30 +++++++++++++----------------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/test/lock-time.js b/test/lock-time.js index c9119d8..70e836f 100644 --- a/test/lock-time.js +++ b/test/lock-time.js @@ -3,20 +3,37 @@ const assert = require('assert').strict; const { locks } = require('..'); const { sleep } = require('./test-utils.js'); +const { TimeoutError } = require('../errors.js'); module.exports = async () => { await (async () => { const caseName = 'Should release a resource after timeout deadline'; const startTs = Date.now(); - await locks.request('LockTime', { timeout: 100 }, async () => { - await sleep(200); - }); + await locks + .request('LockTime', { timeout: 100 }, async () => { + await sleep(200); + }) + .catch(() => {}); const isCaseSuccess = Date.now() - startTs < 200; assert.strictEqual(isCaseSuccess, true, caseName); })(); + await (async () => { + const caseName = 'Should release a resource and throw the TimeoutError'; + let error; + + await locks + .request('LockTime', { timeout: 100 }, async () => { + await sleep(200); + }) + .catch(err => (error = err)); + + const isCaseSuccess = error instanceof TimeoutError; + assert.strictEqual(isCaseSuccess, true, caseName); + })(); + await (async () => { const caseName = 'Should release a resource after handler complete a task'; const startTs = Date.now(); diff --git a/web-locks.js b/web-locks.js index 24d94c9..2774504 100644 --- a/web-locks.js +++ b/web-locks.js @@ -23,9 +23,9 @@ class Lock { if (!buffer) Atomics.store(this.flag, 0, UNLOCKED); } - enter(handler, timeout) { - return new Promise(resolve => { - this.queue.push({ handler, resolve, timeout }); + enter(handler, timeout, signal) { + return new Promise((resolve, reject) => { + this.queue.push({ handler, resolve, reject, timeout, signal }); this.trying = true; setTimeout(() => { this.tryEnter(); @@ -41,18 +41,21 @@ class Lock { this.trying = false; let timer; - const { handler, resolve, timeout } = this.queue.shift(); + const { handler, resolve, reject, timeout, signal } = this.queue.shift(); - const finalize = () => { + const finalize = error => { this.leave(); if (timer !== undefined) { clearTimeout(timer); timer = undefined; } - resolve(); + if (error) reject(error); + else resolve(); }; - if (timeout) timer = setTimeout(finalize, timeout); + if (signal) signal.on('abort', finalize); + if (timeout) + timer = setTimeout(finalize, timeout, new TimeoutError('Time Out')); handler(this).finally(finalize); } @@ -112,16 +115,9 @@ class LockManager { locks.send(message); } - const finished = lock.enter(handler, timeout); - let aborted = null; - if (signal) { - aborted = new Promise((resolve, reject) => { - signal.on('abort', reject); - }); - await Promise.race([finished, aborted]); - } else { - await finished; - } + const finished = lock.enter(handler, timeout, signal); + + await finished; setTimeout(() => { lock.tryEnter();