🚀 go-pugleaf

"Rex Chen" <chensc@iii.org.tw> wrote in message
news:4162683A.71B0D6DB@iii.org.tw...

> My job is to implement a win32 service with a COM Interface in it.

Ok so far.

>   char msg[] = "Some Message here....";
>   int   msglen = sizeof(msg);

Use strlen() instead:

    int msglen = strlen(msg)+1;

Or, you could just use an AnsiString or a std::string, then you don't need a
separate length variable at all since string classes always know their
string lengths:

    AnsiString msg= "Some Message here....";
    //...
    ClientSocket1->Socket->SendBuf( msg.c_str(), msg.Length()+1 );

>     // looping until Active is true
>     while( ClientSocket1->Active == false)
> ServiceThread->ProcessRequests(false);

That has the potential of looping indefinately, not to mention that it also
is problematic in that your loop is not responding to the service being shut
down while the loop is still running.

>     // message sending
>     ClientSocket1->Socket->SendBuf( msg, msglen);

You are not checking the return value of SendBuf() to make sure the send
actually succeeded.

>   catch( ... )

You should be catching VCL exceptions explitically instead:

    catch( const Exception & )

> void __fastcall TService1::ClientSocket1Read(TObject *Sender,
TCustomWinSocket
> *Socket)

I would recommend that you not use TClientSocket in non-blocking mode for
what you are doing.  It just makes your code more complicated then it needs
to be.  You are trying to use the TClientSocket inlined within a single
method call, thus you should change your code to use the TClientSocket in
blocking mode instead, using a TWinSocketStream for the reading and writing.
Then you can get rid of the event handlers altogether and all of your code
can reside of Issue() directly.  For example:

    int __fastcall TService1::Issue()
    {
        AnsiString msg[] = "Some Message here....";

        try
        {
            // set ClientSocket1->ClientType to ctBlocking beforehand
            ClientSocket1->Active = true;    // initially it is false

            TWinSocketStream *Stream = new
TWinSocketStream(ClientSocket1->Socket, 5000);
            try
            {
                // message sending
                Stream->Write(msg.c_str(), msg.Length()+1);

                // read the reply here as needed...
            }
            __finally {
                delete Stream;
            }

            ClientSocket1->Active = false;
        }
        catch( const Exception & )
        {
            ClientSocket1->Active = false;
            return -1;
        }

        return 0;
    }

> What you've said is true. I should have my program multithreading

That depends on how exactly you have your COM object set up, and how many
clients are going to be using it at a time.  Are you currently using
apartment-threading for the COM object?  If so, then only 1 client can call
Issue() at a time.  If you need multiple clients to call Issue() at the same
time, then you should use free-threading instead, in which case using the
TClientSocket in blocking mode still makes more sense since all of the code
is inside of Issue() only.  Also, you appear to currently be sharing a
single TClientSocket instance for all of your calls to Issue().  Each
instance of the COM object should have its own TClientSocket instance
instead.


Gambit