[Slim-Checkins] r33898 - in /7.8/trunk/server: Changelog7.html Slim/Control/Commands.pm Slim/Utils/OS.pm Slim/Utils/OS/OSX.pm Slim/Utils/OS/Unix.pm Slim/Utils/OS/Win32.pm Slim/Web/Settings/Server/Plugins.pm slimserver.pl slimservice.pl

[email protected] Tue, 20 Mar 2012 11:14:56 -0000
Newsgroups gmane.music.equipment.slimdevices.cvs
Message-ID <[email protected]>
Author: mherger
Date: Tue Mar 20 04:14:56 2012
New Revision: 33898

URL: http://svn.slimdevices.com/slim?rev=33898&view=rev
Log:
Bug: 17729
Description: apply patch to clean up restart behaviour - thanks quiet.dragon!

Modified:
    7.8/trunk/server/Changelog7.html
    7.8/trunk/server/Slim/Control/Commands.pm
    7.8/trunk/server/Slim/Utils/OS.pm
    7.8/trunk/server/Slim/Utils/OS/OSX.pm
    7.8/trunk/server/Slim/Utils/OS/Unix.pm
    7.8/trunk/server/Slim/Utils/OS/Win32.pm
    7.8/trunk/server/Slim/Web/Settings/Server/Plugins.pm
    7.8/trunk/server/slimserver.pl
    7.8/trunk/server/slimservice.pl

Modified: 7.8/trunk/server/Changelog7.html
URL: http://svn.slimdevices.com/slim/7.8/trunk/server/Changelog7.html?rev=33898&r1=33897&r2=33898&view=diff
==============================================================================
--- 7.8/trunk/server/Changelog7.html (original)
+++ 7.8/trunk/server/Changelog7.html Tue Mar 20 04:14:56 2012
@@ -1,3 +1,18 @@
+<h2><a name="v7.8.0" id="v7.8.0"></a>Version 7.8.0</h2>
+<ul>
+	<li>New Features:</li>
+	<ul>
+		<li>...</li>
+	</ul>
+	<br />
+
+	<li>Bug Fixes:</li>
+	<ul>
+		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=17729">#17729</a> - On Linux SBS 7.4.1 and above: service cannot be stopped or restarted after an 'internal' restart</li>
+	</ul>
+	<br />
+</ul>
+
 <h2><a name="v7.7.2" id="v7.7.2"></a>Version 7.7.2</h2>
 <ul>
 	<li>New Features:</li>

