mafintosh / mafintosh/mutexify
Discuss defined behaviour for "releasing" the same lock twice
- 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