On Wed, Jan 13, 2010 at 08:06:03PM -0600, Shawn Walker wrote: > On 01/13/10 07:58 PM, [email protected] wrote: > >If you're trying to prevent concurrent access to the image object by the > >same threads in the process, it sounds like you're duplicating a lot of > >the functionality of the API's activity lock. Would you look at how > >the functionality of the activity lock and the image lock can be > >commonized? I think that the activity lock may be redundant if you're > >using the image's NRLock to serialize. > > 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 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? > >>>>I'm still not convinced that there isn't value here in the timeout > >>>>logic. Since the logic here is nearly identical to that used by > >>>>python's own mechanism for threading conditions, couldn't the same > >>>>problems happen? > >>> > >>>I don't understand why the timeout is necessary. Either the caller > >>>wants to grab the lock and is willing to wait its turn, or the caller > >>>wants to try the lock and return a suitable error message if it is > >>>already locked. The timeout doesn't do application code any good, since > >> > >>Even if I took the approach of "wait" or "don't wait", how do I > >>ensure callers have control over how long to wait without doing > >>this? > > > >What caller needs control over how long to wait? I tried to lay out the > >cases where I thought this lock might get contended, but there didn't > >seem to be a plausible explanation for a timeout. Either the caller > >needs the lock and must wait, or the caller doesn't want to wait and > >should return. What's the use case for a timeout for the API's consumers? > > 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. > 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 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. -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
