-
Notifications
You must be signed in to change notification settings - Fork 0
Test3 #7
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: master
Are you sure you want to change the base?
Test3 #7
Conversation
| { | ||
| static void Main(string[] args) | ||
| { | ||
| Console.WriteLine("Hello World!"); |
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.
Это не нужно
| @@ -0,0 +1,18 @@ | |||
| FROM mcr.microsoft.com/dotnet/runtime:5.0 AS base | |||
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.
Вы теперь даже задачу контрольной в Docker собираете? :)
| @@ -0,0 +1,42 @@ | |||
| using System.Linq; | |||
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.
UnitTest1.cs как имя файла не очень, и не совпадает с именем класса внутри.
| private readonly Object _lockAdded = new(); | ||
| private readonly Object _lockDelete = new(); | ||
| private int _size = 0; | ||
| private bool flag = false; |
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.
Флаг чего? Плохое имя переменной
|
|
||
| value = _queue[1].value; | ||
| _queue.RemoveRange(1, 1); | ||
| Volatile.Write(ref flag, false); |
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.
Так не получится отследить ситуацию, когда сразу несколько потоков ждут на Dequeue. Да и не нужно это, можно сигналить Monitor.PulseAll, даже если нас никто не ждёт
| } | ||
|
|
||
| value = _queue[1].value; | ||
| _queue.RemoveRange(1, 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.
А вот несинхронизированные _queue.RemoveRange и _queue.Insert могут всё сломать: _queue — это List, который не даёт гарантий потокобезопасности, и тут мы обращаемся к его методам из-под разных lock-ов, которые никак не координируются друг с другом. Потенциальная гонка.
| for (int i = 0; i < threads.Length; ++i) | ||
| { | ||
| var localI = i; | ||
| Task.Run(() => |
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.
Так Вы теряете ссылку на задачу, и может оказаться, что не даёте ей доработать. У меня вот тест не проходит, потому что к 37-й строке размер очереди 49, а надо 50 — одна из задач не закончила одну итерацию цикла :)
| array = array.Reverse().ToArray(); | ||
| Assert.AreEqual(50, queue.Size()); | ||
| Assert.AreEqual(array[0].Item1, queue.Dequeue()); | ||
| Assert.Pass(); |
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.
Это не нужно
| /// </summary> | ||
| public class QueuePriory | ||
| { | ||
| private List<(int value, int priority)> _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.
Стоило сделать её генериком, хранить только инты не очень полезно
| while (_size == 0) | ||
| { | ||
| Volatile.Write(ref flag, true); | ||
| Monitor.Wait(_lockAdded); |
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.
Вообще, ждать и сигналить можно только захваченный замок, насколько я помню. Это ведь не WaitHandle. Два независимых замка друг с другом всё равно не синхронизируются.
No description provided.