Re: Bug#642608: /usr/bin/dpkg-gencontrol: Race condition with tempfile for parallel builds
[ CCing debian-perl for a RFP ]
On Tue, 11 Oct 2011, Guillem Jover wrote:
> On Tue, 2011-10-11 at 08:34:48 +0200, Raphael Hertzog wrote:
> > On Mon, 10 Oct 2011, Guillem Jover wrote:
> > > Also we should be using fcntl(2) instead of flock(2) to get NFS support,
> > > but this seems tricky on perl?
> >
> > Not really, it's a built-in function on systems that support it. The
> > question is whether we have to care about systems that do not support it.
>
> I'm assuming you mean it's not tricky. But to lock using fcntl(2) the
> function needs a struct flock which would need to be packed from the
> perl code, but the order of its members is not standardized anywhere,
> and sizeof(off_t) might vary, that's what seems tricky to me.
Oh, right, I did not dig that far. I just checked its availability.
> The easy solution to this would seem to be to package something like
> the perl File::FcntlLock module:
>
> <http://search.cpan.org/~jtt/File-FcntlLock-0.12/>
Dear Debian perl team members, it would be nice if one of you could
package this module. :-)
> I also considered as a possibility rewritting dpkg-distaddfile in C
> or creating a trival C helper to update debian/files, which I started
> locally but discared as it seems File::FcntlLock is a way way better
> solution, better even than reinventing ourselves such XS perl module,
> and avoids needing to create a new arch:any package to put any of
> those in.
It adds a new dependency to dpkg-dev but I guess it's ok since that
module must be relatively trivial and portable.
> Availability of the interface here is not an issue as mentioned before
> dpkg just needs fcntl(2), but portability is, given that we cannot
> just hardcode struct flock layout in the perl code. Also using perl's
> flock makes it an unreliable interface as it depends on what perl is
> going to use internally, and it might end up using the wrong underlying
> interface.
The perl we have would use flock() since it's not built with -Du_flock.
So it would indeed not work over NFS.
> > I have another idea to propose. Create "debian/files.new" with
> > O_CREAT|O_EXCL and if it fails with EEXIST, sleep for Xms and try again.
> > It's not optimal in the sense that we're not going to sleep
> > exactly the required amount of time, but at least it doesn't involve
> > messing with unrelated files.
>
> Besides the loop being somewhat suboptimal, usage of “O_CREAT | O_EXCL”
> on NFS might be unreliable.
The documentation mentions a work around:
Portable programs that want to perform atomic file locking using a lockfile,
and need to avoid reliance on NFS support for O_EXCL, can create a unique
file on the same file system (e.g., incorporating hostname and PID), and use
link(2) to make a link to the lockfile. If link(2) returns 0, the lock is
successful. Otherwise, use stat(2) on the unique file to check if its link
count has increased to 2, in which case the lock is also successful.
Would this be better?
If I get this right it's:
while (1) {
open(LOCK, ">", "debian/files.new.$$")
my $ok = link("debian/files.new.$$", "debian/files.new")
if (!ok) {
my @st = stat("debian/files.new.$$") || syserr("");
$ok = 1 if $st[3] == 2;
}
unlink("debian/files.new.$$") || syserr("");
last if $ok;
sleep(1); # Or rather usleep from Time::HiRes
}
I'm not sure what case it covers to check for the link count on a failure...
Cheers,
--
Raphaël Hertzog ◈ Debian Developer
Pre-order a copy of the Debian Administrator's Handbook and help
liberate it: http://debian-handbook.info/go/ulule-rh/
Reply to: