mafintosh / mafintosh/mutexify

Discuss defined behaviour for "releasing" the same lock twice

Open
#11 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
9
PR merge metrics
No merged PRs in 30d

Description

Hello!

I paged through past issues and PRs and didn't see anything, so I'd like to start a discussion on defining explicit behaviour around releasing the same lock twice. Consider the following "strange" code where a lock is released twice:

```javascript
const mutexify = require('mutexify')
const lock = mutexify()

const release = await lock()

release()
release() // Fails with TypeError
```

```
$ node mutexify-repro.js
/node_modules/mutexify/index.js:6
used(release)
^

TypeError: used is not a function
at call (/node_modules/mutexify/index.js:6:5)
at processTicksAndRejections (internal/process/task_queues.js:79:11)
```

I think it's reasonable to say that this is an explicit mis-use of the library. However, I don't like that the error message doesn't lead the developer towards a solution. Once we `release()` the first time, `used` becomes undefined and then the second `release()` fails not because we have an opinion that "releasing a mutex lock twice is bad form", but because of a type error in the language.

Another option would be to make `release()` `noop` when `used` is undefined. So sure, it'll "release" a lock multiple times, but the first one is all that mattered.

What do you think about the following options?

1. If a lock is "released" twice, it throws an explicit error that this operation is not supported, rather than falling through to a `TypeError`
2. If a lock is "released" twice, it simply returns on release calls after the first and effectively does nothing.

For our stuff, I don't think it really matters all the much which way we go in the library (if any change is made at all) because we can re-organize around a decision. For more information, the flow that actually produced our error was something more like:

```javascript
const release = await lock()

try {
// Do something requiring that lock

release()

logger.warn('This WARN should have been logger.warning and caused a error that triggers the catch.')
} catch (error) {
// After the syntax error, the lock is released a second time, causing the TypeError
release()
}
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the release path in node_modules/mutexify/index.js and reproduce the behavior using mutexify-repro.js. Review the two proposed options and establish an agreed behavior for repeated release calls; done means the project has a decided, explicit outcome rather than the current incidental TypeError.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.