* Switched from explicit cleanup functions to "resource acquisition by definition",

  implemented by the new ScopedResource class. This change improves robustness
  with respect to signals and unexpected errors and makes the code cleaner.

git-svn-id: http://svn.openslx.org/svn/openslx/openslx/trunk@1678 95ad53e4-c205-0410-b2fa-d234c58c8868

5 files changed, 171 insertions, 121 deletions

diff --git a/installer/OpenSLX/OSSetup/Distro/Base.pm b/installer/OpenSLX/OSSetup/Distro/Base.pm
index 39572e5d..573114e8 100644
--- a/installer/OpenSLX/OSSetup/Distro/Base.pm
+++ b/installer/OpenSLX/OSSetup/Distro/Base.pm
@@ -122,11 +122,6 @@ sub startSession
     my $self  = shift;
     my $osDir = shift;
     
-    # ensure that the session will be finished even if the script crashes:
-    addCleanupFunction(
-        "slxos-setup::distro::chroot", sub { $self->finishSession(); }
-    );
-
     # setup a fixed locale environment to avoid warnings about unset locales
     # (like for instance shown by apt-get)
     $ENV{LC_ALL} = 'POSIX';
@@ -152,7 +147,7 @@ sub startSession
 
     # mount /proc (if we have 'mount' available)
     if (qx{which mount 2>/dev/null}) {
-        if (!-e "$osDir/proc" && !mkpath("$osDir/proc")) {
+        if (!-e '/proc' && !mkpath('/proc')) {
             die _tr("unable to create folder '%s' (%s)\n", "$osDir/proc", $!);
         }
         if (slxsystem("mount -t proc proc '/proc'")) {
@@ -160,15 +155,13 @@ sub startSession
         }
     }
 
-    return;
+    return 1;
 }
 
 sub finishSession
 {
     my $self = shift;
     
-    removeCleanupFunction('slxos-setup::distro::chroot');
-
     # umount /proc (if we have 'umount' available)
     if (qx{which umount 2>/dev/null}) {
         if (slxsystem("umount /proc")) {
@@ -176,7 +169,7 @@ sub finishSession
         }
     }
 
-    return;
+    return 1;
 }
 
 sub getDefaultPathList
diff --git a/installer/OpenSLX/OSSetup/Engine.pm b/installer/OpenSLX/OSSetup/Engine.pm
index 9518c74a..7fce8a59 100644
--- a/installer/OpenSLX/OSSetup/Engine.pm
+++ b/installer/OpenSLX/OSSetup/Engine.pm
@@ -31,6 +31,7 @@ use File::Basename;
 use URI;
 
 use OpenSLX::Basics;
+use OpenSLX::ScopedResource;
 use OpenSLX::Syscall;
 use OpenSLX::Utils;
 
@@ -117,8 +118,7 @@ use vars qw(%supportedDistros);
     },
 );
 
-our $localHttpServerMasterPID;
-our %localHttpServers;
+my %localHttpServers;
 
 ################################################################################
 ### interface methods
@@ -132,22 +132,6 @@ sub new
     return bless $self, $class;
 }
 
-sub DESTROY
-{
-    my $self = shift;
-
-    my $httpServerPID = $localHttpServerMasterPID || '0';
-    if ($httpServerPID == $$) {
-        # we are the master process, so we clean up all the servers that we
-        # have started:
-        while (my ($localURL, $serverInfo) = each %localHttpServers) {
-            vlog(1, _tr("stopping local HTTP-server for URL '%s'.", $localURL));
-            kill TERM => $serverInfo->{pid};
-        }
-    }
-    return;
-}
-
 sub initialize
 {
     my $self         = shift;
@@ -291,7 +275,11 @@ sub installVendorOS
     }
     $self->_createVendorOSPath();
 
-    $self->_startLocalURLServersAsNeeded();
+    my $httpServers = OpenSLX::ScopedResource->new({
+        name    => 'local-http-servers',
+        acquire => sub { $self->_startLocalURLServersAsNeeded(); 1 },
+        release => sub { $self->_stopLocalURLServers(); 1 },
+    });
 
     my $baseSystemFile = "$self->{'vendor-os-path'}/.openslx-base-system";
     if (-e $baseSystemFile) {
@@ -436,7 +424,11 @@ sub updateVendorOS
             $self->{'vendor-os-path'});
     }
 
