TSeverThead, TServerSocket

← →
dozcent © (2008-08-07 19:53) [0]

Здравствуйте уважаемые мастера.
Что то у меня не совсем правильно работает мой сервер осованный на TServerSocket. Есть пара вопросов по поводу класса TServerThread. Стоит ли его перегружать внутренними функциями, или лучше сделать большую функцию ClientExecute.
Стоит ли добавлять в класс компоненты, допустим, для связи с бд, или создать их в функции?

Вот код, который работает кривовастенько... Возникают ошибки, прога иногда вылетает, остаются подвешенные потоки,
перестаёт выполнять хранимые процедуры в БД.

constructor TServerThread.Create(CreateSuspended: Boolean; ASocket: TServerClientWinSocket;
DataBaseName: string; DBUser: string; DBPassword: string);
begin
inherited Create(CreateSuspended, ASocket);
List := TStringList.Create;
LogType := "";
DataBase := TIBDatabase.Create(nil);
Transaction := TIBTransaction.Create(DataBase);
Proc := TIBStoredProc.Create(DataBase);
Query := TIBQuery.Create(DataBase);
try
DataBase.DatabaseName := DataBaseName;
DataBase.Params.Add("user_name=" + DBUser);
DataBase.Params.Add("password=" + DBPassword);
DataBase.LoginPrompt := false;
DataBase.SQLDialect := 1;
DataBase.DefaultTransaction := Transaction;
DataBase.Connected := true;
Transaction.Active := true;
Query.Database := DataBase;
Query.Transaction := Transaction;
Proc.Database := DataBase;
proc.Transaction := Transaction;
except
on e: Exception do
LogError("[" + DateToStr(Now) + "]:::::: ThreadCreate::::" + e.Message);
end;
List.Add("[" + DateToStr(Now) + "]::: Thread created sucefuly");
end;

procedure TServerThread.ClientExecute;
var
i, j: integer;
buf1: array[0..2] of byte;
c, cz, temp: byte;
MessTest: TDisplayMesage;
begin
inherited FreeOnTerminate := TRUE;
try
fSocketStream := TWinSocketStream.Create(ClientSocket, ssTimeOut);
try
while (not Terminated) and (ClientSocket.Connected) do
begin
if (not Terminated) and (not fSocketStream.WaitForData(ssTimeOut)) then
begin
List.Add("[" + DateToStr(Now) + "]::: Thread closed by timeout ");
ClientSocket.Close;
fSocketStream.Free;
Free;
end;
DevNo := ClientSocket.ReceiveText;
DevNo := Trim(DevNo);
List.Add("[" + DateToStr(Now) + "]::: Thread recive number ");
if DevNo = "TEST" then
ClientSocket.SendText("OK")
else
begin
if DeviceInDB then //function TServerThread.DeviceInDB:Boolean Check board is present =============
begin
if UpdateTime then //function TServerThread.UpdateTime:Boolean Check update time =============
begin
CreatePackedge; //procedure TServerThread.CreatePackedge; Getting record for this board ======
for j := 0 to 18 do //===================== Sending Erase command =============
begin
temp := clear_command[j];
ClientSocket.SendBuf(temp, 1);
end;
List.Add("[" + DateToStr(Now) + "]::: Sending erase command " + DevNo);
for j := 0 to Length(mess) - 1 do //===================== Sending Messages ==================
for i := 0 to Length(mess[j].PrepereMasage) - 1 do
ClientSocket.SendBuf(mess[j].PrepereMasage[i], 1);
List.Add("[" + DateToStr(Now) + "]::: Sending packadge " + DevNo);
if (not Terminated) and (not fSocketStream.WaitForData(ssTimeOut * 5)) then //================ Waiting For Answer =====================
begin
List.Add("[" + DateToStr(Now) + "]::: Closed by time out on recive message count " + DevNo);
ClientSocket.Close;
fSocketStream.Free;
Free;
end;
cz := ClientSocket.ReceiveLength; //================ Reciving Message Count =================
ClientSocket.ReceiveBuf(buf1, cz);
List.Add("[" + DateToStr(Now) + "]::: resived message count " + DevNo);
if buf1[2] >= MessCount then //================ SetUP update time ======================
begin
SetUpUpdateTime;
logstr := "Äèñïëåé ïîäêëþ÷èëñÿ è ïîëó÷èë " + IntToStr(buf1[2]) + " ôàéëîâ." + #13#10;
LogType := "UpdateOK";
LogConnection;
end
else
begin
logstr := "Äèñïëåé ïîäêëþ÷èëñÿ íî ïîëó÷èë " + IntToStr(buf1[2]) + " ôàéëîâ, à íå " + IntToStr(MessCount + 2);
LogType := "TrasferFaild";
LogConnection;
end;
end; //================ Closing Conection =================
if LogType = "" then
begin
logstr := "Äèñïëåé ïîäêëþ÷èëñÿ, íî âðåìÿ îáíîâëåíèÿ åù¸ íå ïðèøëî.";
LogType := "NoTime";
LogConnection;
end;
end
else
begin
MessTest := TDisplayMesage.Create;
MessTest.CR;
MessTest.PrepMess;
for i := 0 to Length(MessTest.PrepereMasage) - 1 do
ClientSocket.SendBuf(MessTest.PrepereMasage[i], 1);
List.Add("[" + DateToStr(Now) + "]::: Sending number for device out DB " + DevNo);
end;
if LogType = "" then
begin
logstr := "Ïîäêëþ÷åííîå óñòðîéñòâî îòñóòñòâóåò â áàçå äàííûõ.";
LogType := "NoDevice";
LogConnection;
ClientSocket.Close;
end;
List.Add("[" + DateToStr(Now) + "]::: Connection close It""s OK " + DevNo);
LogServerEvents(List);
ClientSocket.Close;
end;
end;
finally
List.Add("[" + DateToStr(Now) + "]::: Finnaly " + DevNo);
fSocketStream.Free;
Free;
end;
except
on e: exception do
begin
LogError("[" + DateToStr(Now) + "]::: ThreadExecute::::" + e.Message);
ClientSocket.Close;
Terminate;
end;
end;

