Conversation
| // The work of 4 threads will take approximately 1 second. | ||
| Assert.That(stopwatch.Elapsed.TotalSeconds, Is.LessThan(1.5)); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Надо бы ещё тесты на разные другие случаи потенциальных гонок в пуле потоков, типа Submit одновременно с Shutdown, ContinueWith с Shutdown и т.п. И неплохо бы ещё тест на количество потоков в пуле.
| /// <typeparam name="TResult">The type of the result produced by the task.</typeparam> | ||
| internal class MyTask<TResult> : IMyTask<TResult> | ||
| { | ||
| private readonly object @lock = new(); |
There was a problem hiding this comment.
В современном .NET лучше использовать не object, а Lock. Про него знает компилятор и генерит более эффективный код.
Ну и называть поле "lock" кажется сомнительной идеей, '@' выглядит страшно.
| private TResult result = default!; | ||
| private Exception exception = null!; |
There was a problem hiding this comment.
Если они по смыслу могут быть null хоть в какое-то время своей жизни, сделайте их nullable. Я вообще против оператора ! в продакшн-коде (в тестах ок) — C# только-только встал на путь исправления "ошибки на миллиард долларов", сделав костыль в виде nullability-анализа, и мы такие "да ну нафиг, я тут ! напишу, и будет null там, где с точки зрения типизации его быть не может".
| { | ||
| if (this.continuations == null) | ||
| { | ||
| throw new InvalidOperationException("Cannot add continuation to a completed task with cleared continuations."); |
There was a problem hiding this comment.
Она completed, но у неё isCompleted false, что?
| this.waitHandle.Set(); | ||
| if (this.continuations != null) | ||
| { | ||
| foreach (var continuation in this.continuations) |
There was a problem hiding this comment.
Если текущая задача завершилась исключением, continuation-ы по идее не должны запускаться, результата исходной задачи ведь нет. Наверное, надо им выставить исключение из "родительской" задачи и объявить их сделанными.
| { | ||
| foreach (var continuation in this.continuations) | ||
| { | ||
| this.pool.QueueTask(continuation); |
There was a problem hiding this comment.
Если пул остановлен, то QueueTask бросит исключение, сюда (т.е. в поток из пула), так что "хозяева" задач-продолжений про него ничего не узнают и задедлочатся, вызвав Result.
| if (threadCount <= 0) | ||
| { | ||
| throw new ArgumentOutOfRangeException(nameof(threadCount), "Thread count must be positive."); | ||
| } |
There was a problem hiding this comment.
Используйте ArgumentOutOfRangeException.ThrowIfLessOrEqual.
| throw new InvalidOperationException("Cannot submit tasks to a shutdown thread pool."); | ||
| } | ||
|
|
||
| var task = new MyTask<TResult>(this, func); |
There was a problem hiding this comment.
Как раз в этот момент this.isShutdown может стать true, так что в проверке выше вроде как нет особого смысла. В QueueTask его всё равно ещё раз проверят, так что ничего не сломается, но несинхронизированная проверка разделяемой между потоками переменной всегда вызывает ощущение лёгкого удивления.
2 participants
No description provided.