-    $self->_startLocalURLServersAsNeeded();
+    my $httpServers = OpenSLX::ScopedResource->new({
+        name    => 'local-http-servers',
+        acquire => sub { $self->_startLocalURLServersAsNeeded(); 1 },
+        release => sub { $self->_stopLocalURLServers(); 1 },
+    });
 
     callInSubprocess(
         sub {
@@ -467,7 +459,11 @@ sub startChrootedShellForVendorOS
         );
     }
 
-    $self->_startLocalURLServersAsNeeded();
+    my $httpServers = OpenSLX::ScopedResource->new({
+        name    => 'local-http-servers',
+        acquire => sub { $self->_startLocalURLServersAsNeeded(); 1 },
+        release => sub { $self->_stopLocalURLServers(); 1 },
+    });
 
     callInSubprocess(
         sub {
@@ -500,7 +496,11 @@ sub callChrootedFunctionForVendorOS
         );
     }
 
-    $self->_startLocalURLServersAsNeeded();
+    my $httpServers = OpenSLX::ScopedResource->new({
+        name    => 'local-http-servers',
+        acquire => sub { $self->_startLocalURLServersAsNeeded(); 1 },
+        release => sub { $self->_stopLocalURLServers(); 1 },
+    });
 
     callInSubprocess(
         sub {
@@ -1189,8 +1189,6 @@ sub _startLocalURLServersAsNeeded
 {
     my $self = shift;
 
-    $localHttpServerMasterPID ||= $$;
-
     my $port = 5080;
     my %portForURL;
     foreach my $repoInfo (values %{$self->{'distro-info'}->{repository}}) {
@@ -1220,6 +1218,16 @@ sub _startLocalURLServersAsNeeded
     return;
 }
 
+sub _stopLocalURLServers
+{
+    my $self = shift;
+
+    while (my ($localURL, $serverInfo) = each %localHttpServers) {
+        vlog(1, _tr("stopping local HTTP-server for URL '%s'.", $localURL));
+        kill TERM => $serverInfo->{pid};
+    }
+}
+
 sub _setupStage1A
 {
     my $self = shift;
@@ -1582,15 +1590,20 @@ sub _callChrootedFunction
         'updateConfig' => '?',
     });
 
-    $self->{distro}->startSession($params->{chrootDir});
+    my $distro = $self->{distro};
+    my $distroSession = OpenSLX::ScopedResource->new({
+        name    => 'ossetup::distro::session',
+        acquire => sub { $distro->startSession($params->{chrootDir}); 1 },
+        release => sub { $distro->finishSession(); 1 },
+    });
 
-    # invoke given function:
-    $params->{function}->();
+    die $@ if ! eval {
+        # invoke given function:
+        $params->{function}->();
+        $distro->updateDistroConfig() if $params->{updateConfig};
+        1;
+    };
 
-    if ($params->{updateConfig}) {
-        $self->{'distro'}->updateDistroConfig();
-    }
-    $self->{'distro'}->finishSession();
     return;
 }
 
diff --git a/lib/OpenSLX/Basics.pm b/lib/OpenSLX/Basics.pm
index 230bf72b..9af056e6 100644
--- a/lib/OpenSLX/Basics.pm
+++ b/lib/OpenSLX/Basics.pm
@@ -30,12 +30,10 @@ $VERSION = 1.01;
     &vlog
     &checkParams
     &instantiateClass
-    &addCleanupFunction &removeCleanupFunction
 );
 
 our (%openslxConfig, %cmdlineConfig, %openslxPath);
 
-use sigtrap qw( die normal-signals error-signals );
 use subs qw(die warn);
 
 use open ':utf8';
@@ -139,8 +137,6 @@ my %openslxCmdlineArgs = (
     'verbose-level=i' => \$cmdlineConfig{'verbose-level'},
 );
 
-my %cleanupFunctions;
-
 # filehandle used for logging:
 my $openslxLog = *STDERR;
 
