report at bugs
Nov 7, 2009, 7:17 AM
Post #7 of 12
(85 views)
Permalink
|
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>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com
|