Modified: 7.8/trunk/server/Slim/Control/Commands.pm
URL: http://svn.slimdevices.com/slim/7.8/trunk/server/Slim/Control/Commands.pm?rev=33898&r1=33897&r2=33898&view=diff
==============================================================================
--- 7.8/trunk/server/Slim/Control/Commands.pm (original)
+++ 7.8/trunk/server/Slim/Control/Commands.pm Tue Mar 20 04:14:56 2012
@@ -2853,7 +2853,7 @@
 
 	# pass true value if we want to restart the server
 	if ($request->isCommand([['restartserver']])) {
-		Slim::Utils::OSDetect->getOS()->restartServer();
+		main::restartServer();
 	}
 	else {
 		main::stopServer();

Modified: 7.8/trunk/server/Slim/Utils/OS.pm
URL: http://svn.slimdevices.com/slim/7.8/trunk/server/Slim/Utils/OS.pm?rev=33898&r1=33897&r2=33898&view=diff
==============================================================================
--- 7.8/trunk/server/Slim/Utils/OS.pm (original)
+++ 7.8/trunk/server/Slim/Utils/OS.pm Tue Mar 20 04:14:56 2012
@@ -422,19 +422,20 @@
 sub directFirmwareDownload { 0 };
 
 
-=head2 restartServer( )
-
-The server can initiate a restart on some systems. 
-This should call main::cleanup() or stopServer() to cleanly shut down before restarting
-
-=cut
-
-sub restartServer {
-	my $class = shift;
-	main::stopServer(1) if $class->canRestartServer();
-}
-
-sub canRestartServer { 1 }
+=head2 restartServer( ) and canRestartServer
+
+The server can initiate a restart on some systems.
+These methods must only be called from main::restartServer and
+main::canRestartServer which supervise other cleanup operations.
+
+The implementations of the methods are redefined in each of the
+OS implementations that can support the restart feature.
+
+=cut
+
+sub restartServer { 0 }
+
+sub canRestartServer { 0 }
 
 sub progressJSON { }
 

Modified: 7.8/trunk/server/Slim/Utils/OS/OSX.pm
URL: http://svn.slimdevices.com/slim/7.8/trunk/server/Slim/Utils/OS/OSX.pm?rev=33898&r1=33897&r2=33898&view=diff
==============================================================================
--- 7.8/trunk/server/Slim/Utils/OS/OSX.pm (original)
+++ 7.8/trunk/server/Slim/Utils/OS/OSX.pm Tue Mar 20 04:14:56 2012
@@ -436,11 +436,19 @@
 sub installerExtension { 'dmg' }; 
 sub installerOS { 'osx' }
 
+sub canRestartServer { 1 }
+
 sub restartServer {
 	my $class  = shift;
 	my $helper = Slim::Utils::Misc::findbin('restart-server.sh');
 
-	system("'$helper' &") if $helper;
+	if ($helper) {
+		system("'$helper' &");
+		# XXX - the restart helper doesn't validate its success
+		return 1;
+	}
+
+	return 0;
 }
 
 

Modified: 7.8/trunk/server/Slim/Utils/OS/Unix.pm
URL: http://svn.slimdevices.com/slim/7.8/trunk/server/Slim/Utils/OS/Unix.pm?rev=33898&r1=33897&r2=33898&view=diff
==============================================================================
--- 7.8/trunk/server/Slim/Utils/OS/Unix.pm (original)
+++ 7.8/trunk/server/Slim/Utils/OS/Unix.pm Tue Mar 20 04:14:56 2012
@@ -128,4 +128,23 @@
 # leave log rotation to the system
 sub logRotate {}
 
-1;
+sub canRestartServer { 1 }
+
+sub restartServer {
+
+	my $class = shift;
+	my ($progFile, $progArgs)  = @_;
+
+	# Prefer to execute the script directly if possible, otherwise
+	# invoke the interpreter to start the script.
+	#
+	# The difference between the two approaches is visible on
+	# some systems in the process title. See the process name
+	# in /proc/$$/stat on Linux as an example.
+
+	my $execProg = (-x $progFile) ? $progFile : $^X;
+
+	return exec($execProg, $progFile, @$progArgs);
+}
+
+1;

Modified: 7.8/trunk/server/Slim/Utils/OS/Win32.pm
URL: http://svn.slimdevices.com/slim/7.8/trunk/server/Slim/Utils/OS/Win32.pm?rev=33898&r1=33897&r2=33898&view=diff
==============================================================================
--- 7.8/trunk/server/Slim/Utils/OS/Win32.pm (original)
+++ 7.8/trunk/server/Slim/Utils/OS/Win32.pm Tue Mar 20 04:14:56 2012
@@ -882,6 +882,9 @@
 		) {
 			$log->error("Couldn't restart Logitech Media Server service (squeezesvc)");
 		}
+		else {
+			return 1;
+		}
 	}
 	
 	elsif ($PerlSvc::VERSION) {
@@ -890,13 +893,16 @@
 		if (open(RESTART, ">$restartFlag")) {
 			close RESTART;
 			main::stopServer();
+			return 1;
 		}
 		
 		else {
 			$log->error("Can't write restart flag ($restartFlag) - don't shut down");
 		}
 	}
-};
+
+	return;
+}
 
 sub canRestartServer { return $PerlSvc::VERSION ? 1 : 0; }
 

Modified: 7.8/trunk/server/Slim/Web/Settings/Server/Plugins.pm
URL: http://svn.slimdevices.com/slim/7.8/trunk/server/Slim/Web/Settings/Server/Plugins.pm?rev=33898&r1=33897&r2=33898&view=diff
==============================================================================
--- 7.8/trunk/server/Slim/Web/Settings/Server/Plugins.pm (original)
+++ 7.8/trunk/server/Slim/Web/Settings/Server/Plugins.pm Tue Mar 20 04:14:56 2012
@@ -14,8 +14,6 @@
 use Slim::Utils::Prefs;
 use Slim::Utils::PluginManager;
 use Slim::Utils::OSDetect;
