Conversation
| public void TearDown() | ||
| { | ||
| this.threadPool?.Shutdown(); | ||
| } |
| } | ||
|
|
||
| [Test] | ||
| public void Submit_SinglTask_ReturnCorrectResult() |
There was a problem hiding this comment.
| public void Submit_SinglTask_ReturnCorrectResult() | |
| public void Submit_SingleTask_ReturnCorrectResult() |
| } | ||
|
|
||
| [Test] | ||
| public void Submit_MultiplyTask_ReturnCorrectResult() |
There was a problem hiding this comment.
| public void Submit_MultiplyTask_ReturnCorrectResult() | |
| public void Submit_MultipleTasks_ReturnsCorrectResult() |
| } | ||
|
|
||
| [Test] | ||
| public void ThreadPool_NumberOfThreads_ShouldUsetheSpecifiedAmount() |
There was a problem hiding this comment.
| public void ThreadPool_NumberOfThreads_ShouldUsetheSpecifiedAmount() | |
| public void ThreadPool_NumberOfThreads_ShouldUseTheSpecifiedAmount() |
|
|
||
| this.threadPool = null; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Тесты очень годные. Но надо бы ещё тесты на разные случаи потенциальных гонок в самом пуле потоков, типа Submit одновременно с Shutdown, ContinueWith с Shutdown и т.п.
| throw new InvalidOperationException("Пул остановлен"); | ||
| } | ||
|
|
||
| ArgumentNullException.ThrowIfNull(func, nameof(func)); |
There was a problem hiding this comment.
К этому моменту Shutdown может быть вызван из другого потока и даже полностью отработать, так что проверка выше довольно бессмысленна. Как и любая несинхронизированная проверка разделяемой между потоками переменной, она проверяет лишь значение на некоторый момент в прошлом.
Тем более что EnqueueAction всё равно делает эту проверку, уже синхронизированно.
| throw new InvalidOperationException("Пул остановлен"); | ||
| } | ||
|
|
||
| ArgumentNullException.ThrowIfNull(func, nameof(func)); |
There was a problem hiding this comment.
ThrowIfNull сам делает nameof(func), для этого в компилятор даже добавили поддержку какого-то атрибута.
| } | ||
|
|
||
| this.isShutdown = true; | ||
| this.cancellationTokenSource.Cancel(); |
There was a problem hiding this comment.
Shutdown никто не мешает вызвать из двух потоков одновременно, что может привести тут к странным последствиям. Вроде как ничего не сломается, но, например, проверка выше в этом плане бессмысленна.
| throw new InvalidOperationException("Пул остановлен"); | ||
| } | ||
|
|
||
| var nextTask = new MyTask<TNewResult>( |
There was a problem hiding this comment.
Пул про taskLock ничего не знает, поэтому вполне может быть, что тут pool.isShutdown уже true. Вообще, я бы доверил пулу потоков самому думать, остановлен он или нет, он-то может синхронизированно проверить своё состояние. Проверка выше может иметь смысл как оптимизация, чтобы вообще не ставить задачу в очередь и ничего не делать, если пул выключен, но тогда стоило её вынести из lock (потому что зачем брать замок, если мы ничего делать не будем).
| { | ||
| try | ||
| { | ||
| this.pool.EnqueueAction(execute); |
There was a problem hiding this comment.
Если текущая задача завершилась исключением, continuation-ы по идее не должны запускаться, результата исходной задачи ведь нет. Наверное, надо им выставить исключение из "родительской" задачи и объявить их сделанными, чтобы пул вообще не беспокоить.
2 participants
No description provided.