🚀 go-pugleaf

"Paul" <plmacca@clara.co.uk> wrote in message
news:425448ec$1@newsgroups.borland.com...

> Its all as per the tutorial really.

Then I can tell you that the tutorial code has problems in it.

> void __fastcall TMyServerThread::ClientExecute(void)
> {
>    // make sure connection is active
>    while (!Terminated && ClientSocket->Connected)
>    {
>       try
>       {
>          // Now, use TWinSocketStream to read or write information
>          // over a blocking socket connection
>          TWinSocketStream *pStream = new TWinSocketStream(ClientSocket,
> CLIENTWAITTIME);

Rather than creating a new TWinSocketStream each time the loop reiterates, I
would suggest moving the TWinSocketStream creation to be outside of the
loop.  That way, you create 1 new stream for each connection, not 1 new
stream for each individual read/write operation.

>             // give the client 60 seconds to start writing
>             if (pStream->WaitForData(pStream->TimeOut))

There is a difference in meaning between the TimeOut property of the stream
and the Timeout parameter of WaitFor().  The TimeOut property only applies
to Read() and Write().  It controls how long they wait for data to actually
transmit before then returning.  Then Timeout parameter of WaitFor(), on the
other hand, specified how long the stream waits for the socket be become
signaled as readable, meaning that data is available to be read, or the
socket has been disconnected.

>                if (pStream->Read(buffer, sizeof(buffer)) == 0)
>                {
>                   // (if can't read in 60 seconds) than close the
connection
>                   // ClientSocket->Close();
>                }

You are not taking int account that Read() can return -1 if an error occurs.
You should also be storing the returnvalue into a variable, because Read()
specifies how much data was actually received, which you can then use later
in determining how much buffered data is actually available for processing.

>                   // Client to Server test text
>                   // Form1->Memo1->Lines->Add(AnsiString("(Client) ")
> +AnsiString(buffer) );

That code is not thread safe at all.  You cannot safely access the GUI
directly from inside a worker thread.  You need to use the thread's
Synchronize() method for that.

You are also not taking into account the condition where Read() received as
many bytes as the buffer can hold maximum.  In which case, the buffer would
not be null-terminated.  To work around that, you should be using
AnsiString's second constructor that takes a buffer length, so that you do
not accidentally convert the wrong memory.

    int iRead = pStream->Read(buffer, sizeof(buffer);
    //...
    ... = AnsiString(buffer, iRead);

>                   // Back again to Client
>                   pStream->Write( buffer, sizeof(buffer));

You are not taking into account the actual return value of Read(), thus you
may be writing more data back to the client than it actually sent to begin
with.

    int iRead = pStream->Read(buffer, sizeof(buffer));
    //...
    pStream->Write(buffer, iRead);

>          HandleException();

I would suggest getting rid of that altogether.  All it does is calls the
thread's Synchronize() method in order to show the exception in a popup
message.  Not very useful to the rest of your code.  If you are going to use
exception handling, then you should be prepared to process the exception
yourself.

>     if (ClientSocket1->Active == true)
>       if (Socket->Connected == true)

Rather than doing that, call the Socket's ReceiveLength() method instead.
If it returns a value > 0 then there is data available to read.  If Active
or Connected were false, the event would not be triggered in the first
place.

> void __fastcall TForm2::ClientSocket1Write(TObject *Sender,
TCustomWinSocket
> *Socket)

You are misusing that event.  It is only triggered when the connection is
first established, or when the socket becomes writable again after a
previous writing operation reported blocking when the socket is used in
non-blocking mode.  Use the OnConnect event instead for the former, and
ignore the latter altogether as it will only complicate your code more than
it is currently set up to handle.


With that said, try this code instead:

    --- Server ---

    int __fastcall TMyServerThread::Read(TWinSocketStream *Stream, void
*Buffer, int BufSize)
    {
        try
        {
            int iRead = Stream->Read(Buffer, BufSize);
            return (iRead < 0) ? 0 : iRead;
        }
        catch(const Exception &)
        {
            return 0;
        }
    }

    bool __fastcall TMyServerThread::Write(TWinSocketStream *Stream, void
*Buffer, int BufSize)
    {
        LPBYTE pBuf = (LPBYTE) Buffer;

        try
        {
            while( BufSize > 0 )
            {
                int iSent = Stream->Write(pBuf, pEnd-pBuf);
                if( iSent <= 0 )
                    return false;

                pBuf += iSent;
                BufSize -= iSent;
            }
            return true;
        }
        catch(const Exception &)
        {
            return false;
        }
    }

    void __fastcall TMyServerThread::ClientExecute(void)
    {
        char buffer[BUFFERSIZE];

        TWinSocketStream *pStream = new TWinSocketStream(ClientSocket,
5000);
        try
        {
            while( (!Terminated) && (ClientSocket->Connected) )
            {
                if( pStream->WaitForData(1000) )
                {
                    int iRead = Read(pStream, buffer, sizeof(buffer));
                    if( iRead > 0 )
                    {
                        // declare FBuffer to be a member of the thread
class
                        FBuffer = AnsiString(buffer, iRead);
                        Synchronize(ReportBuffer);

                        // Process data here...

                        if( Write(pStream, buffer, iRead) )
                            continue;
                    }

                    ClientSocket->Close();
               }
            }
        }
        __finally
        {
            delete pStream;
        }
    }

    void __fastcall TMyServerThread::ReportBuffer()
    {
        Form1->Memo1->Lines->Add("(Client) " + FBuffer);
    }


    --- Client ---

    void __fastcall TForm2::ClientSocket1Read(TObject *Sender,
TCustomWinSocket *Socket)
    {
        Memo1->Lines->Add("Read Event Handler");

        int iBytes = Socket->ReceiveLength();
        if( iBytes > 0 )
            Memo1->Lines->Add( "(Server) " + Socket->ReceiveText() );
    }

    void __fastcall TForm2::ClientSocket1Connect(TObject *Sender,
TCustomWinSocket *Socket)
    {
        Memo1->Lines->Add("Connect Event Handler");
        Memo1->Lines->Add("..Port Active");
        int i = Socket->SendText("This text is passed to and from");
        Memo1->Lines->Add(i);
    }


Now, with that said, I do not advise you to use SendText() and ReceiveText()
in the first place.  They do not provide for adequate error handling, nor do
they help with handling split packets (where a packet requires multiple
read/writes in order to receive/send it all).  A better implementation would
be as follows:

    --- Server ---

    bool __fastcall TMyServerThread::Read(TWinSocketStream *Stream, void
*Buffer, int BufSize)
    {
        LPBYTE pBuf = (LPBYTE) Buffer;

        try
        {
            while( BufSize > 0 )
            {
                int iRead = Stream->Read(pBuf, BufSize);
                if( iRead <= 0 )
                    return false;

                pBuf += iRead;
                BufSize -= iRead;
            }
            return true;
        }
        catch(const Exception &)
        {
            return false;
        }
    }

    bool __fastcall TMyServerThread::Write(TWinSocketStream *Stream, void
*Buffer, int BufSize)
    {
        LPBYTE pBuf = (LPBYTE) Buffer;

        try
        {
            while( BufSize > 0 )
            {
                int iSent = Stream->Write(pBuf, pEnd-pBuf);
                if( iSent <= 0 )
                    return false;

                pBuf += iSent;
                BufSize -= iSent;
            }
            return true;
        }
        catch(const Exception &)
        {
            return false;
        }
    }

    void __fastcall TMyServerThread::ClientExecute(void)
    {
        TWinSocketStream *pStream = new TWinSocketStream(ClientSocket,
5000);
        try
        {
            while( (!Terminated) && (ClientSocket->Connected) )
            {
                if( pStream->WaitForData(1000) )
                {
                    int iLength = 0;
                    if( Read(pStream, &iLength, sizeof(int)) )
                    {
                        FBuffer.SetLength(iLength);
                        if( Read(pStream, FBuffer.c_str(), iLength) )
                        {
                            Synchronize(ReportBuffer);

                            // Process data here...

                            if( Write(pStream, FBuffer.c_str(),
FBuffer.Length()) )
                                continue;
                    }

                    ClientSocket->Close();
               }
            }
        }
        __finally
        {
            delete pStream;
        }
    }

    void __fastcall TMyServerThread::ReportBuffer()
    {
        Form1->Memo1->Lines->Add("(Client) " + FBuffer);
    }


    --- Client ---

    // things get a bit trickier here since you are using a non-blocking
socket.
    // you should be buffering the incoming data until you know that you
have
    // received full packets, and only process full packets.  You are NOT
    // guaranteed to have a full packet available every time the OnRead
event
    // is triggered.

    class TMemoryBuffer : public TMemoryStream
    {
    public:
        __fastcall TMemoryBuffer() : TMemoryStream() {}

        int Available() { return (Size - Position); }

        void* Expand(int Count)
        {
            int Old = Size;
            Size = (Old + Count);

            return ( ((LPBYTE) Memory) + Old );
        }

        void Remove(int Count)
        {
            int iAvailable = Available();
            if( Count > iAvailable )
                Count = iAvailable;

            int NewSize = (Size - Count);
            if( NewSize != Size )
            {
                if( NewSize < Size )
                {
                    void *ptr = ( ((LPBYTE) Memory) + Position );
                    ::MoveMemory(Memory, ptr, NewSize);
                    Size = NewSize;
                }
                else
                    Clear();

                Position = 0;
            }
        }
    };


    TMemoryBuffer *InBuffer = NULL;
    TMemoryBuffre *OutBuffer = NULL;

    __fastcall TForm2::TForm2(TComponent *Owner)
        : TForm(Owner)
    {
        InBuffer = new TMemoryBuffer;
        OutBuffer = new TMemoryBuffer;
    }

    __fastcall TForm2::~TForm2()
    {
        delete InBuffer;
        delete OutBuffer;
    }

    void __fastcall TForm2::ClientSocket1Connect(TObject *Sender,
TCustomWinSocket *Socket)
    {
        Memo1->Lines->Add("Connect Event Handler");
        Memo1->Lines->Add("..Port Active");
        InBuffer->Clear();
        OutBuffer->Clear();
        Write(ClientSocket1->Socket, "This text is passed to and from", 31);
    }

    void __fastcall TForm2::ClientSocket1Disconnect(TObject *Sender,
TCustomWinSocket *Socket)
    {
        Memo1->Lines->Add("Disconnect Event Handler");
        Memo1->Lines->Add("..Port not Active");
        InBuffer->Clear();
        OutBuffer->Clear();
    }

    void __fastcall TForm2::ClientSocket1Error(TObject* Sender,
TCustomWinSocket* Socket, TErrorEvent ErrorEvent, int &ErrorCode)
    {
        Memo1->Lines->Add("Client Error: " + AnsiString(ErrorCode) );
        Socket->Close();
        ErrorCode = 0;
    }

    void __fastcall TForm2::ClientSocket1Read(TObject *Sender,
TCustomWinSocket *Socket)
    {
        Memo1->Lines->Add("Read Event Handler");

        int Bytes = Socket->ReceiveLength();
        if( Bytes > 0 )
        {
            char *ptr = (char*) InBuffer->Expand(Bytes);
            Socket->ReceiveBuf(ptr, Bytes);

            while( InBuffer->Available() >= sizeof(int) )
            {
                int Length = 0;
                InBuffer->Read(&Length, sizeof(int));

                if( InBuffer->Available() < Length )
                {
                    InBuffer->Seek(-sizeof(int), soFromCurrent);
                    break;
                }

                ptr = ( ((char*) InBuffer->Memory) + InBuffer->Position );
                InBuffer->Seek(Length, soFromCurrent);

                Memo1->Lines->Add("(Server) " + AnsiString(ptr, Length));
            }

            InBuffer->Remove(InBuffer->Position);
    }

    void __fastcall TForm2::ClientSocket1Write(TObject *Sender,
TCustomWinSocket *Socket)
    {
        LPBYTE pBuf = (LPBYTE) OutBuffer->Memory
        int BufSize = OutBuffer->Size;

        if( BufSize > 0 )
        {
            do
            {
                int iSent = Socket->SendBuf(pBuf, BufSize);
                if( iSent < 1 )
                {
                    if( iSent == 0 )
                        return; // disconnected

                    // try the data again later
                    break;
                }

                pBuf += iSent;
                BufSize -= iSent;
            }
            while( BufSize > 0 );

            OutBuffer->Remove(OutBuffer->Size - BufSize);
        }
    }

    void __fastcall TForm1::Write(TCustomWinSocket *Socket, void *Buffer,
int BufSize)
    {
        if( OutBuffer->Size > 0 )
        {
            // socket still has pending data from previous
            // write so try this data again later
            OutBuffer->Write(Buffer, BufSize);
        }
        else
        {
            LPBYTE pBuf = (LPBYTE) Buffer;
            while( BufSize > 0 )
            {
                int iSent = Socket->SendBuf(pBuf, BufSize);
                if( iSent < 1 )
                {
                    if( iSent == 0 )
                        return; // disconnected

                    // try the data again later
                    if( Socket->Connected )
                        OutBuffer->Write(pBuf, BufSize);
                    break;
                }

                pBuf += iSent;
                BufSize -= iSent;
            }
        }
    }


Gambit