Yes your approach seems good.
But only one problem.
if one client want to send any package to other it will use like follow as
procedure TForm1.ServerExecute(AThread: TIdPeerThread);
var
b: Byte;
AThread2: TMyPeerThread;
Stream: TMemoryStream;
begin
TMyPeerThread(AThread).SendQueue;
AThread.Connection.ReadBuffer(b, SizeOf(b));
if b = 89 then
begin
AThread.Connection.ReadBuffer(b, SizeOf(b));
Nick := AThread.Connection.ReadString(b);
Stream := TMemoryStream.Create;
try
AThread.Connection.ReadStream(Stream);
except
Stream.Free;
raise;
end;
AThread2 := FindAThreadFromNick(Nick);
if (AThread2 <> nil) and AThread2.AddToQueue(Stream) then Exit;
Stream.Free;
end
else
if b = 90 then
...
end;
"AThread2 := FindAThreadFromNick(Nick);"
"if (AThread2 <> nil) and AThread2.AddToQueue(Stream) then Exit;"
Yes this adds one task to destination queue of Athread.
But destination Athread can not aware this task unless it receives any package..
"TMyPeerThread(AThread).SendQueue;"
"AThread.Connection.ReadBuffer(b, SizeOf(b));"
Because it stays on "AThread.Connection.ReadBuffer(b, SizeOf(b));" line.
Any package must be processed by destination Athread suddenly..Dont have to wait...
Thanks..
"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"mustafakorkmaz" <mustafakorkmaz@gmail.com> wrote in message
>news:4610b53e$1@newsgroups.borland.com...
>
>> "if Task.Stream.size < 10000 then"
>> What is the meaing of this line?
>> if the size of stream smaller(<) then 10000 send package Else dont
>send.
>> "<" this expression means smaller...
>> Or there is some thing about streams that I dont know.
>
>My mistake. I thought you were using '>' instead of '<'.
>
>But my earlier statements are still valid, just reversed now. If the
>stream contains too much data, then none of it will be sent. So you
>are probably pushing multiple packages, or fewer but larger packages,
>into the stream before sending it, and then none of them are getting
>sent. So my suggestion to get rid of checking the stream Size
>altogether is still valid.
>
>> This server receive the packages from clients and it reads these
>> packages. And send these packages to all other users or only
>> one user.
>
>Only if the packages are small. But your receiving code is not
>enforcing that.
>
>> Server reads first byte of the packages and it decide to what
>> must it do? And it writes all data to stream and create one TTask
>> class. And it adds this class to queue of proper writer thread.
>> And writer thread sends this package to receiver.
>> Writer thread takes package from its queue one by one.
>
>Then why are you bothering with checking the stream Size at all? Each
>Task is a complete package. It must be sent as-is, or it may never
>get sent at all because your code is restricting it.
>
>Your code is wrong for other reasons, too. The biggest is that you
>have a buffer overflow vulnerability. You are allowing the client to
>specify any length it wants for the stream data, but you are not
>validating it before reading the data into your buffer. Instead of
>using a fixed-length buffer, just read directly into the stream
>instead. Then you can handle the full length regardless of what it
>is.
>
>Also, you are not doing any error checking in regards to locating the
>Nick to forward the data to. A package that specifies a non-existant
>user will crash your code because you are not validating anything.
>
>Also, your writer thread does not appear to be using a thread-safe
>list, either. You really should be.
>
>
>Try this instead:
>
> procedure TForm1.ServerExecute(AThread: TIdPeerThread);
> var
> b: Byte;
> AThread2: TIdPeerThread;
> Stream: TMemoryStream;
> No: Integer;
> Task: TTask;
> begin
> AThread.Connection.ReadBuffer(firstbyte, SizeOf(firstbyte));
>
> if b = 89 then
> begin
> AThread.Connection.ReadBuffer(b, SizeOf(b));
> Nick := AThread.Connection.ReadString(b);
>
> Stream := TMemoryStream.Create;
> try
> AThread.Connection.ReadStream(Stream); // calls
>ReadInteger() internally
>
> AThread2 := FindAThreadFromNick(Nick);
> if AThread2 <> nil then
> begin
> No := FindProperWriterThreadNumber(AThread2);
> if No <> -1 then
> begin
> Task := TTask.Create(AThread2, Stream);
> Stream := nil;
> try
> WriterThreads[No].AddTaskList(Task);
> except
> Task.Free;
> raise;
> end;
> end;
> end;
> except
> Stream.Free;
> raise;
> end;
> end
> else
> if b = 90 then
> ...
> end;
>
>
> procedure TWriterThread.Execute;
> var
> Task: TTask;
> List. TList;
> Count: Integer;
> begin
> While not Terminated do
> begin
> Count := 0;
> try
> List := MyTaskList.LockList; // MyTaskList is a
>TThreadList now
> try
> while List.Count > 0 do
> begin
> Task := TTask(MyTaskList[0]);
> MyTaskList.Delete(0);
>
> try
> criticalnumber :=
>TMyClient(Task.AThread.Data).criticalnumber;
>
>
>EnterCriticalSection(CriticalArray[criticalnumber]);
> try
> waittime := GetTickCount;
>
>Task.AThread.Connection.WriteStream(Task.Stream);
> if GetTickDiff(waittime, GetTickCount)
>> 3500 then
> ThreadExceptionToLog(' time:' +
>IntToStr(GetTickDiff(waittime, GetTickCount)) + ' size:' +
>IntToStr(Task.Stream.Size));
> finally
>
>LeaveCriticalSection(CriticalArray[criticalnumber]);
> end;
> finally
> Task.Free;
> end;
> end;
> finally
> Count := List.Count;
> MyTaskList.UnlockList;
> end;
> except
> end;
> if Count = 0 then Sleep(10);
> end;
> end;
>
>Now, with all of that said, your use of writer thread arrays, critical
>section arrays, etc is very suspicious and potentially very dangerous
>as you are not using them very safely. I strongly suggest you
>re-think your design. A better way would be to derive a new class
>from TIdPeerThread and let it handle its own queuing system locally.
>Then your server code becomes a lot cleaner and safer. For example:
>
> type
> TMyPeerThread = class(TIdPeerThread)
> private
> FQueue: TThreadList;
> FWriteLock: TCriticalSection;
> public
> constructor Create(ACreateSuspended: Boolean = True);
>override;
> destructor Destroy; override;
> function AddToQueue(Stream: TStream): Boolean;
> procedure SendQueue;
> end;
>
>
> constructor TMyPeerThread.Create(ACreateSuspended: Boolean =
>True);
> begin
> inherited;
> FQueue := TThreadList.Create;
> FWriteLock := TCriticalSection.Create;
> end;
>
> destructor TMyPeerThread.Destroy;
> var
> List: TList;
> I: Integer;
> begin
> if FQueue <> nil then
> begin
> List := FQueue.LockList;
> try
> for I := List.Count-1 do
> TStream(List[I]).Free;
> finally
> FQueue.UnlockList;
> end;
> end;
> FQueue.Free;
> FWriteLock.Free;
> inherited;
> end;
>
> function TMyPeerThread.AddToQueue(Stream: TStream): Boolean;
> begin
> Result := False;
> if Stream <> nil then
> try
> FQueue.Add(Stream);
> Result := True;
> except
> end;
> end;
>
> procedure TMyPeerThread.SendQueue;
> var
> List: TList;
> Stream: TStream;
> StartTime: Cardinal;
> begin
> List := FQueue.LockList;
> try
> while List.Count > 0 do
> begin
> Stream := TStream(List[0]);
> List.Delete(0);
>
> try
> FWriteLock.Enter;
> try
> StartTime := GetTickCount;
> Connection.WriteStream(Stream);
> if GetTickDiff(StartTime, GetTickCount) > 3500
>then
> ThreadExceptionToLog(' time:' +
>IntToStr(GetTickDiff(StartTime, GetTickCount)) + ' size:' +
>IntToStr(Stream.Size));
> finally
> FWriteLock.Leave;
> end;
> finally
> Stream.Free;
> end;
> end;
> finally
> FQueue.UnlockList;
> end;
> end;
>
>
> constructor TForm1.Create(AOner: TComponent);
> begin
> inherited;
> TCPServer.ThreadClass := TMyPeerThread;
> end;
>
> procedure TForm1.ServerExecute(AThread: TIdPeerThread);
> var
> b: Byte;
> AThread2: TMyPeerThread;
> Stream: TMemoryStream;
> begin
> TMyPeerThread(AThread).SendQueue;
>
> AThread.Connection.ReadBuffer(b, SizeOf(b));
>
> if b = 89 then
> begin
> AThread.Connection.ReadBuffer(b, SizeOf(b));
> Nick := AThread.Connection.ReadString(b);
>
> Stream := TMemoryStream.Create;
> try
> AThread.Connection.ReadStream(Stream); // calls
>ReadInteger() internally
> except
> Stream.Free;
> raise;
> end;
>
> AThread2 := FindAThreadFromNick(Nick);
> if (AThread2 <> nil) and AThread2.AddToQueue(Stream) then
>Exit;
>
> Stream.Free;
> end
> else
> if b = 90 then
> ...
> end;
>
>
>Gambit
