TThread + Synchronize = bug

← →
Дмитрий Белькевич (2010-12-21 10:40) [40]

> .... попробовал :( виснет так же. Зацикливается внутри TThread.
> WaitFor.

Уже прогресс. Ищи, почему у тебя треды не останавливаются (не выходят из Execute).

← →
Дмитрий Белькевич (2010-12-21 10:41) [41]

>вот это FreeOnTerminate := true; - забудь как страшный сон...

лучше без этого, хотя если логика хорошо отработана - то можно.

← →
Дмитрий Белькевич (2010-12-21 10:45) [42]

> Зацикливается внутри TThread.> WaitFor.

Нет, не там. А в TThread.Execute.

← →
abb777 © (2010-12-21 12:06) [43]

> > Зацикливается внутри TThread.> WaitFor.
>
> Нет, не там. А в TThread.Execute.

Ну может я и балбес, но я вижу где отладчик ходит. А в данном случае он ходит именно внутри WaitFor(). А все треды висят (видно в окошке Threads) на том же самом WaitForSingleObject() который ну никак не может быть моим, я это проверял выставляя перед ним флаг. Так что не крутятся мои треды внутри Execute, а именно стоят. Иначе окошко Threads показало бы другое.

После полученного просветления относительно SendMessage/PostMessage попробовал их. SendMessage работает точно так же, как и Synchronize() хотя где при этом треды гуляют или стоят, я не смотрел. Сразу попробовал PostMessage(). Оно сработало. Правда был конфликт - в обработчике события у формы был вставлен Application.ProcessMessages, что приводило к рекурсии и, как результату EListError в этом месте:

procedure TREngine.DoDisplay; var i : integer; begin if Assigned(FOnStep) then begin begin EnterCriticalSection(FRCS); for i := 0 to FDispList.Count - 1 do if i < FDispList.Count then FOnStep(Self, FDispList.DisplayRec[i].FLayer, FDispList.DisplayRec[i].FCurStep); <<<<<<<<<<<<<<< FDispList.Clear; LeaveCriticalSection(FRCS); end; end; end;

Остался еще вопрос. Все работает с виду ОК, но только если и в вышеприведенном коде и в коде ниже вставлены крит. секции:

procedure TREngine.WndProc(var Msg: TMessage); begin with Msg do if Msg = WM_RE_NOTIFY then begin DoDisplay; Result := 1; end else begin Result := DefWindowProc(FWND, Msg, WParam, LParam); end; end; procedure TREDisplayThread.Execute; begin while not Terminated do begin EnterCriticalSection(FRCS); if FEngine.FDispList.Count > 0 then PostMessage(FEngine.FWND, WM_RE_NOTIFY, 0, 0); LeaveCriticalSection(FRCS); Sleep(0); end; end;

Если убрать КС в TREDisplayThread.Execute, то отображение сильно тормозится. А в TREngine.DoDisplay() вообще убирать нельзя, иначе AV. Если насчет причины AV еще понятно, то насчет тормозов с отображением не очень. Можете объяснить причину? Просто интересно.

← →
Ega23 © (2010-12-21 12:15) [44]

EnterCriticalSection(FRCS); for i := 0 to FDispList.Count - 1 do if i < FDispList.Count then FOnStep(Self, FDispList.DisplayRec[i].FLayer, FDispList.DisplayRec[i].FCurStep); <<<<<<<<<<<<<<< FDispList.Clear; LeaveCriticalSection(FRCS);

А вот и дедлок.
Запомни навсегда, выучи как Присягу:
EnterCriticalSection try finally LeaveCriticalSection end;

Иначе любое исключение после входа приведёт к тому, что выхода не будет. И все будут ждать.

← →
Ega23 © (2010-12-21 12:16) [45]

> Sleep(0);

Не надо 0, поставь ну там 20, например.

← →
abb777 © (2010-12-21 12:26) [46]

Спасибо, за ценное замечание. Хотите сказать, что это и есть причина первоначальной проблемы? Я, признаться, о таком не подумал. Там, с виду, негде было исключениям взяться. Сейчас везде вставлю.

После убирания Application.ProcessMessages из обработчика события у формы вариант с SendMessage() тоже работает, но вокруг него надо убрать крит. секцию. Иначе не работает.

← →
abb777 © (2010-12-21 12:45) [47]

Поправил. С SendMessage() 15 сек полет нормальный...
А так можно (см. отмеченные строчки)?
procedure TREngine.CancelIteration; var i : integer; f : boolean; begin WaitOn; for i := 0 to FLayers.Count - 1 do begin EnterCriticalSection(FRCS); f := true; try if FLayers.Layer[i].FIsActive then begin FLayers.Layer[i].FThread.Terminate; LeaveCriticalSection(FRCS); f := false; while FLayers.Layer[i].FIsActive do begin Sleep(20); end; end else begin LeaveCriticalSection(FRCS); f := false; end; finally if f then <<<<<<<<<<<<<<<< LeaveCriticalSection(FRCS); <<<<<<<<<<<<<<<< end; end; .......................... end;

← →
han_malign (2010-12-21 12:47) [48]

так, к слову:
if ... (FLayer.FEvent = INVALID_HANDLE_VALUE) ...

- почитай Return Values для CreateEvent

← →
abb777 © (2010-12-21 13:15) [49]

Ничего не сказано про результат в случае ошибки. Только про GetLastError.

У меня оно действительно не анализируется. Пока что не думал об этом. А процитированный код связан вот с этим:

destructor TRLayer.Destroy; begin CancelIteration; if FEvent <> INVALID_HANDLE_VALUE then begin CloseHandle(FEvent); FEvent := INVALID_HANDLE_VALUE; end;

← →
han_malign (2010-12-21 13:15) [50]

> procedure TREDisplayThread.Execute;
> begin
> while not Terminated do
> begin
> EnterCriticalSection(FRCS);
> if FEngine.FDispList.Count > 0 then
> PostMessage(FEngine.FWND, WM_RE_NOTIFY, 0, 0);
> LeaveCriticalSection(FRCS);
> Sleep(0);
> end;
> end;

- выкидываешь нафик
procedure TLayerThread.Execute; begin ......... EnterCriticalSection(FRCS); if not FLayer.FTerminating then begin FLayer.FEngine.FDispList.Add(TREDisplayRec.Create(FLayer, FLayer.FCurStep)); if FEngine.FDispList.Count = 1 then//подавляем накопление лишних нотификаций в лаге до захвата данных основным потоком PostMessage(FLayer.FEngine.FWND, WM_RE_NOTIFY, 0, 0); end; LeaveCriticalSection(FRCS);

← →
Ega23 © (2010-12-21 13:16) [51]

> А так можно (см. отмеченные строчки)?

Дурость.

Вот смотри:

procedure TForm1.Button1Click(Sender: TObject); begin try if CheckBox1.Checked then Exit; ShowMessage("try block"); finally ShowMessage("finally block"); end; end;

← →
han_malign (2010-12-21 13:57) [52]

> EnterCriticalSection(FRCS);
> for i := 0 to FDispList.Count - 1 do
> if i < FDispList.Count then
> FOnStep(Self,
> FDispList.DisplayRec[i].FLayer,
> FDispList.DisplayRec[i].FCurStep); <<<<<<<<<<<<<<<
> FDispList.Clear;
> LeaveCriticalSection(FRCS);

- поясняю схематически(каждый раз создавать/удалять объект моветон, но углубляться в кольцевые буфера мне что то лень):
TREDisplayRec.FLayer - должно быть копией мгновенного состояния, а не ссылкой на рабочий объект текущего состояния!!!
(иначе, вообще говоря, смысла в списке никакого нет)
EnterCriticalSection(FRCS); snapshotDispList:= FDispList; FDispList:= TКакойТоТамDispList.Create; LeaveCriticalSection(FRCS);//позволяю работать остальным потокам!!! for i := 0 to snapshotDispList.Count - 1 do //if i < FDispList.Count then - бред FOnStep(Self, snapshotDispList.DisplayRec[i].FLayer, snapshotDispList.DisplayRec[i].FCurStep); snapshotDispList.Free;
sapienti sat

← →
abb777 © (2010-12-21 20:05) [53]

To han_malign:

............
> FDispList:= TКакойТоТамDispList.Create;
...........

Я этого всего совсем не понял. Извините. У меня объект списка вообще создавался 1 раз. А Вы предлагаете его:
а) создавать в ОСНОВНОМ треде, а не там, где непосредственно появляются данные для него. Кто будет данные туда пихать и зачем, если можно просто опросить все TRLayer-ы об их состоянии?
б) создавать каждый раз при вызове процедуры отображения, а не 1 раз в самом начале.

