diff --git a/Source/Scheduling/Operation.cs b/Source/Scheduling/Operation.cs index 9363474..9d6c183 100644 --- a/Source/Scheduling/Operation.cs +++ b/Source/Scheduling/Operation.cs @@ -39,24 +39,13 @@ namespace Nuclex.Support.Scheduling { /// public virtual void End() { - // Use some ingenious double-checked locking to set the endCalled flag. - // Quite a lot of effort for a mere safety feature that prevents the programmer - // from calling End() twice. - bool error; - if(!this.endCalled) { - lock(this) { - if(!this.endCalled) { - this.endCalled = true; - error = false; - } else { - error = true; - } - } // lock - } else { - error = true; + // By design, end can only be called once! + lock(this) { + if(this.endCalled) + throw new InvalidOperationException("End() has already been called"); + + this.endCalled = true; } - if(error) - throw new InvalidOperationException("End() has already been called"); // If the progression itself hasn't ended yet, block the caller until it has. if(!Ended) diff --git a/Source/Tracking/ProgressionTracker.Test.cs b/Source/Tracking/ProgressionTracker.Test.cs index 8060519..86da6c9 100644 --- a/Source/Tracking/ProgressionTracker.Test.cs +++ b/Source/Tracking/ProgressionTracker.Test.cs @@ -302,6 +302,25 @@ namespace Nuclex.Support.Tracking { this.mockery.VerifyAllExpectationsHaveBeenMet(); } + /// + /// Tries to provoke a deadlock by re-entering the tracker from one of + /// its own events. + /// + [Test] + public void TestProvokedDeadlock() { + ProgressionTracker tracker = new ProgressionTracker(); + + TestProgression test1 = new TestProgression(); + tracker.Track(test1); + + tracker.AsyncIdleStateChanged += + (EventHandler)delegate(object sender, IdleStateEventArgs arguments) { + tracker.Track(Progression.EndedDummy); + }; + + test1.End(); + } + /// Mocks a subscriber for the events of a tracker /// Tracker to mock an event subscriber for /// The mocked event subscriber diff --git a/Source/Tracking/ProgressionTracker.cs b/Source/Tracking/ProgressionTracker.cs index cb3f680..ec1b7f3 100644 --- a/Source/Tracking/ProgressionTracker.cs +++ b/Source/Tracking/ProgressionTracker.cs @@ -189,7 +189,7 @@ namespace Nuclex.Support.Tracking { observedProgression.Dispose(); } - } + } // if progression ended } // lock @@ -213,6 +213,7 @@ namespace Nuclex.Support.Tracking { { ObservedWeightedProgression wrappedProgression = this.trackedProgressions[removeIndex]; + this.trackedProgressions.RemoveAt(removeIndex); wrappedProgression.Dispose(); } @@ -295,7 +296,7 @@ namespace Nuclex.Support.Tracking { this.progress = totalProgress; OnAsyncProgressUpdated(totalProgress); - } + } // lock } /// Called when one of the tracked progressions has ended @@ -319,7 +320,7 @@ namespace Nuclex.Support.Tracking { // progressions were finished, so it's safe to trigger this here. setIdle(true); - } + } // lock } /// Called when one of the tracked progression has achieved progress @@ -338,6 +339,10 @@ namespace Nuclex.Support.Tracking { OnAsyncIdleStateChanged(idle); } + /// Whether the tracker is currently idle + private volatile bool idle; + /// Current summed progress of the tracked progressions + private volatile float progress; /// Total weight of all progressions being tracked private volatile float totalWeight; /// Progressions being tracked by this tracker @@ -346,10 +351,6 @@ namespace Nuclex.Support.Tracking { 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 volatile float progress; }