Skip to content

Commit

Permalink
Delete the entry for the waiter when the timeout is reached
Browse files Browse the repository at this point in the history
When the lock times out the promise rejects, and the calling code will do "something sensible". Something sensible
could be to retry later -- but that actually requires the lock to be able to be acquired again. Right now this is
not possible, because the failed `tryToAcquire` is still inside the waiting queue -- and will never release the lock.
  • Loading branch information
ankon committed Apr 8, 2020
1 parent fe4d73d commit 86ef01b
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
10 changes: 6 additions & 4 deletions src/utils/lock.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@ module.exports = class Lock {
}

this[PRIVATE.WAITING].add(tryToAcquire)
timeoutId = setTimeout(
() => reject(new KafkaJSLockTimeout(this[PRIVATE.TIMEOUT_ERROR_MESSAGE]())),
this[PRIVATE.TIMEOUT]
)
timeoutId = setTimeout(() => {
// The message should contain the number of waiters _including_ this one
const error = new KafkaJSLockTimeout(this[PRIVATE.TIMEOUT_ERROR_MESSAGE]())
this[PRIVATE.WAITING].delete(tryToAcquire)
reject(error)
}, this[PRIVATE.TIMEOUT])
})
}

Expand Down
20 changes: 20 additions & 0 deletions src/utils/lock.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,26 @@ describe('Utils > Lock', () => {
).rejects.toHaveProperty('message', 'Timeout while acquiring lock (2 waiting locks)')
})

it('allows lock to be acquired after timeout', async () => {
const lock = new Lock({ timeout: 60 })
const resource = jest.fn()
const callResource = async () => {
await lock.acquire()
try {
resource(Date.now())
await sleep(100)
} finally {
lock.release()
}
}

await expect(
Promise.all([callResource(), callResource(), callResource()])
).rejects.toHaveProperty('message', 'Timeout while acquiring lock (2 waiting locks)')

await expect(callResource()).resolves.toBeUndefined()
})

describe('with a description', () => {
it('throws an error with the configured description if the lock cannot be acquired within a period', async () => {
const lock = new Lock({ timeout: 60, description: 'My test mock' })
Expand Down

0 comments on commit 86ef01b

Please sign in to comment.