end;

← →
Сергей М. © (2008-08-08 08:26) [1]

> лучше сделать большую функцию ClientExecute

Не функцию, а процедурный метод.
На то он, виртуальный, кстати, и существует, чтобы его можно было перекрыть в наследнике и реализовать в нем свою логику.

> или создать их в функции

Именно.
Особенно в твоем случае с потоконебезопасными объектами доступа к IB.

> DataBase.Connected := true;

Засада поджидает уже в этой строчке - коннект ты осуществляешь в дополнительном слушающем потоке сервера, вызвавшем конструктор, а последующую работу с базой в теле ClientExecute ты осуществляешь уже в другом доп.потоке, ассоциированном со вновь установленным сокетным соединением.

> FreeOnTerminate := TRUE;

Для объектов-наследников TServerClientThread "саморазрушение" недопустимо - сервер сам управляет временами жизни объектов-потоков, регистрируемых им во внутреннем кэше после конструирования.

← →
Сергей М. © (2008-08-08 08:29) [2]

И вообще - твоя логика работы с методами приема/передачи объекта TCustomWinSocket не выдерживает никакой критики.
И доп.потоки тут ни причем - при такой логике граблей можно нахватать в любом потоке.

← →
Slym © (2008-08-08 09:14) [3]

dozcent © (07.08.08 19:53)
Стоит ли его перегружать внутренними функциями
Стоит! без паллитры в этом листинге не разберешься
dozcent © (07.08.08 19:53)
Стоит ли добавлять в класс компоненты
У где они у тебя? наруже? нипорядок!!!
1. нитрогай constructor TServerThread.Create... перенеси все (создание конектов и прочей фегни) в ClientExecute, оставить можешь только присвоение DataBaseName/DBUser/DBPassword во внутренние переменные и пользовать их в ClientExecute...
2. inherited FreeOnTerminate := TRUE; - НИЗЯ!!!
3. Free; - АХТУНГ! НИЗЯ!
4.var buf1: array[0..2] of byte; ... cz := ClientSocket.ReceiveLength;//ОПТИМИСТ, ДА? ClientSocket.ReceiveBuf(buf1, cz);
А я тебе фсе телевидение испорчу :) послав более SizeOf(buf1) и получишь AV или уязвимость с выполнением вредоносного кода
5.
fSocketStream := TWinSocketStream.Create(ClientSocket, ssTimeOut); try ... if (not Terminated) and (not fSocketStream.WaitForData(ssTimeOut)) then ... fSocketStream.Free; Free; end; ... finally ... fSocketStream.Free; Free; end;
итого налицо двойное убийство, с особой тяжестью... приговор отрубление кривых рук сроком на 10 лет :)
6. Протокол основан на честном слове - никаких проверок на размеры и соответствие полей, а в сочетании с сокетами которые умеют клеить и дробить пакеты ты огребеш... и в DevNo := ClientSocket.ReceiveText; ты получиш или пол сообщения или несколько