-
-my $os = Slim::Utils::OSDetect->getOS();
 
 sub name {
 	return Slim::Web::HTTP::CSRF->protectName('SETUP_PLUGINS');
@@ -84,7 +82,7 @@
 	my ($class, $paramRef, $noRestartMsg) = @_;
 	
 	# show a link/button to restart SC if this is supported by this platform
-	if ($os->canRestartServer()) {
+	if (main::canRestartServer()) {
 				
 		$paramRef->{'restartUrl'} = $paramRef->{webroot} . $paramRef->{path} . '?restart=1';
 		$paramRef->{'restartUrl'} .= '&rand=' . $paramRef->{'rand'} if $paramRef->{'rand'};
@@ -109,7 +107,7 @@
 sub restartServer {
 	my ($class, $paramRef, $needsRestart) = @_;
 	
-	if ($needsRestart && $paramRef->{restart} && $os->canRestartServer()) {
+	if ($needsRestart && $paramRef->{restart} && main::canRestartServer()) {
 		
 		$paramRef->{'warning'} = '<span id="popupWarning">'
 			. Slim::Utils::Strings::string('RESTARTING_PLEASE_WAIT')
@@ -130,7 +128,7 @@
 
 	} else {
 
-		$os->restartServer();
+		main::restartServer();
 	}
 }
 

Modified: 7.8/trunk/server/slimserver.pl
URL: http://svn.slimdevices.com/slim/7.8/trunk/server/slimserver.pl?rev=33898&r1=33897&r2=33898&view=diff
==============================================================================
--- 7.8/trunk/server/slimserver.pl (original)
+++ 7.8/trunk/server/slimserver.pl Tue Mar 20 04:14:56 2012
@@ -924,11 +924,13 @@
 	}
 
 	open STDOUT, '>>/dev/null';
-	
+
+	# Do not attempt to daemonize again.
+	@argv = grep { $_ ne '--daemon' } @argv;
+
 	# On Leopard, GD will crash because you can't run CoreServices code in a forked child,
 	# so we have to exec as well.
 	if ( $^O =~ /darwin/ ) {
-		@argv = grep { $_ ne '--daemon' } @argv;
 		exec $^X . ' "' . $0 . '" ' . join( ' ', @argv );
 		exit;
 	}
@@ -1083,22 +1085,39 @@
 #
 # Clean up resources and exit.
 #
+# All server code must direct stop and restart requests here and should
+# not attempt to manipulate or interrogate the OS specific code directly.
+#
+sub canRestartServer {
+	return !$::norestart && Slim::Utils::OSDetect->getOS()->canRestartServer();
+}
+
+sub restartServer {
+
+	if ( canRestartServer() ) {
+		cleanup();
+		logger('')->info( 'Logitech Media Server restarting...' );
+		
+		if ( !Slim::Utils::OSDetect->getOS()->restartServer($0, \@argv) ) {
+			logger('')->error("Unable to restart Logitech Media Server");
+		}
+	}
+
+	# XXX - shouldn't we ignore the restart command if we can't restart?
+	else {
+		logger('')->error("Unable to restart Logitech Media Server - shutting down.");
+		stopServer();
+	}
+
+	exit();
+}
+
 sub stopServer {
-	my $restart = shift;
-
-	logger('')->info( 'Logitech Media Server ' . ($restart && !$::norestart ? 'restarting...' : 'shutting down.') );
-	
-	$::stop = 1;
-	
+
 	cleanup();
-	
-	if ($restart && !$::norestart
-		&& Slim::Utils::OSDetect->getOS()->canRestartServer() 
-		&& !main::ISWINDOWS)
-	{
-		exec($^X, $0, @argv);
-	}
-
+
+	logger('')->info( 'Logitech Media Server shutting down.' );
+	
 	exit();
 }
 

Modified: 7.8/trunk/server/slimservice.pl
URL: http://svn.slimdevices.com/slim/7.8/trunk/server/slimservice.pl?rev=33898&r1=33897&r2=33898&view=diff
==============================================================================
--- 7.8/trunk/server/slimservice.pl (original)
+++ 7.8/trunk/server/slimservice.pl Tue Mar 20 04:14:56 2012
@@ -652,6 +652,10 @@
 #
 # Clean up resources and exit.
 #
+sub restartServer {
+	stopServer();
+}
+
 sub stopServer {
 
 	logger('')->info("Logitech Media Server shutting down.");