On Mon, Dec 18, 2006 at 11:05:15AM +0100, Tim Dijkstra wrote: > On Mon, 18 Dec 2006 00:33:58 +0100 > Bill Allombert <[EMAIL PROTECTED]> wrote: > > > > > What worry me about the patch is the fact that create_lock() and > > > > check_dpkglock() are not performed in the same order. In particular, if > > > > create_lock() fail we exit with error 0 instead of 1 thus maybe > > > > concealing a real error. > > > > > > You used to exit with 1 only if both creat_lock and check_dpkglock were > > > false at the same time. If that is what you want, that is easy to fix. > > > New patch attached. The locking logic is now the same as in the original. > > > > If I am not mistaken, your new patch adds a race condition between > > create_lock() and check_dpkglock(). If dpkg release the lock after > > create_lock() and before check_dpkglock(), then we do exit(1) while we > > should not. In the original version, check_dpkglock was checked first > > so this race condition was not possible. > > I don't think that can happen. In normal circumstances we are a child > process of dpkg until the fork. So it can't release the lock in between > those two calls; it is waiting for us.
You are absolutly correct: this cannot happen in normal circumstances. > Anyway, in the attached patch I swapped the order of check_dpkglock and > create_lock again to match the original. This one looks fine, thanks. > > Sorry to be so slow dealing with this issue... > > That's OK. I'm happy we're dealing with it know. And it is good you are > reviewing the patch thoroughly. I do not have much choice, since i cannot really test it. Cheers, -- Bill. <[EMAIL PROTECTED]> Imagine a large blue swirl here. -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
