Jeffrey Yasskin <jyass...@gmail.com> added the comment:
This patch doesn't apply cleanly against the py3k tree. Since Python 2.7 is in
beta, and there's no 2.8, this can only go into python 3, so you should work
against that tree.
It's a bit annoying that the R in RWLock stands for a different word from in
RLock. But I can't think of a better name.
Hm, the test for RWLock should use a Barrier to check for multiple readers.
I wonder if it makes sense to add a DeadlockError as a subclass of RuntimeError
and throw that from trying to upgrade a read-lock to a write-lock.
The docs should describe why upgrading a read-lock to a write-lock is banned,
or else people will try to add the feature.
test_wrrecursion should also check pure writer recursion. Oh, no, that's tested
by RLockTests. Comment that please. The name of the test should probably
mention write_then_read_recursion to distinguish it from write-then-write or
read-then-write.
test_readers_writers doesn't quite test enough. (Feel free to add another test
rather than changing this one.) You want to test that readers can't starve
writers. For example, say we have:
lock = RWLock()
done = False
def r():
with lock.rdlocked():
if done:
return
_wait()
def w():
nonlocal done
with lock.wrlocked():
done = True
readers = Bunch(r, 50)
_wait()
writers = Bunch(w, 1)
readers.wait_for_finished()
writers.wait_for_finished()
In a naive implementation, there may never be a point where no reader has the
lock held, and so the writer may never get in to tell them all to exit. I
believe your implementation will already pass this test.
spelling: mathced
I'm not sure that "#threads will be few" is true for a read-lock, but I don't
mind for the first implementation.
Generally put a space after # when it introduces a comment, start comments with
a capital letter, and end them with punctuation so we know you didn't stop
typing early.
In _rdlock could you flip the "not self.nw" condition? That way the else won't
be a double-negative.
Can self.rcond.wait() ever throw an exception? I suspect you should use
try:finally: to guard the "self.nr -= 1"
Why must the user hold a write lock to use Condition.wait? Semantically, a
waiting on a read-lock would release the current thread's recursive read-locks,
and all should be well.
It looks like self.owning holds one copy of each thread's id for each re-entry
by that thread. That wasn't obvious so deserves a comment.
General API questions:
1. Given that the other locking APIs use "acquire" and "release" instead of
"lock" and "unlock", perhaps this class should use "rdacquire" and "wracquire"?
2. I wonder if it helps actual code to be able to just call "unlock" instead
of "rdunlock" and "wrunlock". Code releasing the lock should always know which
kind of lock it holds.
Otherwise, this looks basically correct. Thanks!
----------
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue8800>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
