Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Mutex.lock(key, timeout, cb) function #12

Closed
wants to merge 1 commit into from

Conversation

zlumer
Copy link

@zlumer zlumer commented Jun 9, 2016

Solves #11
Usage:

var lock = require('locks').Mutex.lock;
lock('user_123', function(done) {
  function doSomethingAsync() {
    // ... do something async
    done();
  }
  setTimeout(doSomethingAsync, 500);
});

Custom timeout:

var lock = require('locks').Mutex.lock;
lock('user_123', 20000, function(done) {
  function doSomethingAsync() {
    // ... do something async
    done();
  }
  setTimeout(doSomethingAsync, 10000);
});

@zlumer
Copy link
Author

zlumer commented Jun 9, 2016

Inspired by redis-lock module.
Based on the code by @tjenkinson.

@gitawego
Copy link

I think it's a quite useful addition, please consider to merge it.

@ronkorving
Copy link
Collaborator

I see the value in this. But I don't think this belongs inside this repo. Some people may want to lock on objects, not strings, (as you can do in C# if I'm not mistaken). I'm also not a fan of the default timeout hard-coded in this project. To me, those are userland decisions.

I think that increasing the scope of this project should be limited to adding functionality that can't be easily created around it. In this case, it can be. Also, please keep in mind that Mutex is just one of the various synchronization, so if we would go down this path, I would like it to be a more fundamental improvement to the entire library.

Sorry to disappoint, and while I appreciate the engagement, for the sake of protecting the scope of this project, I'm going to close this.

@ronkorving ronkorving closed this Nov 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants