-
Notifications
You must be signed in to change notification settings - Fork 0
ThreadPool #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
ThreadPool #3
Conversation
|
|
||
| private static IEnumerable<TestCaseData> TestRemoveCaseData() => new TestCaseData[] | ||
| { | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Хм.
| yield return new TestCaseData(value.Result, counter, pool); | ||
| counter++; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это странный способ записать просто тест. Это ж не данные для тестируемой системы, а содержательная логика
| int a = task.Result; | ||
| Assert.That(pool.CountOfThreads, Is.EqualTo(numberOfThreads)); | ||
| pool.ShutDown(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если pool.CountOfThreads просто возвращает то, что передали в конструктор, потоков может быть сколько угодно. Было бы интереснее запустить N > numberOfThreads потоков и посмотреть, что за раз отработало ровно numberOfThreads из них.
| var pool = new MyThreadPool.MyThreadPool(10); | ||
|
|
||
| // ������ ������ � ������� � �������� ����� ���������� > 1000 �� | ||
| var task = pool.Submit(() => { Thread.Sleep(1000); return 1; }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Используйте ResetEvent-ы. Создали задачу, запустили, она встала на ResetEvent-е, проверили, что хотели, отпустили ResetEvent, продолжили, проверили, что всё ок. Thread.Sleep действительно не нужен
| var task = pool.Submit(() => 1); | ||
| pool.ShutDown(); | ||
| bool result = false; | ||
| Assert.That(result, Is.EqualTo(task.IsCompleted)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Она вполне могла успеть посчитаться до ShutDown
| /// <summary> | ||
| /// Get CancellationTokenSource | ||
| /// </summary> | ||
| public CancellationTokenSource Source => source; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я бы не стал это наружу показывать. Так любой может отменить токен без нашего ведома, и вообще.
| public int CountOfThreads => threads.Count; | ||
|
|
||
| // Method that puts the method in the execution queue | ||
| public void QueueWorkItem(Action func) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Опять-таки, метод public, поэтому кто угодно может его вызвать. Даже если поток уже остановлен.
| } | ||
|
|
||
| // При создании объекта в нем должно начать работу n потоков | ||
| foreach(var thread in threads) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| foreach(var thread in threads) | |
| foreach (var thread in threads) |
| /// </summary> | ||
| public void ShutDown() | ||
| { | ||
| lock(queue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| lock(queue) | |
| lock (queue) |
| lock(queue) | ||
| { | ||
| // Должны вернуть управление когда остановятся все потоки | ||
| // Хотим оставноить рабочий поток |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // Хотим оставноить рабочий поток | |
| // Хотим остановить рабочий поток |
No description provided.