[issue3001] RLock's are SLOW

[Antoine Pitrou]

Antoine Pitrou <pitrou at free.fr> added the comment:

> rlock_acquire_doc: "(...) return None once the lock is acquired".
> That's wrong, acquire() always return a boolean (True or False).

You're right, I should fix that docstring.

> rlock_release(): Is the assert(self->rlock_count > 0); really required?
> You're checking its value few lines before.

Right again :) I forgot this was unsigned.

> rlock_acquire_restore(): raise an error if the lock acquire failed,
> whereas rlock_acquire() doesn't. Why not raising an error in
> rlock_acquire() (especially if blocking is True)?

For rlock_acquire(), I mimicked what the Python code (_PyRLock.acquire)
does: if locking fails, it returns False instead. It is part of the API.

(and I agree this is not necessarily right, because failing to lock if
blocking is True would probably indicate a low-level system error, but
the purpose of the patch is not to change the API)

But you're right that the Python version of rlock_acquire_restore()
doesn't check the return code either, so I may remove this check from
the C code, although the rest of the method clearly assumes the lock has
been taken.

> rlock_acquire_restore(): (maybe) set owner to 0 if count is 0.
> 
> rlock_release_save(): may also reset self->rlock_owner to 0, as
> rlock_release().

As I said to Gregory, the current code doesn't rely on rlock_owner to be
reset but, yes, we could still add it out of safety.

> rlock_new(): why not reusing type instead of Py_TYPE(self) in
> "Py_TYPE(self)->tp_free(self)"?

Good point.

> __exit__: rlock_release() is defined as __exit__() with METH_VARARGS,
> but it has no argument. Can it be a problem?

I just mimicked the corresponding declarations in the non-recursive lock
object. Apparently it's safe on all architectures Python has been
running on for years, although I agree it looks strange.

> If your patch is commited, you can reconsider #3618 (possible deadlock
> in python IO implementation).

Indeed.

Thanks !

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue3001>
_______________________________________