Antoine Pitrou <pit...@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 <rep...@bugs.python.org> <http://bugs.python.org/issue3001> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
