🚀 go-pugleaf

"Stefano Bonifazi" <stefboombastic@email.it> wrote in message
news:45a7f92c@newsgroups.borland.com...

> Anyway if for some reason the user close the client application
> before it can send the disconnect command, the server
> application crashes

It will not crash.  It will likely raise a suitable exception, which
is normal behavior by design.  But that is NOT a crash.

> even if I wrapped the code inside OnExecute inside a try catch!

You shouldn't be doing that in the first place.  TIdPeerThread has
exception handlers internally to deal with that appropriately.  If you
must use exception handlers, then at least make sure not to disgard
any of Indy's own exception types (those derived from EIdException).
Re-throw them when needed.

> The same happens if I try to close the server application before
> all clients have disconnected!

Again, throwing an exception for an abnormaly closed connection is
perfectly normal behavior for Indy.  It is by design.  Ignore it.  The
server will handle it internaly.

> this is my code inside OnExecute:

Get rid of the exception handler altogether.  You are also not freeing
the Buffer.  You don't need it anyway.  Just use ReadString() instead
of ReadBuffer().  Try this code instead:

    void __fastcall TForm1::IdTCPServer1Execute(TIdPeerThread
*AThread)
    {
        TNodzClientData* pNodz = (TNodzClientData*) AThread->Data;

        AThread->Connection->ReadLn("�");
        int command = AThread->Connection->ReadInteger(false);
        int datasize = AThread->Connection->ReadInteger(false);
        AnsiString Data = AThread->Connection->ReadString(datasize);

        switch( command )
        {
            case cmdStringCommand:
                if( Data=="Disconnect" )
                    AThread->Connection->Disconnect();
                    break;
            ...
        }

        AThread->Connection->ReadLn("�");
    }

> Inside the OnDisconnect I call thread safe code for deleting my
custom data

Why are you maintaining a list of active clients in the first place?
TIdTCPServer already does that for you.  You can access its Threads
property whenever needed.  You can then allocate your TNodzClientData
inside the OnConnect event and free it in the OnDisconnect event
without any synchronizing at all.  Even better, derive TNodzClientData
from TIdPeerThread and set the server's ThreadClass property, and then
you don't have to allocate/free anything at all.

> How should I correct my code in order it can handle Client
application
> closes

Get rid of the exception handler completely.  You are hindering the
server's ability to handle abnormal disconnects.  You are not helping
it.

> without crashes??

What you think is a crash is NOT a real crash.  An exception in
general is NOT a crash.  A real crash would be an unexpected exception
that is not handled, taking the whole server down with it.  That is
not the case here.


Gambit

Hi Mr. Lebeau!
  Thank You for Your reply, and as this is my first post in this New 
Year, I wish U every luck in 2007!!!


> It will not crash.  It will likely raise a suitable exception, which
> is normal behavior by design.  But that is NOT a crash.