> //подавляем накопление лишних нотификаций в лаге до захвата
> данных основным потоком

Идея была такая. Как Вы можете видеть, в каждом TRLayer.Execute фигурирует TRLayer.FCurStep, определяющая текущий шаг расчета. Эта переменная устанавливается одновременно со сбросом состояния FIsReady. Затем после опр. вычислений FIsReady устанавливается, что говорит о том, что данный объект (тред) закончил опр. этап расчетов на шаге FCurStep, и информация для его соседа готова. Сосед может начать шаг расчета. Сам же тред идет дальше и еще кое-чем занят какое-то время. И только после этого он сообщает треду отображения о том, что инфа по очередному шагу FCurStep готова и увеличивает FCurStep на 1 переходя к следующему шагу. Поэтому тред отображения должен иметь не только ссылку на TRLayer[i], откуда он берет всю инфу (которой много), но и значение FCurStep, для которого TRLayer[i] это все сосчитал, т.к. в момент работы треда отображения у рабочего треда FCurStep может быть уже другой, а состояние инфы для этого шага еще не подтверждено. Поэтому опрерируем указателем на TRLayer[i] и значением TRLayer[i].FCurStep, которые были переданы. Я не пойму, в чем изъян такого метода?

А если мы "подавим лишние нотификации", то инфа будет приходить отрывочными кусками по каким-то случайным (а не по всем) TRLayer-ам. Разве нет? Ведь список наполняется не одним TRLayer-ом, а разными и независимо друг от друга. И надо учесть, что процедура расчета в каждом "рабочем" треде будет работать на порядки (на много порядков) дольше, чем все эти создания объектов, помещения их в список и т.п. Там будет решаться система из 7 нелин. дифуров, а это довольно трудоемкий процесс, насколько я знаю...

