On 01/13/10 08:16 PM, [email protected] wrote:
On Wed, Jan 13, 2010 at 08:06:03PM -0600, Shawn Walker wrote:
Except I'm not. This ensures that internal callers that lock the
image are behaving correctly.
I could do this with a simple boolean flag instead, but that seems
pointless since NRLock() does what I want already.
Again, the whole purpose of using NRLock() is to expose bad internal
logic that is attempting to lock the current image multiple times.
It helped me when I was debugging immensely.
Ok, I get that you're using this as a debugging aid. However, isn't it
true that all of the cases where we must grab the activity lock we also
grab the image's NRLock? If so, then you're taking another lock
No, it isn't, especially since not everything goes through the
pkg.client.api yet.
Not only that, even if you exclude the NRLock, there's still the issue
that I don't want lock() done multiple times for the lock file.
In short, this is left there so that we explode with failure
(purposefully) if there's some misbehaving lock logic somewhere.
unnecessarily. Unless I've misunderstood, can't we have the NRLock do
double-duty as the enforcer of recursive image-locking, and as the
object that serializes concurrent access to the image?
There's two separate locks here. There's an API lock that the API uses
for its own objects, and then there's a image lock that's needed for
changing the image.
Acquiring an activity lock does not imply the image is locked, and
vice-versa.
As Danek pointed out though, a caller may not want to wait forever.
We don't know how clients will use the API, and so we can't guess
what their use case is.
It sounded like Danek was talking about the case where an unprivileged
process blocks out a privileged one that's trying to modify the image.
I look at it as the same reason why transport has a tuneable timeout
mechanism. If you don't have one, you're completely at the whims of the
underlying OS' default timeout mechanism. I, personally, would not want
to call a function that blocks unless I had some reasonable guarantee
that it wouldn't wait longer than a certain amount of time.
I have run into bugs in the API in the past where it wasn't releasing a
lock and then would wait forever (I gave up after a minute or so) while
attempting to acquire a lock.
My fear is that blocking on the lock will lead to unfortunate issues
where the client just hangs waiting because of some locking bug.
My use case was that I didn't want pkg(1) to immediately exit
needlessly with a lock error, so the timeout was my attempt to
mitigate that. I just didn't want to shove the responsibility of
timeout code onto callers and felt it was nice to place it into the
api instead.
It was my earnest attempt to slightly reduce potential user
frustration of "it said it was locked by pid XXX, but I don't see
one!". In other words, trying to avoid the "execute pkg in a loop
until the lock is available."
IMO, you can still do this. In the CLI it should be perfectly okay to
block. You can perform a trylock first, and if that fails, print a
message saying, "Pid XXX holds this lock. We're waiting for it. Press
CTRL-C or kill the offending process if you don't feel like waiting."
I don't think that behaviour lends itself well to automated scripting or
the test suite. If we discover that we need this enhancement, we can do
so later.
I'm just not comfortable implementing that kind of behaviour right now.
I still don't agree that a timeout isn't potentially useful, but
since there appear to be no firm, existing use cases for it I won't
keep the logic. I'll post a new webrev shortly that always attempts
a non-blocking lock and fails immediately if it can't acquire one.
I don't think you want a non-blocking only approach. Let the caller
decide whether to block. IMO, the GUI should use this in non-blocking
mode unless they're prepared to wait. The CLI can block and let the
user intervene if they're tired of waiting.
For now, I don't have sufficient motivation to provide control over
blocking behaviour. If we find that clients need it, we can add it later.
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
