From e35a4bf8ac815c64ec0c51b9634f0ef802044c2b Mon Sep 17 00:00:00 2001 From: "kay.one" Date: Mon, 14 Nov 2011 18:38:15 -0800 Subject: [PATCH] Fixed an upgrade/service bug where it would try to stop an already stopped service. --- .../NzbDrone.Common.Test.csproj | 2 +- ...rollerTests.cs => ServiceProviderTests.cs} | 20 ++++++++++++++++-- NzbDrone.Common/ServiceProvider.cs | 21 ++++++++++++------- .../UpdateProviderStartFixture.cs | 16 +++++++++++++- NzbDrone.Update/Providers/UpdateProvider.cs | 11 +++++----- NzbDrone/Router.cs | 1 + 6 files changed, 54 insertions(+), 17 deletions(-) rename NzbDrone.Common.Test/{ServiceControllerTests.cs => ServiceProviderTests.cs} (82%) diff --git a/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj b/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj index 2b5a0dfda..ffdf61160 100644 --- a/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj +++ b/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj @@ -67,7 +67,7 @@ - + diff --git a/NzbDrone.Common.Test/ServiceControllerTests.cs b/NzbDrone.Common.Test/ServiceProviderTests.cs similarity index 82% rename from NzbDrone.Common.Test/ServiceControllerTests.cs rename to NzbDrone.Common.Test/ServiceProviderTests.cs index 9e3999bc0..2831403f4 100644 --- a/NzbDrone.Common.Test/ServiceControllerTests.cs +++ b/NzbDrone.Common.Test/ServiceProviderTests.cs @@ -8,10 +8,10 @@ using NzbDrone.Test.Common; namespace NzbDrone.Common.Test { [TestFixture] - public class ServiceControllerTests:TestBase + public class ServiceProviderTests : TestBase { private const string ALWAYS_INSTALLED_SERVICE = "SCardSvr"; //Smart Card - private const string TEMP_SERVICE_NAME = "NzbDrone_Nunit"; //Smart Card + private const string TEMP_SERVICE_NAME = "NzbDrone_Nunit"; private ServiceProvider serviceProvider; @@ -91,5 +91,21 @@ namespace NzbDrone.Common.Test serviceProvider.GetService(ALWAYS_INSTALLED_SERVICE).Status .Should().Be(ServiceControllerStatus.Stopped); } + + [Test] + public void Should_log_warn_if_on_stop_if_service_is_already_stopped() + { + serviceProvider.GetService(ALWAYS_INSTALLED_SERVICE).Status + .Should().NotBe(ServiceControllerStatus.Running); + + //Act + serviceProvider.Stop(ALWAYS_INSTALLED_SERVICE); + + //Assert + serviceProvider.GetService(ALWAYS_INSTALLED_SERVICE).Status + .Should().Be(ServiceControllerStatus.Stopped); + + ExceptionVerification.ExcpectedWarns(1); + } } } diff --git a/NzbDrone.Common/ServiceProvider.cs b/NzbDrone.Common/ServiceProvider.cs index 97ff3c507..a5a2138e6 100644 --- a/NzbDrone.Common/ServiceProvider.cs +++ b/NzbDrone.Common/ServiceProvider.cs @@ -100,17 +100,24 @@ namespace NzbDrone.Common Logger.Info("Service is currently {0}", service.Status); - service.Stop(); - service.WaitForStatus(ServiceControllerStatus.Stopped, TimeSpan.FromSeconds(60)); - - service.Refresh(); - if (service.Status == ServiceControllerStatus.Stopped) + if (service.Status != ServiceControllerStatus.Stopped) { - Logger.Info("{0} has stopped successfully."); + service.Stop(); + service.WaitForStatus(ServiceControllerStatus.Stopped, TimeSpan.FromSeconds(60)); + + service.Refresh(); + if (service.Status == ServiceControllerStatus.Stopped) + { + Logger.Info("{0} has stopped successfully."); + } + else + { + Logger.Error("Service stop request has timed out. {0}", service.Status); + } } else { - Logger.Error("Service stop request has timed out. {0}", service.Status); + Logger.Warn("Service {0} is already in stopped state.", service.ServiceName); } } diff --git a/NzbDrone.Update.Test/UpdateProviderStartFixture.cs b/NzbDrone.Update.Test/UpdateProviderStartFixture.cs index a6ea8e2eb..02853de30 100644 --- a/NzbDrone.Update.Test/UpdateProviderStartFixture.cs +++ b/NzbDrone.Update.Test/UpdateProviderStartFixture.cs @@ -51,9 +51,10 @@ namespace NzbDrone.Update.Test } [Test] - public void should_stop_nzbdrone_service_if_installed() + public void should_stop_nzbdrone_service_if_installed_and_running() { WithInstalledService(); + WithServiceRunning(true); //Act Mocker.Resolve().Start(TARGET_FOLDER); @@ -62,6 +63,19 @@ namespace NzbDrone.Update.Test Mocker.GetMock().Verify(c => c.Stop(ServiceProvider.NZBDRONE_SERVICE_NAME), Times.Once()); } + [Test] + public void should_not_stop_nzbdrone_service_if_installed_but_not_running() + { + WithInstalledService(); + WithServiceRunning(false); + + //Act + Mocker.Resolve().Start(TARGET_FOLDER); + + //Assert + Mocker.GetMock().Verify(c => c.Stop(ServiceProvider.NZBDRONE_SERVICE_NAME), Times.Never()); + } + [Test] public void should_not_stop_nzbdrone_service_if_service_isnt_installed() { diff --git a/NzbDrone.Update/Providers/UpdateProvider.cs b/NzbDrone.Update/Providers/UpdateProvider.cs index 004eab575..05bcc76d1 100644 --- a/NzbDrone.Update/Providers/UpdateProvider.cs +++ b/NzbDrone.Update/Providers/UpdateProvider.cs @@ -49,15 +49,14 @@ namespace NzbDrone.Update.Providers bool isService = false; logger.Info("Stopping all running services"); - if (_serviceProvider.ServiceExist(ServiceProvider.NZBDRONE_SERVICE_NAME)) + + if (_serviceProvider.ServiceExist(ServiceProvider.NZBDRONE_SERVICE_NAME) + && _serviceProvider.IsServiceRunning(ServiceProvider.NZBDRONE_SERVICE_NAME)) { - if (_serviceProvider.IsServiceRunning(ServiceProvider.NZBDRONE_SERVICE_NAME)) - { - isService = true; - } + isService = true; _serviceProvider.Stop(ServiceProvider.NZBDRONE_SERVICE_NAME); } - + logger.Info("Killing all running processes"); var processes = _processProvider.GetProcessByName(ProcessProvider.NzbDroneProccessName); foreach (var processInfo in processes) diff --git a/NzbDrone/Router.cs b/NzbDrone/Router.cs index 1b8902670..d264d945a 100644 --- a/NzbDrone/Router.cs +++ b/NzbDrone/Router.cs @@ -33,6 +33,7 @@ namespace NzbDrone { Logger.Info("Application mode: {0}", applicationMode); + //TODO:move this outside, it should be one of application modes (ApplicationMode.Service?) if (!_enviromentProvider.IsUserInteractive) { _serviceProvider.Run(_applicationServer);