[issue3001] RLock's are SLOW
report at bugs.python.org
Sat Nov 7 16:17:16 CET 2009
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
> rlock_acquire_restore(): (maybe) set owner to 0 if count is 0.
> rlock_release_save(): may also reset self->rlock_owner to 0, as
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
> __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).
Python tracker <report at bugs.python.org>
More information about the Python-bugs-list