From da2b36514a8b9e241908d5ac6ee1bf08407b125e Mon Sep 17 00:00:00 2001 From: ta264 Date: Sun, 14 Jul 2019 21:29:01 +0100 Subject: [PATCH] Fixed: Prevent two TypeExclusive commands running at once The check was bypassed if a disk access command was running at the same time. --- .../Messaging/Commands/CommandQueueFixture.cs | 19 ++++ .../Messaging/Commands/CommandQueue.cs | 94 +++++++++---------- 2 files changed, 63 insertions(+), 50 deletions(-) diff --git a/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueFixture.cs b/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueFixture.cs index 2e759bdeb..f27453095 100644 --- a/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueFixture.cs +++ b/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueFixture.cs @@ -88,6 +88,25 @@ namespace NzbDrone.Core.Test.Messaging.Commands command.Should().BeNull(); } + [Test] + public void should_not_return_type_exclusive_command_if_another_and_disk_access_command_running() + { + GivenStartedTypeExclusiveCommand(); + GivenStartedDiskCommand(); + + var newCommandModel = Builder + .CreateNew() + .With(c => c.Name = "ImportListSync") + .With(c => c.Body = new ImportListSyncCommand()) + .Build(); + + Subject.Add(newCommandModel); + + Subject.TryGet(out var command); + + command.Should().BeNull(); + } + [Test] public void should_return_type_exclusive_command_if_another_not_running() { diff --git a/src/NzbDrone.Core/Messaging/Commands/CommandQueue.cs b/src/NzbDrone.Core/Messaging/Commands/CommandQueue.cs index 3c4a6bc47..f5e0de695 100644 --- a/src/NzbDrone.Core/Messaging/Commands/CommandQueue.cs +++ b/src/NzbDrone.Core/Messaging/Commands/CommandQueue.cs @@ -134,59 +134,53 @@ namespace NzbDrone.Core.Messaging.Commands .Select(x => x.Body.Name) .ToList(); - var localItem = _items.Where(c => - { - // If an executing command requires disk access don't return a command that - // requires disk access. A lower priority or later queued task could be returned - // instead, but that will allow other tasks to execute whiule waiting for disk access. - if (startedCommands.Any(x => x.Body.RequiresDiskAccess)) - { - return c.Status == CommandStatus.Queued && - !c.Body.RequiresDiskAccess; - } - - // If an executing command is TypeExclusive don't return a command with same type - if (startedCommands.Any(x => x.Body.IsTypeExclusive)) { - return c.Status == CommandStatus.Queued && !exclusiveTypes.Any(x => x == c.Body.Name); - } - - return c.Status == CommandStatus.Queued; - }) - .OrderByDescending(c => c.Priority) - .ThenBy(c => c.QueuedAt) - .FirstOrDefault(); - - // Nothing queued that meets the requirements - if (localItem == null) - { - rval = false; - } - - // If any executing command is exclusive don't want return another command until it completes. - else if (startedCommands.Any(c => c.Body.IsExclusive)) - { - rval = false; - } - - // If the next command to execute is exclusive wait for executing commands to complete. - // This will prevent other tasks from starting so the exclusive task executes in the order it should. - else if (localItem.Body.IsExclusive && startedCommands.Any()) - { - rval = false; - } - - // A command ready to execute - else - { - localItem.StartedAt = DateTime.UtcNow; - localItem.Status = CommandStatus.Started; - - item = localItem; - } + var queuedCommands = _items.Where(c => c.Status == CommandStatus.Queued); + + if (startedCommands.Any(x => x.Body.RequiresDiskAccess)) + { + queuedCommands = queuedCommands.Where(c => !c.Body.RequiresDiskAccess); + } + + if (startedCommands.Any(x => x.Body.IsTypeExclusive)) + { + queuedCommands = queuedCommands.Where(c => !exclusiveTypes.Any(x => x == c.Body.Name)); + } + + var localItem = queuedCommands.OrderByDescending(c => c.Priority) + .ThenBy(c => c.QueuedAt) + .FirstOrDefault(); + + // Nothing queued that meets the requirements + if (localItem == null) + { + rval = false; + } + + // If any executing command is exclusive don't want return another command until it completes. + else if (startedCommands.Any(c => c.Body.IsExclusive)) + { + rval = false; + } + + // If the next command to execute is exclusive wait for executing commands to complete. + // This will prevent other tasks from starting so the exclusive task executes in the order it should. + else if (localItem.Body.IsExclusive && startedCommands.Any()) + { + rval = false; + } + + // A command ready to execute + else + { + localItem.StartedAt = DateTime.UtcNow; + localItem.Status = CommandStatus.Started; + + item = localItem; } } + } return rval; - } } + } }