I even thought of that at first, but I am used to Exception message to 
pop up when some Exception is thrown, instead I only got the Windows XP 
window message of request to send debug data to Microsof :(



> Get rid of the exception handler altogether.  

Ok I've done that now!
And my code is the following after Ur advices:

__fastcall TNodzServerMainForm::TNodzServerMainForm(TComponent* Owner)
         : TForm(Owner)
{
  NodzIdTCPServer->ThreadClass=__classid(TNodzClientThread);
}

void __fastcall 
TNodzServerMainForm::NodzIdTCPServerExecute(TIdPeerThread *AThread)
{

   TNodzClientThread *pNodzThread = 
reinterpret_cast<TNodzClientThread*>(AThread);
pNodzThread->Connection->ReadLn("�");
int command=pNodzThread->Connection->ReadInteger(false);
int datasize=pNodzThread->Connection->ReadInteger(false);
AnsiString Data = pNodzThread->Connection->ReadString(datasize);
TUpdateData(AThread,command,Data).Synchronize();
}

class TUpdateData : public TIdSync
{
     protected:
         CmdType command;
         AnsiString &Data;
         void __fastcall DoSynchronize()
         {
          TNodzClientThread* 
pNodzThread=reinterpret_cast<TNodzClientThread *>(this);
          switch(command)
          {
           case cmdStringCommand:
            if(Data=="Disconnect")
             pNodzThread->Connection->Disconnect();
            break;
           case cmdApplicationLog:
            pNodzThread->ClientLogTxt += Data;
            pNodzThread->ClientLogNeedUpdate=true;
            break;
           case cmdSCILog:
            pNodzThread->SCILogTxt += Data;
            pNodzThread->SCILogNeedUpdate=true;
            break;
           case cmdSPILog:
            pNodzThread->SPILogTxt += Data;
            pNodzThread->SPILogNeedUpdate=true;
            break;
           case cmdCPULog:
            pNodzThread->CPULogTxt += Data;
            pNodzThread->CPULogNeedUpdate=true;
            break;
          }

         }
     public:
         __fastcall TUpdateData(TIdThread* AThread,int CT,AnsiString &data)
             : TIdSync(AThread),command(CT),Data(data){}
};

class TNodzClientThread: public TIdPeerThread
{
  public:
   TNodzClientForm *NodzClientForm;
   TTabSheet       *pTabSheet;
   TRichEdit       *SPILog;
   TRichEdit       *SCILog;
   TRichEdit       *ClientLog;
   TRichEdit       *CPULog;

   AnsiString       SysCommandTxt;
   AnsiString       SCILogTxt;
   bool             SCILogNeedUpdate;
   AnsiString       SPILogTxt;
   bool             SPILogNeedUpdate;
   AnsiString       CPULogTxt;
   bool             CPULogNeedUpdate;
   AnsiString       ClientLogTxt;
   bool             ClientLogNeedUpdate;

   int              CommandType;
   int              LastUpdateTime;
   bool             NeedUpdate;
    __fastcall TNodzClientThread(bool ACreateSuspended);


};
//---------------------------------------------------------------------------

Well at first I did not use TUpdateData, but I have been getting Access 
violation Exceptions, so I moved the switch block inside the 
Synchronized class, but nothing has changed (( .. before, without 
deriving from TIdPeerThread all worked well (before I used to allocate 
my class data in a thread safe class, inside the OnConnect event handler 
and assigning it to the data member of TIdPeerThread)!! What am I 
mistaken with in the new code? I even tried to pass TNodzClientThread 
pointer to TUpdateData in its constructor, instead of reinterpreting 
this pointer inside TUpdateData method, but nothing changed ((

Thank U!
Stefano

Hi!

> Well at first I did not use TUpdateData, but I have been getting Access 
> violation Exceptions, so I moved the switch block inside the 
> Synchronized class, but nothing has changed (( .. before, without 
> deriving from TIdPeerThread all worked well (before I used to allocate 
> my class data in a thread safe class, inside the OnConnect event handler 
> and assigning it to the data member of TIdPeerThread)!! What am I 
> mistaken with in the new code? I even tried to pass TNodzClientThread 
> pointer to TUpdateData in its constructor, instead of reinterpreting 
> this pointer inside TUpdateData method, but nothing changed ((

I've found that the matter was in assigning the ThreadClass of the indy 
TCPServer and not the one of its ThreadMgr member... so the constructor 
of my thread class were never called.. and so I kept getting access 
violations! Now that is fixed, but inside my custom thread I gotta 
create a new MDI window for each new client that connects.. anyway 
creating those inside the new thread context made them not responding to 
windows messages(( ... is there a workaround maybe synchronizing that 
creation? or should I revert to the non custom threads, and allocating 
custom data inside OnConnect with thread safe code?
Thank U in advance!
Stefano B.

"Stefano Bonifazi" <stefboombastic@email.it> wrote in message
news:45a8c3c4@newsgroups.borland.com...

> I even thought of that at first, but I am used to Exception message
> to pop up when some Exception is thrown

Only if you are running inside the IDE's debugger at the time.
Otherwise no.  There will be no popup at all.

> instead I only got the Windows XP window message of request
> to send debug data to Microsof :(

Then you likely have a third-party debugger running that is
intercepting the exception before the application is.  Under normal
circumstances, that will NOT happen.  TIdTCPServer handles uncaught
exceptions internaly.

> __fastcall TNodzServerMainForm::TNodzServerMainForm(TComponent*
Owner)
>          : TForm(Owner)
> {
>   NodzIdTCPServer->ThreadClass=__classid(TNodzClientThread);
> }

Make sure that you set the ThreadClass property BEFORE the server is
activated.  Do not set the server to Active=true at design-time.

>    TNodzClientThread *pNodzThread =
> reinterpret_cast<TNodzClientThread*>(AThread);

Use static_cast instead of reinterpret_cast.

> TUpdateData(AThread,command,Data).Synchronize();

TUpdateData is a TIdSync-derived class.  You cannot construct it on
the stack like that.  VCL-based objects must be constructed on the
heap only.  That is a requirement of the VCL runtime.  In fact, the
C++ compiler can produce invalid code if you try to use VCL objects on
the stack.

>          void __fastcall DoSynchronize()
>          {
>           TNodzClientThread*
> pNodzThread=reinterpret_cast<TNodzClientThread *>(this);

Why are you doing that?  TUpdateData has no relation to
TNodzClientThread at all.  You can't cast one to the other like that.
I suspect you meant to cast the Thread property instead, correct?

    TNodzClientThread* pNodzThread = static_cast<TNodzClientThread
*>(this->Thread);

> Well at first I did not use TUpdateData, but I have been getting
> Access violation Exceptions

Then you are likely still using the code wrong to begin with.

> so I moved the switch block inside the Synchronized class

Which you should not have done.  There is nothing in that code that
needs to be Synchronize()'d in the first place.  Keep in side of the
OnExecute event handler where it belongs.

> before, without deriving from TIdPeerThread all worked well (before
> I used to allocate my class data in a thread safe class, inside the
> OnConnect event handler and assigning it to the data member of
> TIdPeerThread)!!

You have not shown that code yet.  But the fact that that code was
working but the TIdPeerThread derived class is not working tells me
that you did not set up the new code properly in the first place.


Gambit

"Stefano Bonifazi" <stefboombastic@email.it> wrote in message
news:45abdf15@newsgroups.borland.com...

> I've found that the matter was in assigning the ThreadClass of the
> indy TCPServer and not the one of its ThreadMgr member...

DO NOT assign the ThreadMgr.ThreadClass property directly.
TIdTCPServer handles that internally for you.  Assign only the
TIdTCPServer's own ThreadClass property.  The ThreadMgr.ThreadClass is
assigned automatically while the server is being activated.

> inside my custom thread I gotta create a new MDI window for each
> new client that connects..

You must synchronize that with the main thread.  You cannot create MDI
child windows outside of the main thread context.

> anyway creating those inside the new thread context made them not
> responding to windows messages((

That is only one of several reasons why you can't create them in the
context of worker threads.

> is there a workaround maybe synchronizing that creation?

You already know how to do that.


Gambit

Thankkkkkkkkk UUUUU :))
  I had to keep work stopped for some days coz of those problems :( Well 
I'm a real novice with Indy, and all I learnt about it, it's thanks to 
Ur previous posts )

> DO NOT assign the ThreadMgr.ThreadClass property directly.
> TIdTCPServer handles that internally for you.  Assign only the
> TIdTCPServer's own ThreadClass property.  The ThreadMgr.ThreadClass is
> assigned automatically while the server is being activated.

Well the problem was that I set TIdTCPServer active at designe time, so 
ThreadMgr.ThreadClass was never assigned by itself!

> You must synchronize that with the main thread.  You cannot create MDI
> child windows outside of the main thread context.

Yes I managed to do that with a TidSync descendant class )

I got rid of all the try-catch blocks and used a custom descendant of 
TIdPeerThread, now Indy does all for me: allocating, deallocating .. so 
even if my clients disconnects, or my server shuts down, i get no more 
windows error messages(following to exeptions)!

Thank U very much again!
Stefano B

THANKKK UUU!!! :))
Now my work can go on :))

Remy Lebeau (TeamB) wrote:
> Then you likely have a third-party debugger running that is
> intercepting the exception before the application is.  Under normal
> circumstances, that will NOT happen.  TIdTCPServer handles uncaught
> exceptions internaly.

I dunno if Microsoft Developer Studio .NET did! .. it is so overbearing!

> Make sure that you set the ThreadClass property BEFORE the server is
> activated.  Do not set the server to Active=true at design-time.

Yes exactly there was the problem I set the servrr active at design time :(
> 
> Use static_cast instead of reinterpret_cast.

what does it change?

> TUpdateData is a TIdSync-derived class.  You cannot construct it on
> the stack like that.  VCL-based objects must be constructed on the
> heap only.  That is a requirement of the VCL runtime.  In fact, the
> C++ compiler can produce invalid code if you try to use VCL objects on
> the stack.

well I copied some code snippet.. without thinking of that matter at 
all! Thanks!

> 
>>          void __fastcall DoSynchronize()
>>          {
>>           TNodzClientThread*
>> pNodzThread=reinterpret_cast<TNodzClientThread *>(this);
> 
> Why are you doing that?  TUpdateData has no relation to
> TNodzClientThread at all.  You can't cast one to the other like that.
> I suspect you meant to cast the Thread property instead, correct?
> 
>     TNodzClientThread* pNodzThread = static_cast<TNodzClientThread
> *>(this->Thread);

Yes :)) I copied the wrong code .. sorry U are right!

Thank U very much again!
Best regards!!
Stefano B.