← →
Slym © (2008-08-08 09:17) [4]

dozcent © (07.08.08 19:53)
PrepereMasage
Мнебы сейчас массажистку...
кОкай языг учiлЪ?

← →
Сергей М. © (2008-08-08 09:52) [5]

Короче говоря, в приведенном коде косяк на косяке косяком погоняет)

Проще выкинуть его в топку и переписать "с нуля" - с чувством, с толком, с расстановкой)

← →
dozcent © (2008-08-09 15:08) [6]

Разъясните тогда пожалуйста, кто когда и почему удаляется в этой конструкции, а то я что-то непойму...
Если у меня
type TServerThread = class(TServerClientThread) .......... private DataBase: TIBDatabase; .......... end;
Я вызываю её создание в ClientExecute, то когда её нужно убить...
И почему нельзя
FreeOnTerminate := TRUE;
Что происходит при ClientSocket.Close кроме закрытия сокета?

← →
dozcent © (2008-08-09 15:14) [7]

Вот тут кое что подправил, появилась куча ошибок и вобще что попало..
procedure TServerThread.ClientExecute; var buf1: array[0..2] of byte; c, cz: byte; begin try fSocketStream := TWinSocketStream.Create(ClientSocket, ssTimeOut); try while (not Terminated) and (ClientSocket.Connected) do begin if (not Terminated) and (not fSocketStream.WaitForData(100000)) then begin ClientSocket.Close; end; ConnectToDb; cz := ClientSocket.ReceiveLength; if cz <> 17 then//если длинна не равна 17, то это не моё устройство!!! begin DataBase.Free; ClientSocket.Close; fSocketStream.Free; end; IMEI := ClientSocket.ReceiveText; IMEI := Trim(IMEI); LogServerEvents("event"); LogServerEvents("event"); if DeviceInDB then //Check board is present begin if UpdateTime then // Check update time begin CreatePackedge; // Getting record for this device SendingMessage; if (not Terminated) and (not fSocketStream.WaitForData(1000000)) then begin ClientSocket.Close; end; cz := ClientSocket.ReceiveLength; //Reciving Message Count ClientSocket.ReceiveBuf(buf1, cz); LogServerEvents("event"); if buf1[2] >= MessCount then // SetUP update time begin SetUpUpdateTime; LogConnection(1); ClientSocket.Close; end else begin LogConnection(2); ClientSocket.Close; end; end else begin LogConnection(3); ClientSocket.Close; end; end else begin SendingMessageToNewDevice; LogConnection(4); ClientSocket.Close; end; LogServerEvents("event") end; finally LogServerEvents("event"); DataBase.Free; fSocketStream.Free; end; except on e: exception do begin LogServerEvents("event"); LogError("Error"); DataBase.Free; fSocketStream.Free; ClientSocket.Close; end; end; end;
Что то мне непонятно, когда же эти потоки убиваются!

← →
Сергей М. © (2008-08-10 21:10) [8]

> if cz <> 17 then//если длинна не равна 17, то это не моё
> устройство!!!

TCP - протокол поточный !

тебе это о чем-то говорит ?