To Ega23 :
Да, что-то я сам себя перехитрил. Решил стать святее папы римского :)

TO ALL:
Всем огромное спасибо за поддержку, только скажите все же, я правильно понимаю, что треды мои висели скорее всего на каком-то входе в крит. секцию, потому что из нее не было сделано выхода вследствие необработки какого-то исключения?

← →
abb777 © (2010-12-21 20:25) [54]

> EnterCriticalSection(FRCS);
> snapshotDispList:= FDispList;
> FDispList:= TКакойТоТамDispList.Create;
> LeaveCriticalSection(FRCS);//позволяю работать остальным
> потокам!!!
>

Кажется, я понял Вашу мысль. Спасибо.

To Ega23:

> Дурость.
>
> Вот смотри:
>
> procedure TForm1.Button1Click(Sender: TObject);
> begin
> try
> if CheckBox1.Checked then Exit;
> ShowMessage("try block");
> finally
> ShowMessage("finally block");
> end;
> end;

При ближайшем рассмотрении нашел неувзку. Вы показываете мне, что finally сработает даже при вызове Exit(), так? А я с этим и не спорил. Но в моем коде внутри try стоял LeaveCriticalSection(). Его два раза подряд можно вызывать? Я думал, что это неправильно. А если в finally не вставить проверку, то LeaveCriticalSection вызовется два раза.

← →
Дмитрий Белькевич (2010-12-22 00:03) [57]

