On 01/15/10 05:18 PM, Edward Pilatowicz wrote:
src/modules/client/api_errors.py
- i'm not a python expert, so perhaps this is ok, but all the __init__
parameters to ImageLockedError default to None, and when returning
an error string you only check if pid and pid_name are None.
shouldn't you also check if hostname is None?
I could, it doesn't really hurt though.
src/modules/client/image.py
- you're privous comments regarding Image.__lock indicate that it is a
debug mechanism used to prevent recursive lock acquisition within a
process. if my understanding is correct, the following code is
Not quite; I was referring to the type of the lock, which in this case
it's an NRLock to prevent recursive lock acquisition.
The actual purpose of the lock itself is simply to prevent access by
different threads to the same image object.
incorrect:
# First, attempt to obtain a thread lock.
if not self.__lock.acquire(blocking=blocking):
raise api_errors.ImageLockedError()
with this code you will:
- fail if a thread tries to lock an image twice. good.
- if another thread has already locked the image and
blocking is true, you will hang. bad.
It's supposed to hang if another thread has already locked the image and
blocking is True. That's what blocking means. That's also why the
blocking property has a warning about this behaviour in the
ImageInterface class.
But I'll assume that this was a result of the misunderstanding above.
- if another thread has already locked the image and
blocking is false, you will return ImageLockedError.
also bad.
No, that's the intended behaviour.
- could you file a bug on the lack of read operation locking?
I'd rather not, because I'd rather wait until we actually identify one
or more specific cases where we *know* (not hypothetically) that we need
read locking and address it that way.
Right now, it'd be some vague hand-waving about "I'm sure, somewhere,
somehow, we might need read-locking but I don't have a firm example of why".
Cheers,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