@@ -313,27 +309,20 @@ sub callInSubprocess
 
     my $pid = fork();
     if (!$pid) {
-
-        # child...
-        # ...execute the given function and exit:
-        my $ok = eval { $childFunc->(); 1 };
-        if (!$ok) {
-            print STDERR "*** $@";
-            exit 5;
-        }
+        # child -> execute the given function and exit:
+        eval { $childFunc->(); 1 }
+            or die $@;
         exit 0;
     }
 
-    # parent...
-    # ...pass on interrupt- and terminate-signals to child...
-    local $SIG{INT}  = sub { kill 'INT',  $pid; waitpid($pid, 0); exit $? };
-    local $SIG{TERM} = sub { kill 'TERM', $pid; waitpid($pid, 0); exit $? };
+    # parent -> pass on interrupt- and terminate-signals to child ...
+    $SIG{INT}  = sub { kill 'INT',  $pid; };
+    $SIG{TERM} = sub { kill 'TERM', $pid; };
 
-    # ...and wait for child to do its work:
+    # ... and wait until child has done its work
     waitpid($pid, 0);
-    if ($?) {
-        exit $?;
-    }
+    exit $? if $?;
+
     return;
 }
 
@@ -355,36 +344,6 @@ sub executeInSubprocess
 }
 
 # ------------------------------------------------------------------------------
-sub addCleanupFunction
-{
-    my $name = shift;
-    my $func = shift;
-
-    $cleanupFunctions{$name} = $func;
-    return;
-}
-
-# ------------------------------------------------------------------------------
-sub removeCleanupFunction
-{
-    my $name = shift;
-
-    delete $cleanupFunctions{$name};
-    return;
-}
-
-# ------------------------------------------------------------------------------
-sub invokeCleanupFunctions
-{
-    my @funcNames = keys %cleanupFunctions;
-    foreach my $name (@funcNames) {
-        vlog(2, "invoking cleanup function '$name'...");
-        $cleanupFunctions{$name}->();
-    }
-    return;
-}
-
-# ------------------------------------------------------------------------------
 sub slxsystem
 {
     vlog(2, _tr("executing: %s", join ' ', @_));
@@ -426,7 +385,6 @@ sub warn
 # ------------------------------------------------------------------------------
 sub confess
 {
-    invokeCleanupFunctions();
     _doThrowOrWarn('confess', @_);
     return;
 }
@@ -434,7 +392,6 @@ sub confess
 # ------------------------------------------------------------------------------
 sub croak
 {
-    invokeCleanupFunctions();
     _doThrowOrWarn('croak', @_);
     return;
 }
@@ -442,7 +399,6 @@ sub croak
 # ------------------------------------------------------------------------------
 sub die
 {
-    invokeCleanupFunctions();
     _doThrowOrWarn('die', @_);
     return;
 }
@@ -636,10 +592,4 @@ sub instantiateClass
     return $class->new;
 }
 
-# ------------------------------------------------------------------------------
-END 
-{
-    invokeCleanupFunctions() if %cleanupFunctions;
-}
-
 1;