← →
dozcent © (2008-08-10 21:52) [9]

> TCP - протокол поточный !

В данном контексте ни о чём не говорит....

← →
dozcent © (2008-08-10 21:54) [10]

Меня больше волнует, почему при имеющемся exception у меня вылазит ошибка!!!!

← →
Сергей М. © (2008-08-10 22:12) [11]

О контексте ты не сказал ни слова.

> if cz <> 17 then//если длинна не равна 17, то это не моё
> устройство!!!

С какого перепугу-то ?

Мало ли сколько байт принятов ДАННЫЙ момент времени ?

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

> почему при имеющемся exception у меня вылазит ошибка!

Куда она у тебя "вылазит" ?

← →
dozcent © (2008-08-10 22:21) [12]

Куда, куда - на экран...
Чесно говоря, сейчас не до гипотетических ошибок, эти 17 байт посылает модем неизвесно каким образом, и они всегда доходят в 100% случаев...
Но очень интересно посмотреть как принять их правильно.
сейчас мне нужно что бы ошибки не останавливали работу сервера и не вылазили на экран, я их ловлю во всех функциях, а они вылазят на экран....
И немогли бы вы кратко объяснить принцип удаления потока из памяти в данной схеме?

← →
dozcent © (2008-08-10 22:26) [13]

Да, и ещё....
Непонятное мне явление...
В блоке finally есть обращение к процедуре LogServerEvents("event");
которое в некоторых потоках срабатывает два раза.....

← →
Сергей М. © (2008-08-10 22:49) [14]

> как принять их правильно

Их надо аккумулировать.

> 17 байт посылает модем

Какой еще нахрен модем ? Речь в твоем коде идет о сокете как об источнике поточных данных ..

> не до гипотетических ошибок

Какая еще нахрен "гипотетика", если источник в одном углу Тырнета, а приемник совсем в другом ?

> они всегда доходят в 100% случаев

Не надо уже трындеть.

Разнеси свои приемник и передатчик по разным углам Тырнета и твоя логика тут же потерпит крах.

И вообще - отладчик тебе в руки, коль ты не желаешь слушать что тебе говорят.

← →
dozcent © (2008-08-10 22:58) [15]

При чём тут нежелаю, я прошу помощи, что бы сказали что-то конкретное...
Что такое аккомулировать? Чем?
В цикле принимать данные, пока не наберёт 17?
Вы имеете в виду, что получать и посылать данные надо fSocketStream"ом?
Или что?
А какие мысли по поводу ошибки, которая на экран вылазит?

← →
dozcent © (2008-08-10 23:01) [16]

А когда я говорил про "ГИПОТЕТИКУ", я имел в виду, что такой проблеммы сейчас не существует, а вот ошибки!!!!!!!!! Которые на экран вылазят!!!!
И о которых я прошу помощи!!!

← →
Сергей М. © (2008-08-10 23:07) [17]

> В цикле принимать данные, пока не наберёт 17?

В цикле или не в цикле - не важно.

Но если ты ожидаешь от партнера по соединению 17 каких-то байт, то их сначала нужно гарантированно принять.

> cz := ClientSocket.ReceiveLength;

Читай внимательно справку по логике работы этого св-ва - твое понимание этой логики НЕверно.

← →
Сергей М. © (2008-08-10 23:09) [18]

> ошибки!!!!!!!!! Которые на экран вылазят

Да по барабану что там у тебя куда-то "вылазит" !

Ошибки в твоей логике начинаются гораздо раньше по ходу твоего алгоритма !

И рано или поздно они дадут о себе знать !

Пойми ты наконец-то это)

← →
Сергей М. © (2008-08-10 23:11) [19]

И зачем, спрашивается, ковыряться в куче вываленного тобой дерьма, если серьезная ошибка в транспортной логике очевидна уже в первых строках приведенного кода ?

Нет аккумулятора, нет машины состояния - дальнейший разговор лишен смысла.

← →
dozcent © (2008-08-10 23:22) [20]

Как реализовать этот аккомулятор и машину состояния?