> Ну может я и балбес, но я вижу где отладчик ходит. А в данном
> случае он ходит именно внутри WaitFor(

Не важно, где ходит отладчик. Важно, что waitfor не может дождаться конца метода execute треда. А пока execute не закончились, треды будут живыми. И кто знает, что они там наворотят после того, как половина программы будет убита. Вызов terminate тред не остановит - это только флаг.

← →
Дмитрий Белькевич (2010-12-22 00:15) [58]

>Внутри Execute() все с виду ОК - Terminated там анализируется

Тред никогда не выходит из Execute (читай - не заканчивается), если виснет на waitfor. Можешь флаг состояния какой-нибудь тестовый дописать или лог и проанализировать, когда треды начинаются, когда заканчивается. Если в реалтайме не удаётся отловить всё, хотя должно.
Мы у себя бывает по десятку разнородных тредов и по 50 одновременных ковыряем - то без логов повесились бы.

> При ближайшем рассмотрении нашел неувзку. Вы показываете
> мне, что finally сработает даже при вызове Exit(), так?
> А я с этим и не спорил. Но в моем коде внутри try стоял
> LeaveCriticalSection(). Его два раза подряд можно вызывать?
> Я думал, что это неправильно. А если в finally не вставить
> проверку, то LeaveCriticalSection вызовется два раза.
>

Дело именно в этом.
Например, я захожу в крит.секцию. Получаю какой-то там параметр. На основании значения этого параметра мне надо либо продолжить работу, либо выйти.
Например, параметр - string

Вариант 1
var s: string; b: Boolean; begin b := False; EnterCritSection try s := GetSomeParam; if (s = "мама") or (s = "мыла") or (s = "раму") then begin b := True; LeaveCritSection; Exit; end; s := DoSomethingWithParam(s); if (s = "мама") or (s = "мыла") or (s = "раму") then begin b := True; LeaveCritSection; Exit; end; ........ Ещё какой-то код finally if not b then LeaveCritSection; end;Вариант 2
Тот же код
var s: string; begin EnterCritSection try s := GetSomeParam; if (s = "мама") or (s = "мыла") or (s = "раму") then Exit; s := DoSomethingWithParam(s); if (s = "мама") or (s = "мыла") or (s = "раму") then Exit; ........ Ещё какой-то код finally LeaveCritSection; end;

Оба приведённых кода с точки зрения логики равнозначны. Но второй - как минимум тупо меньше писать надо. А как максимум - нет ненужных проверок (которые, теоретически, может выкинуть оптимизатор, но далеко не факт, что выкинет).
Собственно, всё.

← →
han_malign (2010-12-22 10:43) [60]

> А если мы "подавим лишние нотификации", то инфа будет приходить
> отрывочными кусками по каким-то случайным (а не по всем)
> TRLayer-ам. Разве нет?

- разве нет... для PostMessage, сообщение помещается в конец очереди сообщений.
Пока до него дойдет очередь:
Layers: CritSect(Count:=1;notify),CritSect(Count:=2),...,CritSect(Count:=N) Main(Message Dispatch Loop): CritSect((Count=N): Count:=0) Layers: CritSect((Count=0): Count:=1,notify)...

Критическая секция обеспечивает сериализацию, главное чтобы операция чтение+изменение была атомарной(внутри одной блокировки)...

Конечно можно просто проверять, что есть новые данные(Count > 0), но в некоторых сильно-нагруженных сценариях можно запросто словить переполнение очереди сообщений...

> вариант с SendMessage() тоже работает, но вокруг него надо убрать крит. секцию.

- естественно(обычно циклограмму в столбцах рисуют по количеству потоков, но тут все равно линейная сериализация получается):
Layer Thread: EnterCriticalSection SendMessage/Synchronize { - Layer Thread - в ожидании завершения синхронной операции Main Thread: EnterCriticalSection - опаньки!!! Main Thread: LeaveCriticalSection } Layer Thread: LeaveCriticalSectionклассический DeadLock
--------------------------------
Layer Thread: EnterCriticalSection PostMessage() Main Thread(Message Dispatch) EnterCriticalSection(call) - WAIT ... Layer Thread: LeaveCriticalSection Main Thread(Message Dispatch): ... EnterCriticalSection(return) Main Thread(Message Dispatch): LeaveCriticalSection

- мораль: как можно меньше вложенных синхронизаций, как можно короче критические секции...
Для данного примера это можно было решить так:
EnterCriticalSection(); haveNewData:= Count > 0;//Layer Threads Owned Data LeaveCriticalSection(); if( haveNewData )then Synchonize(); - хотя, в данном случае, для скаляра - можно спокойно использовать оптимистическую модель синхронизации - то есть вообще без критической секции...

TThread + Synchronize = bug Найти похожие ветки