Skip to content

Commit 26d5c20

Browse files
Martin LercherMartin Lercher
authored andcommitted
better disposal. better tests.
1 parent cb54877 commit 26d5c20

File tree

2 files changed

+28
-18
lines changed

2 files changed

+28
-18
lines changed

src/JsonRpc/ProcessScheduler.cs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ private List<Task> RemoveCompleteTasks(List<Task> list)
5656
return result;
5757
}
5858

59-
59+
public long _TestOnly_NonCompleteTaskCount = 0;
6060
private void ProcessRequestQueue()
6161
{
6262
// see https://github.com/OmniSharp/csharp-language-server-protocol/issues/4
@@ -82,6 +82,7 @@ private void ProcessRequestQueue()
8282
else
8383
throw new NotImplementedException("Only Serial and Parallel execution types can be handled currently");
8484
waitables = RemoveCompleteTasks(waitables);
85+
Interlocked.Exchange(ref _TestOnly_NonCompleteTaskCount, waitables.Count);
8586
}
8687
}
8788
}
@@ -91,18 +92,21 @@ private void ProcessRequestQueue()
9192
throw;
9293
// OperationCanceledException - The CancellationToken has been canceled.
9394
Task.WaitAll(waitables.ToArray(), TimeSpan.FromMilliseconds(1000));
94-
waitables.ForEach((t) =>
95+
var keeponrunning = RemoveCompleteTasks(waitables);
96+
Interlocked.Exchange(ref _TestOnly_NonCompleteTaskCount, keeponrunning.Count);
97+
keeponrunning.ForEach((t) =>
9598
{
96-
if (!t.IsCompleted) {
97-
// TODO: There is no way to abort a Task. As we don't construct the tasks, we can do nothing here
98-
// Option is: change the task factory "Func<Task> request" to a "Func<CancellationToken, Task> request"
99-
}
99+
// TODO: There is no way to abort a Task. As we don't construct the tasks, we can do nothing here
100+
// Option is: change the task factory "Func<Task> request" to a "Func<CancellationToken, Task> request"
100101
});
101102
}
102103
}
103104

105+
private bool _disposed = false;
104106
public void Dispose()
105107
{
108+
if (_disposed) return;
109+
_disposed = true;
106110
_cancel.Cancel();
107111
_thread.Join();
108112
_cancel.Dispose();

test/JsonRpc.Tests/ProcessSchedulerTests.cs

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public void ShouldScheduleCompletedTask(RequestProcessType type)
3333
done.Signal();
3434
return Task.CompletedTask;
3535
});
36-
done.Wait(ALONGTIME_MS).Should().Be(true);
36+
done.Wait(ALONGTIME_MS).Should().Be(true, because: "all tasks have to run");
3737
}
3838
}
3939

@@ -48,7 +48,7 @@ public void ShouldScheduleAwaitableTask(RequestProcessType type)
4848
await Task.Yield();
4949
done.Signal();
5050
});
51-
done.Wait(ALONGTIME_MS).Should().Be(true);
51+
done.Wait(ALONGTIME_MS).Should().Be(true, because: "all tasks have to run");
5252
}
5353
}
5454

@@ -64,7 +64,7 @@ public void ShouldScheduleConstructedTask(RequestProcessType type)
6464
done.Signal();
6565
});
6666
});
67-
done.Wait(ALONGTIME_MS).Should().Be(true);
67+
done.Wait(ALONGTIME_MS).Should().Be(true, because: "all tasks have to run");
6868
}
6969
}
7070

@@ -89,9 +89,11 @@ public void ShouldScheduleSerialInOrder()
8989
for (var i = 0; i < done.CurrentCount; i++)
9090
s.Add(RequestProcessType.Serial, HandlePeek);
9191

92-
done.Wait(ALONGTIME_MS).Should().Be(true);
93-
running.Should().Be(0);
94-
peek.Should().Be(1);
92+
done.Wait(ALONGTIME_MS).Should().Be(true, because: "all tasks have to run");
93+
running.Should().Be(0, because: "all tasks have to run normally");
94+
peek.Should().Be(1, because: "all tasks must not overlap");
95+
s.Dispose();
96+
Interlocked.Read(ref ((ProcessScheduler)s)._TestOnly_NonCompleteTaskCount).Should().Be(0, because: "the scheduler must not wait for tasks to complete after disposal");
9597
}
9698
}
9799

@@ -116,9 +118,11 @@ public void ShouldScheduleParallelInParallel()
116118
for (var i = 0; i<done.CurrentCount; i++)
117119
s.Add(RequestProcessType.Parallel, HandlePeek);
118120

119-
done.Wait(ALONGTIME_MS).Should().Be(true);
120-
running.Should().Be(0);
121-
peek.Should().BeGreaterThan(3);
121+
done.Wait(ALONGTIME_MS).Should().Be(true, because:"all tasks have to run");
122+
running.Should().Be(0, because:"all tasks have to run normally");
123+
peek.Should().BeGreaterThan(3, because:"a lot of tasks should overlap");
124+
s.Dispose();
125+
Interlocked.Read(ref ((ProcessScheduler)s)._TestOnly_NonCompleteTaskCount).Should().Be(0, because: "the scheduler must not wait for tasks to complete after disposal");
122126
}
123127
}
124128

@@ -149,9 +153,11 @@ public void ShouldScheduleMixed()
149153
s.Add(RequestProcessType.Parallel, HandlePeek);
150154
s.Add(RequestProcessType.Serial, HandlePeek);
151155

152-
done.Wait(ALONGTIME_MS).Should().Be(true);
153-
running.Should().Be(0);
154-
peek.Should().BeGreaterThan(2);
156+
done.Wait(ALONGTIME_MS).Should().Be(true, because: "all tasks have to run");
157+
running.Should().Be(0, because: "all tasks have to run normally");
158+
peek.Should().BeGreaterThan(2, because: "some tasks should overlap");
159+
s.Dispose();
160+
Interlocked.Read(ref ((ProcessScheduler)s)._TestOnly_NonCompleteTaskCount).Should().Be(0, because: "the scheduler must not wait for tasks to complete after disposal");
155161
}
156162
}
157163
}

0 commit comments

Comments
 (0)