diff --git a/lib/OpenSLX/ScopedResource.pm b/lib/OpenSLX/ScopedResource.pm
new file mode 100644
index 00000000..c905b50a
--- /dev/null
+++ b/lib/OpenSLX/ScopedResource.pm
@@ -0,0 +1,91 @@
+# Copyright (c) 2008 - OpenSLX GmbH
+#
+# This program is free software distributed under the GPL version 2.
+# See http://openslx.org/COPYING
+#
+# If you have any feedback please consult http://openslx.org/feedback and
+# send your suggestions, praise, or complaints to feedback@openslx.org
+#
+# General information about OpenSLX can be found at http://openslx.org/
+# -----------------------------------------------------------------------------
+# ScopedResource.pm
+#    - a helper class that releases resources if the object leaves scope
+# -----------------------------------------------------------------------------
+package OpenSLX::ScopedResource;
+
+use strict;
+use warnings;
+
+our $VERSION = 1.01;        # API-version . implementation-version
+
+# make sure that we catch any signals in order to properly released scoped
+# resources
+use sigtrap qw( die normal-signals error-signals );
+
+use OpenSLX::Basics;
+
+sub new
+{
+    my $class = shift;
+    my $params = shift;
+    
+    checkParams($params, {
+        name    => '!',
+        acquire => '!',
+        release => '!',
+    });
+
+    my $self = {
+        name    => $params->{name},
+        owner   => 0,
+        acquire => $params->{acquire},
+        release => $params->{release},
+    };
+    
+    bless $self, $class;
+    
+    $self->acquire();
+    
+    return $self;
+}
+
+sub acquire
+{
+    my $self = shift;
+
+    # acquire the resource and set ourselves as owner
+    if ($self->{acquire}->()) {
+        vlog(1, "process $$ acquired resource $self->{name}");
+        $self->{owner} = $$;
+    }
+}
+
+sub release
+{
+    my $self = shift;
+
+    # only release the resource if invoked by the owning process
+    vlog(3, "process $$ tries to release resource $self->{name}");
+    return if $self->{owner} != $$;
+    
+    # release the resource and unset owner
+    if ($self->{release}->()) {
+        vlog(1, "process $$ released resource $self->{name}");
+        $self->{owner} = 0;
+    }
+}
+
+sub DESTROY
+{
+    my $self = shift;
+    
+    $self->release();
+    
+    # remove references to functions, in order to release any closures
+    $self->{acquire} = undef;
+    $self->{release} = undef;
+    
+    return;
+}
+
+1;
diff --git a/os-plugins/OpenSLX/OSPlugin/Engine.pm b/os-plugins/OpenSLX/OSPlugin/Engine.pm
index 61b835a4..3d967166 100644
--- a/os-plugins/OpenSLX/OSPlugin/Engine.pm
+++ b/os-plugins/OpenSLX/OSPlugin/Engine.pm
@@ -1,4 +1,4 @@
-# Copyright (c) 2007 - OpenSLX GmbH
+# Copyright (c) 2007, 2008 - OpenSLX GmbH
 #
 # This program is free software distributed under the GPL version 2.
 # See http://openslx.org/COPYING
@@ -24,6 +24,7 @@ use Storable;
 
 use OpenSLX::Basics;
 use OpenSLX::OSSetup::Engine;
+use OpenSLX::ScopedResource;
 use OpenSLX::Utils;
 
 =head1 NAME
@@ -516,28 +517,30 @@ sub _callChrootedFunctionForPlugin
     # bind-mount openslx basepath to /mnt/openslx of vendor-OS:
     my $basePath             = $openslxConfig{'base-path'};
     my $openslxPathInChroot = "$self->{'vendor-os-path'}/mnt/openslx";
-    mkpath( [ $openslxPathInChroot ] );
-    if (slxsystem("mount -o bind $basePath $openslxPathInChroot")) {
-        croak(
-            _tr(
-                "unable to bind mount '%s' to '%s'! (%s)", 
-                $basePath, $openslxPathInChroot, $!
-            )
-        );
-    }
+    mkpath($openslxPathInChroot);
+
+    my $pluginSession = OpenSLX::ScopedResource->new({
+        name    => 'osplugin::session',
+        acquire => sub { 
+            # bind mount openslx base path into vendor-OS
+            slxsystem("mount -o bind -o ro $basePath $openslxPathInChroot") == 0
+                or die _tr(
+                    "unable to bind mount '%s' to '%s'! (%s)", 
+                    $basePath, $openslxPathInChroot, $!
+                );
+            1 
+        },
+        release => sub {
+            slxsystem("umount $openslxPathInChroot") == 0
+                or die _tr(
+                    "unable to umount '%s'! (%s)", $openslxPathInChroot, $!
+                );
+            1
+        },
+    });
 
     # now let plugin install itself into vendor-OS
-    my $ok = eval {
-        $self->{'ossetup-engine'}->callChrootedFunctionForVendorOS($function);
-    };
-
-    if (slxsystem("umount $openslxPathInChroot")) {
-        croak(_tr("unable to umount '%s'! (%s)", $openslxPathInChroot, $!));
-    }
-    
-    if (!$ok) {
-        die $@;
-    }
+    $self->{'ossetup-engine'}->callChrootedFunctionForVendorOS($function);
     
     return;
 }