diff --git a/Nuclex.Support (PC).csproj b/Nuclex.Support (PC).csproj index 9823ba5..ea87d51 100644 --- a/Nuclex.Support (PC).csproj +++ b/Nuclex.Support (PC).csproj @@ -174,10 +174,6 @@ false ThreadOperation - - false - WeightedRequirableOperation - false BinarySerializer.Test @@ -227,6 +223,11 @@ false ProgressionTracker + + false + ProgressionTracker.Test + ProgressionTracker.cs + false SetProgression diff --git a/Source/Scheduling/IAbortable.cs b/Source/Scheduling/IAbortable.cs index 5771ce1..f52a5ee 100644 --- a/Source/Scheduling/IAbortable.cs +++ b/Source/Scheduling/IAbortable.cs @@ -30,8 +30,8 @@ namespace Nuclex.Support.Scheduling { /// The receive should honor the abort request and stop whatever it is /// doing as soon as possible. The method does not impose any requirement /// on the timeliness of the reaction of the running process, but implementers - /// are advised to not ignore the abort request and try to design their code - /// in such a way that it can be stopped in a reasonable time + /// are advised to not ignore the abort request and urged to try and design + /// their code in such a way that it can be stopped in a reasonable time /// (eg. within 1 second of the abort request). /// void AsyncAbort(); diff --git a/Source/Scheduling/Operation.cs b/Source/Scheduling/Operation.cs index 4c0e66b..dc031ef 100644 --- a/Source/Scheduling/Operation.cs +++ b/Source/Scheduling/Operation.cs @@ -69,7 +69,8 @@ namespace Nuclex.Support.Scheduling { /// Exception that occured while the operation was executing /// /// If this field is null, it is assumed that no exception has occured - /// in the background process. When it is set, the End() method will + /// in the background process. If it is set, however, the End() method will + /// re-raise the exception to the calling thread when it is called. /// public Exception OccuredException { get { return this.occuredException; } @@ -82,8 +83,7 @@ namespace Nuclex.Support.Scheduling { // We allow the caller to set the exception multiple times. While I certainly // can't think of a scenario where this would happen, throwing an exception // in that case seems worse. The caller might just be executing an exception - // handling block and locking the operation instance could cause all even - // more problems. + // handling block and locking + throwing here could cause even more problems. this.occuredException = exception; } diff --git a/Source/Tracking/Internal/ObservedWeightedProgression.cs b/Source/Tracking/Internal/ObservedWeightedProgression.cs index 750df02..0b6dcd7 100644 --- a/Source/Tracking/Internal/ObservedWeightedProgression.cs +++ b/Source/Tracking/Internal/ObservedWeightedProgression.cs @@ -82,14 +82,13 @@ namespace Nuclex.Support.Tracking { asyncDisconnectEvents(); // We don't need those anymore! // If the progress hasn't reached 1.0 yet, make a fake report so that even - // when a progression doesn't report any progress at all, the set of queue + // when a progression doesn't report any progress at all, the set or queue // owning us will have a percentage of progressions completed. // // There is the possibility of a race condition here, as a final progress - // report could have been generated by some other thread that was preempted - // by this thread reporting the end of the progression. This is not a problem, - // however, since the progress is inteded only for informal purposes and - // not to be used for controlling program flow. + // report could have been generated by a thread running the progression + // that was preempted by this thread. This would cause the progress to + // jump to 1.0 and then back to whatever the waiting thread will report. if(this.progress != 1.0f) { this.progress = 1.0f; progressUpdateCallback(); diff --git a/Source/Tracking/ProgressionTracker.Test.cs b/Source/Tracking/ProgressionTracker.Test.cs new file mode 100644 index 0000000..4c3af65 --- /dev/null +++ b/Source/Tracking/ProgressionTracker.Test.cs @@ -0,0 +1,258 @@ +#region CPL License +/* +Nuclex Framework +Copyright (C) 2002-2007 Nuclex Development Labs + +This library is free software; you can redistribute it and/or +modify it under the terms of the IBM Common Public License as +published by the IBM Corporation; either version 1.0 of the +License, or (at your option) any later version. + +This library is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +IBM Common Public License for more details. + +You should have received a copy of the IBM Common Public +License along with this library +*/ +#endregion +using System; +using System.Collections.Generic; +using System.IO; + +#if UNITTEST + +using NUnit.Framework; +using NMock2; + +namespace Nuclex.Support.Tracking { + + /// Unit Test for the progression tracker class + [TestFixture] + public class ProgressionTrackerTest { + + #region interface IProgressionTrackerSubscriber + + /// Interface used to test the progression tracker. + public interface IProgressionTrackerSubscriber { + + /// Called when the progression tracker's progress changes + /// Progression tracker whose progress has changed + /// Contains the new progress achieved + void ProgressUpdated(object sender, ProgressUpdateEventArgs e); + + /// Called when the progression tracker's idle state changes + /// Progression tracker whose idle state has changed + /// Contains the new idle state of the tracker + void IdleStateChanged(object sender, IdleStateEventArgs e); + + } + + #endregion // interface IProgressionTrackerSubscriber + + #region class ProgressUpdateEventArgsMatcher + + /// Compares two ProgressUpdateEventArgsInstances for NMock validation + private class ProgressUpdateEventArgsMatcher : Matcher { + + /// Initializes a new ProgressUpdateEventArgsMatcher + /// Expected progress update event arguments + public ProgressUpdateEventArgsMatcher(ProgressUpdateEventArgs expected) { + this.expected = expected; + } + + /// + /// Called by NMock to verfiy the ProgressUpdateEventArgs match the expected value + /// + /// Actual value to compare to the expected value + /// + /// True if the actual value matches the expected value; otherwise false + /// + public override bool Matches(object actualAsObject) { + ProgressUpdateEventArgs actual = (actualAsObject as ProgressUpdateEventArgs); + if(actual == null) + return false; + + return (actual.Progress == this.expected.Progress); + } + + /// Creates a string representation of the expected value + /// Writer to write the string representation into + public override void DescribeTo(TextWriter writer) { + writer.Write(this.expected.Progress.ToString()); + } + + /// Expected progress update event args value + private ProgressUpdateEventArgs expected; + + } + + #endregion // class ProgressUpdateEventArgsMatcher + + #region class TestProgression + + /// Progression used for testing in this unit test + private class TestProgression : Progression { + + /// Changes the testing progression's indicated progress + /// + /// New progress to be reported by the testing progression + /// + public void ChangeProgress(float progress) { + OnAsyncProgressUpdated(progress); + } + + /// Transitions the progression into the ended state + public void End() { + OnAsyncEnded(); + } + + } + + #endregion // class TestProgression + + /// Initialization routine executed before each test is run + [SetUp] + public void Setup() { + this.mockery = new Mockery(); + } + + /// Validates that the tracker properly sums the progress + [Test] + public void TestSummedProgress() { + ProgressionTracker tracker = new ProgressionTracker(); + + IProgressionTrackerSubscriber mockedSubscriber = mockSubscriber(tracker); + + Expect.Once.On(mockedSubscriber). + Method("IdleStateChanged"). + WithAnyArguments(); + + Expect.Exactly(2).On(mockedSubscriber). + Method("ProgressUpdated"). + With( + new Matcher[] { + new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)), + new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(0.0f)) + } + ); + + TestProgression test1 = new TestProgression(); + tracker.Track(test1); + TestProgression test2 = new TestProgression(); + tracker.Track(test2); + + Expect.Once.On(mockedSubscriber). + Method("ProgressUpdated"). + With( + new Matcher[] { + new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)), + new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(0.25f)) + } + ); + + test1.ChangeProgress(0.5f); + + this.mockery.VerifyAllExpectationsHaveBeenMet(); + } + + /// + /// Validates that the tracker only removes progressions when the whole + /// tracking list has reached the 'ended' state. + /// + /// + /// If the tracker would remove ended progressions right when they finished, + /// the total progress would jump back each time. This is unwanted, of course. + /// + [Test] + public void TestDelayedRemoval() { + ProgressionTracker tracker = new ProgressionTracker(); + + IProgressionTrackerSubscriber mockedSubscriber = mockSubscriber(tracker); + + Expect.Once.On(mockedSubscriber). + Method("IdleStateChanged"). + WithAnyArguments(); + + Expect.Exactly(2).On(mockedSubscriber). + Method("ProgressUpdated"). + With( + new Matcher[] { + new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)), + new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(0.0f)) + } + ); + + TestProgression test1 = new TestProgression(); + tracker.Track(test1); + TestProgression test2 = new TestProgression(); + tracker.Track(test2); + + Expect.Once.On(mockedSubscriber). + Method("ProgressUpdated"). + With( + new Matcher[] { + new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)), + new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(0.25f)) + } + ); + + test1.ChangeProgress(0.5f); + + Expect.Once.On(mockedSubscriber). + Method("ProgressUpdated"). + With( + new Matcher[] { + new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)), + new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(0.75f)) + } + ); + + // Total progress should be 0.75 after this call (one progression at 1.0, + // the other one at 0.5). If the second progression would be removed, + // the progress would jump to 0.5 instead. + test2.End(); + + Expect.Once.On(mockedSubscriber). + Method("ProgressUpdated"). + With( + new Matcher[] { + new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)), + new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(1.0f)) + } + ); + + Expect.Once.On(mockedSubscriber). + Method("IdleStateChanged"). + WithAnyArguments(); + + test1.End(); + + this.mockery.VerifyAllExpectationsHaveBeenMet(); + } + + /// Mocks a subscriber for the events of a tracker + /// Tracker to mock an event subscriber for + /// The mocked event subscriber + private IProgressionTrackerSubscriber mockSubscriber(ProgressionTracker tracker) { + IProgressionTrackerSubscriber mockedSubscriber = + this.mockery.NewMock(); + + tracker.AsyncIdleStateChanged += + new EventHandler(mockedSubscriber.IdleStateChanged); + + tracker.AsyncProgressUpdated += + new EventHandler(mockedSubscriber.ProgressUpdated); + + return mockedSubscriber; + } + + /// Mock object factory + private Mockery mockery; + + } + +} // namespace Nuclex.Support.Tracking + +#endif // UNITTEST diff --git a/Source/Tracking/ProgressionTracker.cs b/Source/Tracking/ProgressionTracker.cs index 24eb3ed..25f2bdc 100644 --- a/Source/Tracking/ProgressionTracker.cs +++ b/Source/Tracking/ProgressionTracker.cs @@ -19,7 +19,7 @@ License along with this library #endregion using System; using System.Collections.Generic; -using System.Text; +using System.Threading; namespace Nuclex.Support.Tracking { @@ -39,6 +39,36 @@ namespace Nuclex.Support.Tracking { /// public class ProgressionTracker : IDisposable { + #region class ProgressionMatcher + + /// Matches a progression to a fully wrapped one + private class ProgressionMatcher { + + /// + /// Initializes a new progression matcher that matches against + /// the specified progression + /// + /// Progression to match against + public ProgressionMatcher(Progression toMatch) { + this.toMatch = toMatch; + } + + /// + /// Checks whether the provided progression matches the comparison + /// progression of the instance + /// + /// Progression to match to the comparison progression + public bool Matches(ObservedWeightedProgression other) { + return ReferenceEquals(other.WeightedProgression.Progression, this.toMatch); + } + + /// Progression this instance compares against + private Progression toMatch; + + } + + #endregion // class ProgressionMatcher + /// Triggered when the idle state of the tracker changes /// /// The tracker is idle when no progressions are being tracked in it. If you're @@ -51,9 +81,30 @@ namespace Nuclex.Support.Tracking { /// Triggered when the total progress has changed public event EventHandler AsyncProgressUpdated; + /// Initializes a new progression tracker + public ProgressionTracker() { + + this.trackedProgressions = new List>(); + this.idle = true; + + this.asyncEndedDelegate = + new ObservedWeightedProgression.ReportDelegate(asyncEnded); + this.asyncProgressUpdatedDelegate = + new ObservedWeightedProgression.ReportDelegate(asyncProgressUpdated); + + } + /// Immediately releases all resources owned by the instance public void Dispose() { - // TODO: Untrack all + lock(this.trackedProgressions) { + + for(int index = 0; index < this.trackedProgressions.Count; ++index) + this.trackedProgressions[index].Dispose(); + + this.trackedProgressions.Clear(); + this.trackedProgressions = null; + + } // lock } /// Begins tracking the specified progression @@ -66,21 +117,98 @@ namespace Nuclex.Support.Tracking { /// Progression to be tracked /// Weight to assign to this progression public void Track(Progression progression, float weight) { - this.trackedProgressions.Add( - new WeightedProgression(progression, weight) - ); - this.totalWeight += weight; + // Add the new progression into the tracking list. This has to be done + // inside a lock to prevent issues with the progressUpdate callback, which could + // access the totalWeight field before it has been updated to reflect the + // new progression added to the collection. + lock(this.trackedProgressions) { + + this.trackedProgressions.Add( + new ObservedWeightedProgression( + new WeightedProgression(progression, weight), + this.asyncProgressUpdatedDelegate, + this.asyncEndedDelegate + ) + ); + + // This can be done after we registered the wrapper to our delegates because + // any incoming progress updates will be stopped from the danger of a + // division-by-zero from the potentially still zeroed totalWeight by the lock. + this.totalWeight += weight; + + // If this is the first progression to be added to the list, tell our + // owner that we're idle no longer! + if(this.trackedProgressions.Count == 1) + OnAsyncIdleStateChanged(false); + + } // lock + + // All done, the total progress is different now, so force a recalculation and + // send out the AsyncProgressUpdated event. recalculateProgress(); + } /// Stops tracking the specified progression /// Progression to stop tracking of - public void Untrack(Progression progression) { } + public void Untrack(Progression progression) { + lock(this.trackedProgressions) { + + // Locate the object to be untracked in our collection + int removeIndex = this.trackedProgressions.FindIndex( + new Predicate>( + new ProgressionMatcher(progression).Matches + ) + ); + if(removeIndex == -1) + throw new InvalidOperationException("Item is not being tracked"); + + // Remove and dispose the progression the user wants to untrack + { + ObservedWeightedProgression wrappedProgression = + this.trackedProgressions[removeIndex]; + this.trackedProgressions.RemoveAt(removeIndex); + wrappedProgression.Dispose(); + } + + // If the list is empty, then we're back in the idle state + if(this.trackedProgressions.Count == 0) { + + this.totalWeight = 0.0f; + + // Report that we're idle now! + OnAsyncIdleStateChanged(true); + + } else { + + // Rebuild the total weight from scratch. Subtracting the removed progression's + // weight would work, too, but might accumulate rounding errors making the sum + // drift slowly away from the actual value. + this.totalWeight = 0.0f; + for(int index = 0; index < this.trackedProgressions.Count; ++index) + this.totalWeight += this.trackedProgressions[index].WeightedProgression.Weight; + + } + + } // lock + } + + /// Whether the tracker is currently idle + public bool Idle { + get { return this.idle; } + } + + /// Current summed progress of the tracked progressions + public float Progress { + get { return this.progress; } + } /// Fires the AsyncIdleStateChanged event /// New idle state to report protected virtual void OnAsyncIdleStateChanged(bool idle) { + this.idle = idle; + EventHandler copy = AsyncIdleStateChanged; if(copy != null) copy(this, new IdleStateEventArgs(idle)); @@ -89,6 +217,8 @@ namespace Nuclex.Support.Tracking { /// Fires the AsyncProgressUpdated event /// New progress to report protected virtual void OnAsyncProgressUpdated(float progress) { + this.progress = progress; + EventHandler copy = AsyncProgressUpdated; if(copy != null) copy(this, new ProgressUpdateEventArgs(progress)); @@ -96,22 +226,77 @@ namespace Nuclex.Support.Tracking { /// Recalculates the total progress of the tracker private void recalculateProgress() { - float totalProgress; + float totalProgress = 0.0f; - for(int index = 0; index < trackedProgressions.Count; ++index) { - float weight = this.trackedProgressions[index].WeightedProgression; - totalProgress = this.trackedProgressions[index].Progress * weight; + // Lock the progression to avoid trouble when someone tries to remove one + // of our tracked progressions while we're just processing a progress update + lock(this.trackedProgressions) { + + // This is a safety measure. In theory, even after all progressions have + // ended and collection of tracked progressions is cleared, a waiting + // thread might deliver another progress update causing this method to + // be entered. In this case, the right thing is to do nothing at all. + if(this.totalWeight == 0.0f) + return; + + // Sum up the total progress + for(int index = 0; index < this.trackedProgressions.Count; ++index) { + float weight = this.trackedProgressions[index].WeightedProgression.Weight; + totalProgress += this.trackedProgressions[index].Progress * weight; + } + + // This also needs to be in the lock to guarantee that the totalWeight is + // the one for the number of progressions we just summed -- by design, + // the total weight always has to be updated at the same time as the collection. + totalProgress /= this.totalWeight; } - totalProgress /= this.totalWeight; + // Finally, trigger the event + OnAsyncProgressUpdated(totalProgress); + } - //OnAsyncProgressUpdated( + /// Called when one of the tracked progressions has ended + private void asyncEnded() { + lock(this.trackedProgressions) { + + // If any progressions in the list are still going, keep the entire list. + // This behavior is intentional in order to prevent the tracker's progress from + // jumping back repeatedly when multiple tracked progressions come to an end. + for(int index = 0; index < this.trackedProgressions.Count; ++index) + if(!this.trackedProgressions[index].WeightedProgression.Progression.Ended) + return; + + // All progressions have finished, get rid of the wrappers and disconnect + // their events. + for(int index = 0; index < this.trackedProgressions.Count; ++index) + this.trackedProgressions[index].Dispose(); + + this.trackedProgressions.Clear(); + this.totalWeight = 0.0f; + + // Notify our owner that we're idle now. + OnAsyncIdleStateChanged(true); + + } + } + + /// Called when one of the tracked progression has achieved progress + private void asyncProgressUpdated() { + recalculateProgress(); } /// Total weight of all progressions being tracked - private float totalWeight; + private volatile float totalWeight; /// Progressions being tracked by this tracker - private List> trackedProgressions; + private List> trackedProgressions; + /// Delegate for the asyncEnded() method + private ObservedWeightedProgression.ReportDelegate asyncEndedDelegate; + /// Delegate for the asyncProgressUpdated() method + private ObservedWeightedProgression.ReportDelegate asyncProgressUpdatedDelegate; + /// Whether the tracker is currently idle + private bool idle; + /// Current summed progress of the tracked progressions + private float progress; }