From 88b2c65bf15734148cde7369d43a263eb77117f1 Mon Sep 17 00:00:00 2001 From: Marc Espie Date: Sun, 14 Jul 2019 07:27:19 +0000 Subject: a bunch of changes, all related to error-handling: - have Handle->register also create a proper END block, so that individual packages don't have to, and explain the issue - kill old Unlink/Copy code that migrated to State years ago - commonalize try{} catch {} for pkg_add/delete and pkg_create, so that debug works the same way in both. - switch printing command name to the catch handler, so that exceptions are simpler to handle and a few comments for the hairy parts... --- usr.sbin/pkg_add/OpenBSD/AddCreateDelete.pm | 21 +++- usr.sbin/pkg_add/OpenBSD/AddDelete.pm | 66 +++++-------- usr.sbin/pkg_add/OpenBSD/Error.pm | 133 +++++++------------------- usr.sbin/pkg_add/OpenBSD/PackageRepository.pm | 14 +-- usr.sbin/pkg_add/OpenBSD/PkgCreate.pm | 58 +++++------ usr.sbin/pkg_add/OpenBSD/State.pm | 4 +- usr.sbin/pkg_add/OpenBSD/Temp.pm | 16 ++-- 7 files changed, 124 insertions(+), 188 deletions(-) (limited to 'usr.sbin') diff --git a/usr.sbin/pkg_add/OpenBSD/AddCreateDelete.pm b/usr.sbin/pkg_add/OpenBSD/AddCreateDelete.pm index cfb937e9c5c..17157bd53a8 100644 --- a/usr.sbin/pkg_add/OpenBSD/AddCreateDelete.pm +++ b/usr.sbin/pkg_add/OpenBSD/AddCreateDelete.pm @@ -1,5 +1,5 @@ # ex:ts=8 sw=4: -# $OpenBSD: AddCreateDelete.pm,v 1.44 2019/06/09 18:58:06 espie Exp $ +# $OpenBSD: AddCreateDelete.pm,v 1.45 2019/07/14 07:27:18 espie Exp $ # # Copyright (c) 2007-2014 Marc Espie # @@ -187,6 +187,25 @@ sub handle_options $state->handle_options($opt_string, $self, @usage); } +sub try_and_run_command +{ + my ($self, $state) = @_; + if ($state->defines('debug')) { + $self->run_command($state); + } else { + try { + $self->run_command($state); + } catch { + $state->errsay("#1: #2", $state->{cmd}, $_); + OpenBSD::Handler->reset; + if ($_ =~ m/^Caught SIG(\w+)/o) { + kill $1, $$; + } + $state->{bad}++; + }; + } +} + package OpenBSD::InteractiveStub; sub new { diff --git a/usr.sbin/pkg_add/OpenBSD/AddDelete.pm b/usr.sbin/pkg_add/OpenBSD/AddDelete.pm index e1a43e04809..e7609def601 100644 --- a/usr.sbin/pkg_add/OpenBSD/AddDelete.pm +++ b/usr.sbin/pkg_add/OpenBSD/AddDelete.pm @@ -1,5 +1,5 @@ # ex:ts=8 sw=4: -# $OpenBSD: AddDelete.pm,v 1.87 2019/07/11 07:14:23 espie Exp $ +# $OpenBSD: AddDelete.pm,v 1.88 2019/07/14 07:27:18 espie Exp $ # # Copyright (c) 2007-2010 Marc Espie # @@ -63,6 +63,7 @@ use OpenBSD::Error; use OpenBSD::Paths; use OpenBSD::PackageInfo; use OpenBSD::AddCreateDelete; +our @ISA = qw(OpenBSD::AddCreateDelete); sub do_the_main_work { @@ -102,49 +103,32 @@ sub handle_end_tags }); } -sub framework +sub run_command { my ($self, $state) = @_; - my $do = sub { - lock_db($state->{not}, $state) unless $state->defines('nolock'); - $state->check_root; - $self->process_parameters($state); - my $dielater = $self->do_the_main_work($state); - # cleanup various things - $self->handle_end_tags($state); - $state->{recorder}->cleanup($state); - $state->ldconfig->ensure; - OpenBSD::PackingElement->finish($state); - $state->progress->clear; - $state->log->dump; - $self->finish_display($state); - if ($state->verbose >= 2 || $state->{size_only} || - $state->defines('tally')) { - $state->vstat->tally; - } - $state->say("Extracted #1 from #2", - $state->{stats}{donesize}, - $state->{stats}{totsize}) - if defined $state->{stats} and $state->verbose; - # show any error, and show why we died... - rethrow $dielater; - }; - if ($state->defines('debug')) { - &$do; - } else { - try { - &$do; - } catch { - $state->errsay("#1", $_); - OpenBSD::Handler->reset; - if ($_ =~ m/^Caught SIG(\w+)/o) { - kill $1, $$; - } - $state->{bad}++; - }; + lock_db($state->{not}, $state) unless $state->defines('nolock'); + $state->check_root; + $self->process_parameters($state); + my $dielater = $self->do_the_main_work($state); + # cleanup various things + $self->handle_end_tags($state); + $state->{recorder}->cleanup($state); + $state->ldconfig->ensure; + OpenBSD::PackingElement->finish($state); + $state->progress->clear; + $state->log->dump; + $self->finish_display($state); + if ($state->verbose >= 2 || $state->{size_only} || + $state->defines('tally')) { + $state->vstat->tally; } - + $state->say("Extracted #1 from #2", + $state->{stats}{donesize}, + $state->{stats}{totsize}) + if defined $state->{stats} and $state->verbose; + # show any error, and show why we died... + rethrow $dielater; } sub parse_and_run @@ -174,7 +158,7 @@ sub parse_and_run } } - $self->framework($state); + $self->try_and_run_command($state); if (defined $lflag) { $termios->setlflag($lflag); diff --git a/usr.sbin/pkg_add/OpenBSD/Error.pm b/usr.sbin/pkg_add/OpenBSD/Error.pm index 1148a84c7f5..90451d1573c 100644 --- a/usr.sbin/pkg_add/OpenBSD/Error.pm +++ b/usr.sbin/pkg_add/OpenBSD/Error.pm @@ -1,5 +1,5 @@ # ex:ts=8 sw=4: -# $OpenBSD: Error.pm,v 1.33 2019/07/11 07:03:45 espie Exp $ +# $OpenBSD: Error.pm,v 1.34 2019/07/14 07:27:18 espie Exp $ # # Copyright (c) 2004-2010 Marc Espie # @@ -17,6 +17,8 @@ use strict; use warnings; +# this is a set of common classes related to error handling in pkg land + package OpenBSD::Auto; sub cache(*&) { @@ -30,32 +32,55 @@ sub cache(*&) *{$callpkg."::$sym"} = $actual; } +# a bunch of other modules create persistent state that must be cleaned up +# on exit (temporary files, network connections to abort properly...) +# END blocks would do that (but see below...) but sig handling bypasses that, +# so we MUST install SIG handlers. + +# note that END will be run for *each* process, so beware! +# (temp files are registered per pid, for instance, so they only +# get cleaned when the proper pid is used) package OpenBSD::Handler; -my $list = []; +# hash of code to run on ANY exit +my $cleanup = {}; + +sub cleanup +{ + my ($class, $sig) = @_; + # XXX note that order of cleanup is "unpredictable" + for my $v (values %$cleanup) { + &$v($sig); + } +} +END { + # XXX localize $? so that cleanup doesn't fuck up our exit code + local $?; + cleanup(); +} + +# register each code block "by name" so that we can re-register each +# block several times sub register { my ($class, $code) = @_; - push(@$list, $code); + $cleanup->{$code} = $code; } my $handler = sub { my $sig = shift; - for my $c (@$list) { - &$c($sig); - } + __PACKAGE__->cleanup($sig); + # after cleanup, just propagate the signal $SIG{$sig} = 'DEFAULT'; kill $sig, $$; }; sub reset { - $SIG{'INT'} = $handler; - $SIG{'QUIT'} = $handler; - $SIG{'HUP'} = $handler; - $SIG{'KILL'} = $handler; - $SIG{'TERM'} = $handler; + for my $sig (qw(INT QUIT HUP KILL TERM)) { + $SIG{$sig} = $handler; + } } __PACKAGE__->reset; @@ -63,95 +88,11 @@ __PACKAGE__->reset; package OpenBSD::Error; require Exporter; our @ISA=qw(Exporter); -our @EXPORT=qw(Copy Unlink try throw catch catchall rethrow); +our @EXPORT=qw(try throw catch catchall rethrow); our ($FileName, $Line, $FullMessage); -my @signal_name = (); - use Carp; - -sub fillup_names -{ - { - # XXX force autoload - package verylocal; - - require POSIX; - POSIX->import(qw(signal_h)); - } - - for my $sym (keys %POSIX::) { - next unless $sym =~ /^SIG([A-Z].*)/; - my $i = eval "&POSIX::$sym()"; - next unless defined $i; - $signal_name[$i] = $1; - } - # extra BSD signals - $signal_name[5] = 'TRAP'; - $signal_name[7] = 'IOT'; - $signal_name[10] = 'BUS'; - $signal_name[12] = 'SYS'; - $signal_name[16] = 'URG'; - $signal_name[23] = 'IO'; - $signal_name[24] = 'XCPU'; - $signal_name[25] = 'XFSZ'; - $signal_name[26] = 'VTALRM'; - $signal_name[27] = 'PROF'; - $signal_name[28] = 'WINCH'; - $signal_name[29] = 'INFO'; -} - -sub find_signal -{ - my $number = shift; - - if (@signal_name == 0) { - fillup_names(); - } - - return $signal_name[$number] || $number; -} - -sub child_error -{ - my $error = shift // $?; - - my $extra = ""; - - if ($error & 128) { - $extra = " (core dumped)"; - } - if ($error & 127) { - return "killed by signal ". find_signal($error & 127).$extra; - } else { - return "exit(". ($error >> 8) . ")$extra"; - } -} - -sub Copy -{ - require File::Copy; - - my $r = File::Copy::copy(@_); - if (!$r) { - print "copy(", join(',', @_),") failed: $!\n"; - } - return $r; -} - -sub Unlink -{ - my $verbose = shift; - my $r = unlink @_; - if ($r != @_) { - print "rm @_ failed: removed only $r targets, $!\n"; - } elsif ($verbose) { - print "rm @_\n"; - } - return $r; -} - sub dienow { my ($error, $handler) = @_; diff --git a/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm b/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm index e19d2eceaa9..25bd6b9a401 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.164 2019/07/10 09:34:38 espie Exp $ +# $OpenBSD: PackageRepository.pm,v 1.165 2019/07/14 07:27:18 espie Exp $ # # Copyright (c) 2003-2010 Marc Espie # @@ -82,18 +82,12 @@ sub unique return $o; } -my $cleanup = sub { +OpenBSD::Handler->register( + sub { for my $repo (values %$cache) { $repo->cleanup; } -}; -END { - my $a = $?; - &$cleanup; - $? = $a; -} - -OpenBSD::Handler->register($cleanup); + }); sub parse_fullurl { diff --git a/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm b/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm index 4f94b9e4b31..ecebae95f13 100644 --- a/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm +++ b/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm @@ -1,6 +1,6 @@ #! /usr/bin/perl # ex:ts=8 sw=4: -# $OpenBSD: PkgCreate.pm,v 1.160 2019/07/11 07:14:23 espie Exp $ +# $OpenBSD: PkgCreate.pm,v 1.161 2019/07/14 07:27:18 espie Exp $ # # Copyright (c) 2003-2014 Marc Espie # @@ -1587,27 +1587,9 @@ sub save_history return $l; } -sub parse_and_run +sub run_command { - my ($self, $cmd) = @_; - - my $regen_package = 0; - my $sign_only = 0; - my $rc = 0; - - my $state = OpenBSD::PkgCreate::State->new($cmd); - $state->handle_options; - - if (@ARGV == 0) { - $regen_package = 1; - } elsif (@ARGV != 1) { - if (defined $state->{contents} || - !defined $state->{signature_params}) { - $state->usage("Exactly one single package name is required: #1", join(' ', @ARGV)); - } - } - - try { + my ($self, $state) = @_; if (defined $state->opt('Q')) { $state->{opt}{q} = 1; } @@ -1617,7 +1599,7 @@ sub parse_and_run } my $plist; - if ($regen_package) { + if ($state->{regen_package}) { if (!defined $state->{contents} || @{$state->{contents}} > 1) { $state->usage("Exactly one single packing-list is required"); } @@ -1641,7 +1623,7 @@ sub parse_and_run $plist->stub_digest($ordered); } else { $state->set_status("checksumming"); - if ($regen_package) { + if ($state->{regen_package}) { $state->progress->visit_with_count($plist, 'verify_checksum'); } else { @@ -1684,7 +1666,7 @@ sub parse_and_run $state->{bad} = 0; my $wname; - if ($regen_package) { + if ($state->{regen_package}) { $wname = $plist->pkgname.".tgz"; } else { $plist->save or $state->fatal("can't write packing-list"); @@ -1701,11 +1683,29 @@ sub parse_and_run if (!$state->defines("stub")) { $self->finish_manpages($state, $plist); } - } catch { - $state->errsay("#1", $_); - $rc = 1; - }; - return $rc; +} + +sub parse_and_run +{ + my ($self, $cmd) = @_; + + my $sign_only = 0; + my $rc = 0; + + my $state = OpenBSD::PkgCreate::State->new($cmd); + $state->handle_options; + + if (@ARGV == 0) { + $state->{regen_package} = 1; + } elsif (@ARGV != 1) { + if (defined $state->{contents} || + !defined $state->{signature_params}) { + $state->usage("Exactly one single package name is required: #1", join(' ', @ARGV)); + } + } + + $self->try_and_run_command($state); + return $state->{bad} != 0; } 1; diff --git a/usr.sbin/pkg_add/OpenBSD/State.pm b/usr.sbin/pkg_add/OpenBSD/State.pm index 6f4172add23..1c3a752723d 100644 --- a/usr.sbin/pkg_add/OpenBSD/State.pm +++ b/usr.sbin/pkg_add/OpenBSD/State.pm @@ -1,5 +1,5 @@ # ex:ts=8 sw=4: -# $OpenBSD: State.pm,v 1.58 2019/07/11 07:14:23 espie Exp $ +# $OpenBSD: State.pm,v 1.59 2019/07/14 07:27:18 espie Exp $ # # Copyright (c) 2007-2014 Marc Espie # @@ -214,7 +214,7 @@ sub _fatal # the way is to eval { croak @_}; and decide what to do with $@. delete $SIG{__DIE__}; $self->sync_display; - croak $self->{cmd}, ": ", @_, "\n"; + croak @_, "\n"; } sub fatal diff --git a/usr.sbin/pkg_add/OpenBSD/Temp.pm b/usr.sbin/pkg_add/OpenBSD/Temp.pm index 7da540ebbd8..db695c1e158 100644 --- a/usr.sbin/pkg_add/OpenBSD/Temp.pm +++ b/usr.sbin/pkg_add/OpenBSD/Temp.pm @@ -1,5 +1,5 @@ # ex:ts=8 sw=4: -# $OpenBSD: Temp.pm,v 1.34 2019/07/10 09:34:09 espie Exp $ +# $OpenBSD: Temp.pm,v 1.35 2019/07/14 07:27:18 espie Exp $ # # Copyright (c) 2003-2005 Marc Espie # @@ -26,26 +26,24 @@ use OpenBSD::Error; our $tempbase = $ENV{'PKG_TMPDIR'} || OpenBSD::Paths->vartmp; +# stuff that should be cleaned up on exit, registered by pid, +# so that it gets cleaned on exit from the correct process + my $dirs = {}; my $files = {}; my ($lastname, $lasterror, $lasttype); -my $cleanup = sub { +OpenBSD::Handler->register( + sub { while (my ($name, $pid) = each %$files) { unlink($name) if $pid == $$; } while (my ($dir, $pid) = each %$dirs) { OpenBSD::Error->rmtree([$dir]) if $pid == $$; } -}; + }); -END { - my $r = $?; - &$cleanup; - $? = $r; -} -OpenBSD::Handler->register($cleanup); sub dir { -- cgit v1.2.3