summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarc Espie <espie@cvs.openbsd.org>2006-03-08 11:22:03 +0000
committerMarc Espie <espie@cvs.openbsd.org>2006-03-08 11:22:03 +0000
commitaa348e4fadad6d83057511f21244ced70a93c6e6 (patch)
treedb367e0df2b1862baa3c86c3dd2861ab1b1b1716
parent7c58ec5d2fa82ef5959dfc1408f22d1b1f1a4ccc (diff)
fix race condition in SCP for real.
We can't control a grand-child death through gzip, so stop spawning grand-children: create two children connected by hand through a pipe, and when we close gzip, explicitly wait for the second child to die as well. This avoids race conditions between sigpipe and sigusr1. (and as usual, the resulting code is easier to follow once you get through the pipe/fork). Thanks theo for the comment. ;-)
-rw-r--r--usr.sbin/pkg_add/OpenBSD/PackageRepository.pm74
-rw-r--r--usr.sbin/pkg_add/OpenBSD/PackageRepository/SCP.pm48
2 files changed, 45 insertions, 77 deletions
diff --git a/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm b/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm
index c04ab7b5d57..870b1e197ed 100644
--- a/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm
+++ b/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm
@@ -1,5 +1,5 @@
# ex:ts=8 sw=4:
-# $OpenBSD: PackageRepository.pm,v 1.9 2006/03/07 17:25:47 espie Exp $
+# $OpenBSD: PackageRepository.pm,v 1.10 2006/03/08 11:22:02 espie Exp $
#
# Copyright (c) 2003-2004 Marc Espie <espie@openbsd.org>
#
@@ -87,6 +87,7 @@ sub close
{
my ($self, $object, $hint) = @_;
close($object->{fh}) if defined $object->{fh};
+ waitpid($object->{pid2}, 0) if defined $object->{pid2};
$self->parse_problems($object->{errors}, $hint)
if defined $object->{errors};
undef $object->{errors};
@@ -366,6 +367,7 @@ sub pkg_copy
my $handler = sub {
my ($sig) = @_;
unlink $filename;
+ close($in);
$SIG{$sig} = 'DEFAULT';
kill $sig, $$;
};
@@ -407,6 +409,7 @@ sub pkg_copy
} else {
unlink $filename;
}
+ close($in);
}
sub open_pipe
@@ -417,48 +420,59 @@ sub open_pipe
$object->{errors} = OpenBSD::Temp::file();
$object->{cache_dir} = $ENV{'PKG_CACHE'};
$object->{parent} = $$;
+
+ my ($rdfh, $wrfh);
+ pipe($rdfh, $wrfh);
+
my $pid = open(my $fh, "-|");
if (!defined $pid) {
die "Cannot fork: $!";
}
if ($pid) {
$object->{pid} = $pid;
- $object->{pid2} = <$fh>;
- return $fh;
} else {
- open STDERR, '>', $object->{errors};
-
- my $pid2 = open(STDIN, "-|");
+ open(STDIN, '<&', $rdfh) or die "Bad dup";
+ close($rdfh);
+ close($wrfh);
+ exec {"/usr/bin/gzip"}
+ "gzip",
+ "-d",
+ "-c",
+ "-q",
+ "-"
+ or die "can't run gzip";
+ }
+ my $pid2 = fork();
- if (!defined $pid2) {
- die "Cannot fork: $!";
- }
- if ($pid2) {
- print $pid2, "\n";
- exec {"/usr/bin/gzip"}
- "gzip",
- "-d",
- "-c",
- "-q",
- "-"
- or die "can't run gzip";
- } else {
- if (defined $object->{cache_dir}) {
- my $pid3 = open(my $in, "-|");
- if (!defined $pid3) {
- die "Cannot fork: $!";
- }
- if ($pid3) {
- $self->pkg_copy($in, $object);
- exit(0);
- } else {
- $self->grab_object($object);
- }
+ if (!defined $pid2) {
+ die "Cannot fork: $!";
+ }
+ if ($pid2) {
+ $object->{pid2} = $pid2;
+ } else {
+ open STDERR, '>', $object->{errors};
+ open(STDOUT, '>&', $wrfh) or die "Bad dup";
+ close($rdfh);
+ close($wrfh);
+ close($fh);
+ if (defined $object->{cache_dir}) {
+ my $pid3 = open(my $in, "-|");
+ if (!defined $pid3) {
+ die "Cannot fork: $!";
+ }
+ if ($pid3) {
+ $self->pkg_copy($in, $object);
} else {
$self->grab_object($object);
}
+ } else {
+ $self->grab_object($object);
}
+ exit(0);
}
+ close($rdfh);
+ close($wrfh);
+ return $fh;
}
sub _list
diff --git a/usr.sbin/pkg_add/OpenBSD/PackageRepository/SCP.pm b/usr.sbin/pkg_add/OpenBSD/PackageRepository/SCP.pm
index febc41c95c4..4a76a3154f3 100644
--- a/usr.sbin/pkg_add/OpenBSD/PackageRepository/SCP.pm
+++ b/usr.sbin/pkg_add/OpenBSD/PackageRepository/SCP.pm
@@ -1,5 +1,5 @@
# ex:ts=8 sw=4:
-# $OpenBSD: SCP.pm,v 1.5 2006/03/07 14:18:51 espie Exp $
+# $OpenBSD: SCP.pm,v 1.6 2006/03/08 11:22:02 espie Exp $
#
# Copyright (c) 2003-2004 Marc Espie <espie@openbsd.org>
#
@@ -64,10 +64,6 @@ sub grab_object
my $cmdfh = $self->{cmdfh};
my $getfh = $self->{getfh};
- $SIG{'USR1'} = sub {
- kill USR1 => $object->{parent};
- exit(1);
- };
print $cmdfh "ABORT\n";
local $_;
@@ -103,22 +99,6 @@ sub grab_object
}
}
-sub pkg_copy
-{
- my ($self, $in, $object) = @_;
-
- $SIG{'USR1'} = sub {
- close($in);
- if (defined $object->{tempname}) {
- unlink $object->{tempname};
- }
- kill USR1 => $object->{parent};
- exit(1);
- };
-
- $self->SUPER::pkg_copy($in, $object);
-}
-
sub _new
{
my ($class, $baseurl) = @_;
@@ -173,32 +153,6 @@ sub list
return $self->{list};
}
-sub finish_and_close
-{
- my ($self, $object) = @_;
- $self->SUPER::close($object);
-}
-
-sub close
-{
- my ($self, $object, $hint) = @_;
- # XXX we have to make sure the grand-child is dead.
- if (defined $object->{pid2}) {
- my $received = 0;
- local $SIG{'USR1'} = sub { $received = 1; };
- kill USR1 => $object->{pid2};
- while (!$received) {
- sleep 0.01;
- }
- }
- close($object->{fh}) if defined $object->{fh};
-
- $self->parse_problems($object->{errors}, $hint)
- if defined $object->{errors};
- undef $object->{errors};
- $object->deref();
-}
-
# XXX not used yet
sub cleanup
{