Inconsisten usage of `_held`

[AndreiPashkin]

1) Here and here - duplicate check for case if a lock is already held - additional overhead. 2) Here - _held is not used, and if not - what is the point of it's existence? 3) Here - it is kind of duplicate, since nx=True is already ensures, that a lock will not override already held lock. And also it produces additional overhead, making additional call to Redis.

So _held produces additional calls to Redis in case of redundant checks or usage of _held, because _held calls to Redis. And it's just violates Feng Shui rules.

[ionelmc]

1) Here and here - duplicate check for case if a lock is already held.

Indeed. Will remove that.

2) Here - _held is not used, and if not - what is the point of it's existence?

So basically it's just used in acquire. Are you proposing to use a script in acquire?

[AndreiPashkin]

So basically it's just used in acquire. Are you proposing to use a script in acquire?

If remove _held call, acquire still won't override already held locks, because of nx=True, so call to _held is partially redundant. Partially - becasue nx=True doesn't perform matching by id's.

[ionelmc]

Yes but it won't raise proper exception either.

[AndreiPashkin]

Yes, but should it?

[ionelmc]

Yes, if user makes something bad then error should be raised instead of treating the problem silently.

[AndreiPashkin]

Another proposition is to make _held public.

[AndreiPashkin]

Yes, if user makes something bad then error should be raised instead of treating the problem silently.

Makes sense.

[ionelmc]

Another proposition is to make _held public.

Hmmm. threading.Lock has a method locked. I think we can match that.

[AndreiPashkin]

Yes, if user makes something bad then error should be raised instead of treating the problem silently.

By the way - currently - this check is performed only in the beginning, but not in blocked state. Should it throw the error in blocked state?

[ionelmc]

Good point. I have to investigate.

[ionelmc]

Uh ... well kinda late but: in acquire there is this check:

        if self._held:
            raise AlreadyAcquired("Already acquired from this Lock instance.")

So I think all problems are resolved?