Вы имеете в виду такую конструкцию...
var ac, readlen : integer; begin FillChar( fRequest, REQUESTSIZE, 0 ); ac := 0; repeat readlen := fSocketStream.Read( fRequest[ac],1024 ); ac := ac+readlen; until ( readlen = 0 ) or ( ac = REQUESTSIZE ); end;

Даже для трёх байт такое надо делать?

> Даже для трёх байт такое надо делать?

Да, даже для двух.

Если тебе пообещали налить бутыль и ты ждешь обещанное, то это вовсе не означает,что ее нальют тебе все разом и сей секунд после обещания - тебе ее когда-нибудь обязательно нальют, но вправе наливать ее через заранее неизвестного/меняющегося диаметра воронку и заранее неизвестными порциями.

Так что готовь тару объемом в бутыль и жди пока она наполнится, коль договорились)

Время наполнения и порции, коими наполнение должно происходить, в общем случае тебе не известно)

> FillChar( fRequest, REQUESTSIZE, 0 );

А это вообще нафиг не нужно.

А в этой функции лучше ставить минимум или максимум?
readlen := fSocketStream.Read( fRequest[ac],1024 );
Тобишь 1 или 17?

приведи протокол устройтсва...

dozcent © (10.08.08 23:26) [21]
Вы имеете в виду такую конструкцию...
почти
procedure ReadBufferEx(SocketStream:TWinSocketStream;var Buffer; Count: Longint); var PBuffer:PByte; Readed:integer; begin PBuffer:=@Buffer; while Count>0 do begin readed:=SocketStream.Read(PBuffer^,Count); Dec(Count,readed); Inc(PBuffer,readed); end; end;

Должно получиться чтото типа этого:
procedure TServerClientThreadEx.ClientExecute; var SocketStream:TWinSocketStream; Packet:array[0..16] of byte; Packet2:array[0..2] of byte; begin try SocketStream:=TWinSocketStream.Create(ClientSocket, 30); try while (not Terminated) and (ClientSocket.Connected) do begin if not SocketStream.WaitForData(SocketStream.TimeOut) then raise Exception.Create("Protocol: timeout"); ReadBufferEx(SocketStream,Packet,SizeOf(Packet)); LogServerEvents("Packet Readed"); if ClientSocket.ReceiveLength<>0 then LogServerEvents("Packet fully Readed, but some socket data still avaliable... Error?"); SetString(IMEI,Packet,SizeOf(Packet)); ConnectToDb; try if DeviceInDB then begin if UpdateTime then begin CreatePackage; SendingMessage; if not SocketStream.WaitForData(SocketStream.TimeOut) then raise Exception.Create("Protocol: timeout"); ReadBufferEx(SocketStream,Packet2,SizeOf(Packet2)); LogServerEvents("Packet2 Readed"); if ClientSocket.ReceiveLength<>0 then LogServerEvents("Packet2 fully Readed, but some socket data still avaliable... Error?"); if Packet2[2]>=MessCount then begin SetUpUpdateTime; LogConnection(1); end else begin LogConnection(2); end; end else begin LogConnection(3); end; end else begin SendingMessageToNewDevice; LogConnection(4); end; finally DisconnectFromDb; end; LogServerEvents("We finish Protocol loop"); end; LogServerEvents("We break IO loop"); finally SocketStream.Free; end; except on e: exception do LogError("Error: "+e.Message); end; end;

> Тобишь 1 или 17?

Читай ровно столько, сколько осталось прочитать из этих 17-ти ожидаемых байт

запросил к чтению 17, фактически прочитал 2, осталось прочитать 15
запросил к чтению 15, фактически прочитал 5, осталось прочитаь 10
запросил к чтению 10, фактически прочитал 1, осталось прочитаь 9
запросил к чтению 9, фактически прочитал 4, осталось прочитаь 5

и т.д. до тех пор пока "осталось прочитаь" не станет равным 0

Эта логика применима во всех случаях, когда заранее известен размер очередного инф.сообщения, ожидаемого к приему от партнера на данном этапе исполнения протокола инф.обмена.

TSeverThead, TServerSocket Найти похожие ветки