await-lock icon indicating copy to clipboard operation
await-lock copied to clipboard

utility function `.acquireWhile`

Open brandonryan opened this issue 2 years ago • 3 comments

I end up implementing this utility function when using this library a lot, so I thought id make a suggestion. To make sure I don't forget unlocking, and to make it more visually apparent, I scope the lock logic in an async callback.

const lock = new AwaitLock()
await lock.acquireWhile(async () => {
    //do things with the guarded data
})

Basically aquireWhile would: 1: aquireAsync 2: await callback with try finally wrapper 3: unlock in finally

Alternate potential function names:

  • acquireDuring
  • access
  • acquireAwait

brandonryan avatar Dec 20 '21 09:12 brandonryan

Will need to think about this a bit. I think the API is good and often how you want to use the lock. Some pros:

  • Can't mess up by returning an unawaited promise from a try-block. This is a pitfall of the current API.
  • Acquiring and releasing the lock are symmetrically encapsulated in this API.

Some cons:

  • Adding a catch-block requires changing your code to use try-catch-finally instead of just adding in a catch-block. Typically APIs are nicer when adding an incremental feature requires just adding incremental code.
  • It's less evident when the passed-in function runs by just reading the code. This is like how for-loops tend to be better than forEach -- the body of the loop syntactically communicates it runs right away whereas a function passed to another function could technically run whenever and you need to read the API contract to understand when it runs.
  • It can be implemented in userspace -- generally prefer to put code in this library if it can't be implemented outside.

ide avatar Dec 21 '21 02:12 ide

Adding a catch-block requires changing your code to use try-catch-finally instead of just adding in a catch-block. Typically APIs are nicer when adding an incremental feature requires just adding incremental code.

Im not sure I understand this statement. Here is how I see this being used. Keeping in mind this is just an optional convenience feature. using acquireDuring cause i think it makes intent most clear

// *** regular way ***
const lock = new AwaitLock()
await lock.acquireAsync()
try {
    // Run async code in the critical section
    //Note: dont return promise!!!
} finally {
    lock.release()
}

// *** convenient way ***
await lock.aquireDuring(async () => {
    //do things with data (no catch or finally needed)
    //potentially throw errors
    //can return promise (inherent cause in async)
})

all i see here is removing the try entirely. Here is what I imagine the implementation looking like.

class AwaitLock {
    async aquireDuring(fn) {
        await this.acquireAsync()
        try  {
            //return for convenience in case they are calling a non inline function and they want the result.
            //any errors that get thrown from func just propegate up because we dont catch
            return await fn()
        } finally {
            await this.release()
        }
    }
}

brandonryan avatar Dec 24 '21 20:12 brandonryan

What I mean is that there is a more direct path from

await lock.acquireAsync();
try {
  ...
} finally {
  lock.release();
}

to

await lock.acquireAsync();
try {
  ...
+ } catch {
+  // Roll back work done in the critical section in order to restore invariants
} finally {
  lock.release();
}

than to start from

await lock.runWithLock(async () => {
  ...
});

ide avatar Dec 25 '21 00:12 ide

Closing this since it can be implemented in user-space e.g. sample usage could look like:

const lock = new ScopedLock(new AwaitLock());
await lock.acquireWhile(async () => {
  ...
});

ide avatar Jun 05 '23 20:06 ide