🚀 go-pugleaf

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:478e1c34$1@newsgroups.borland.com...

> pStream = new TIdTCPStream(AContext->Connection);
> msg = pStream->Read(imageData->rcv_buffer, 20);

You don't need to use a TIdTCPStream like that.  In fact, that class is 
primarily meant for Indy's internal use.  TIdIOHandler has its own reading 
methods that you should use instead, ie:

    TIdBytes rcv_buffer;
    AContext->Connection->IOHandler->ReadBytes(rcv_buffer, 20);

> discards it, then reads two bytes to see how long the remaining
> transmission is:
> msg = pStream->Read(imageData->rcv_buffer, 2);

Use the IOHandler's ReadSmallInt() method instead:

    calculated_length = AContext->Connection->IOHandler->ReadSmallInt();
    AContext->Connection->IOHandler->ReadBytes(rcv_buffer, 
calculated_length);

> Now, the first two reads come back ok, but then the calculated length
> is different on each connection (and it's the same client, so I assume it
> should be the same), and the last read never comes back (that length is
> usually very large, like 40k+)

Sounds like you are not taking endian into account.  Is the client sending 
the 2-byte length value in host byte order or network byte order?  It makes 
a big difference.  If you don't process it in the correct byte order, you 
will end up with the wrong value, potentially telling Indy to expect more 
data than is actually sent.

By default, ReadSmallInt() (and all of the other numeric reading methods) 
expects the value to arrive in network byte order and converts it to host 
byte order before exiting.  You can set the AConvert parameter to false to 
skip that step if needed.


Gambit 

"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>news:478e1c34$1@newsgroups.borland.com...
>
>> pStream = new TIdTCPStream(AContext->Connection);
>> msg = pStream->Read(imageData->rcv_buffer, 20);
>
>You don't need to use a TIdTCPStream like that.  In fact, that class is 
>primarily meant for Indy's internal use.  TIdIOHandler has its own reading 
>methods that you should use instead, ie:
>
>    TIdBytes rcv_buffer;
>    AContext->Connection->IOHandler->ReadBytes(rcv_buffer, 20);
>
>> discards it, then reads two bytes to see how long the remaining
>> transmission is:
>> msg = pStream->Read(imageData->rcv_buffer, 2);
>
>Use the IOHandler's ReadSmallInt() method instead:
>
>    calculated_length = AContext->Connection->IOHandler->ReadSmallInt();
>    AContext->Connection->IOHandler->ReadBytes(rcv_buffer, 
>calculated_length);
>
>> Now, the first two reads come back ok, but then the calculated length
>> is different on each connection (and it's the same client, so I assume it
>> should be the same), and the last read never comes back (that length is
>> usually very large, like 40k+)
>
>Sounds like you are not taking endian into account.  Is the client sending 
>the 2-byte length value in host byte order or network byte order?  It makes 
>a big difference.  If you don't process it in the correct byte order, you 
>will end up with the wrong value, potentially telling Indy to expect more 
>data than is actually sent.
>
>By default, ReadSmallInt() (and all of the other numeric reading methods) 
>expects the value to arrive in network byte order and converts it to host 
>byte order before exiting.  You can set the AConvert parameter to false to 
>skip that step if needed.
>
>
>Gambit 
>
>

That's great... Thanks!!!

However, I have lots of problems using the components, mostly due to the lack of examples. Is there a piece of C++ code I could look at, that contains examples of the usage? I have trouble writing the code for the low-level image connection. 
The original code was very simple:

while (imageData->msg > 0) {
    	imageData->msg = pStream->Read(imageData->rcv_buffer, 32000);
        fwrite(imageData->rcv_buffer, imageData->msg, 1, imageData->imgFile);
}

but now, I'm trying this:

AContext->Connection->IOHandler->CheckForDataOnSource(0);

do {

if ( !AContext->Connection->IOHandler->InputBufferIsEmpty() ) {

bufferSize = AContext->Connection->IOHandler->InputBuffer->Size;

AContext->Connection->IOHandler->ReadBytes(imageData->rcv_buffer, -1, false);

BytesToRaw(imageData->rcv_buffer, vbuff, imageData->rcv_buffer.Length);

fwrite(vbuff, imageData->msg, 1, imageData->imgFile);

AContext->Connection->IOHandler->CheckForDataOnSource(0);

}

} while ( !AContext->Connection->IOHandler->InputBufferIsEmpty());

and I get no data from the socket.

Thanks for any help!

"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>news:4798a5e1$1@newsgroups.borland.com...
>
>> The original code was very simple:
><snip>
>
>Your original code was reading endlessly, as TIdTCPStream::Read() always 
>returns the same Count value that you pass in, so imageData->msg would 
>always be greater than 0 until an exception occured.  Such logic is easy to 
>reproduce using the code I already gave you earlier:
>
>    while( imageData->msg > 0 )
>    {
>        // assuming rcv_buffer is a TIdBytes.  ReadBytes() will not exit 
>until all 32000
>        // bytes have been received.  The same was true for 
>TIdTCPStream::Read() as well...
>        AContext->Connection->IOHandler->ReadBytes(imageData->rcv_buffer, 
>32000);
>        imageData->msg = 32000;
>        fwrite(&imageData->rcv_buffer[0], imageData->msg, 1, 
>imageData->imgFile);
>    }
>
>> but now, I'm trying this:
><snip>
>> and I get no data from the socket.
>
>Then your client is not sending any data at all, or is more likely sending 
>data after a small delay.  You are exiting your loop if the InputBuffer has 
>no data each time you check it.  The first time you check the InputBuffer, 
>if no data has arrived yet, you exit the loop right away.  At the very 
>least, you should increase your timeout value to give the data a better 
>chance of catching up to how fast your reading code is running.
>
>With that said, the best option is to forget looping at all.  Have the 
>client send the data size before sending the actual data.  Then your server 
>knows exactly how many bytes to expect, and can call ReadBytes() or 
>ReadStream() just once to do all of the work.  In fact, that is what you 
>said in your original posting that you were doing, but you are not doing 
>that anymore in this latest code snippet.  So by introducing InputBuffer 
>checking into your code, which you did not need in the first place, you have 
>made the code operate worse, not better.
>
>
>Gambit 
>
>

Thanks for your answers!!!
I wish this was that simple :-)
Unfortunately, the "client" in this case is a Mainframe imaging system (from Big Blue, which does not make it easier).
The whole communication sequence was "reverse-engineered" from the network traces a few years ago. Now... we get two kinds of data on the socket: image (binary, where we have no control how it's sent by the Mainframe), and data (which we control, and the communication is done exactly how your suggested :-)).
When getting the binary data, first I get a piece that contains a binary code that says "image", and then I have to read that image (not knowing how big it is).
Thanks for your help!!! If you have any points for me, I greatly appreciate them!!!

Peter

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:4798a5e1$1@newsgroups.borland.com...

> The original code was very simple:
<snip>

Your original code was reading endlessly, as TIdTCPStream::Read() always 
returns the same Count value that you pass in, so imageData->msg would 
always be greater than 0 until an exception occured.  Such logic is easy to 
reproduce using the code I already gave you earlier:

    while( imageData->msg > 0 )
    {
        // assuming rcv_buffer is a TIdBytes.  ReadBytes() will not exit 
until all 32000
        // bytes have been received.  The same was true for 
TIdTCPStream::Read() as well...
        AContext->Connection->IOHandler->ReadBytes(imageData->rcv_buffer, 
32000);
        imageData->msg = 32000;
        fwrite(&imageData->rcv_buffer[0], imageData->msg, 1, 
imageData->imgFile);
    }

> but now, I'm trying this:
<snip>
> and I get no data from the socket.

Then your client is not sending any data at all, or is more likely sending 
data after a small delay.  You are exiting your loop if the InputBuffer has 
no data each time you check it.  The first time you check the InputBuffer, 
if no data has arrived yet, you exit the loop right away.  At the very 
least, you should increase your timeout value to give the data a better 
chance of catching up to how fast your reading code is running.

With that said, the best option is to forget looping at all.  Have the 
client send the data size before sending the actual data.  Then your server 
knows exactly how many bytes to expect, and can call ReadBytes() or 
ReadStream() just once to do all of the work.  In fact, that is what you 
said in your original posting that you were doing, but you are not doing 
that anymore in this latest code snippet.  So by introducing InputBuffer 
checking into your code, which you did not need in the first place, you have 
made the code operate worse, not better.


Gambit 

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:4798db8f$1@newsgroups.borland.com...

> When getting the binary data, first I get a piece that contains a
> binary code that says "image", and then I have to read that image
> (not knowing how big it is).

In that situation, there are only three reliable ways to detect when 
arbitrary binary data has finished being sent:

1) the server sends the data length beforehand (which it sounds like from 
yor descriptions that it is not)

2) the server sends a unique terminator byte/sequence to mark the end of the 
image data.  You would have to look for that terminator while reading so you 
don't accidentally receive too much data if another packet follows the image 
data right away.

3) the server disconnects the connection at the end of the file.

If the same socket connection is being used for both types of data 
transmissions, where your custom communication packets can arrive after the 
image is finished (without having to reconnect to the server), then #3 is 
not a viable option.  It has to be doing either #1 or #2 to ensure the 
integrity of the communication.

If the server is not doing any of those options, then the only other thing I 
can think of is to see if the image data itself has its own header that 
specifies the data size, assuming the same image format is being used each 
time.


Gambit 

"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>news:4798db8f$1@newsgroups.borland.com...
>
>> When getting the binary data, first I get a piece that contains a
>> binary code that says "image", and then I have to read that image
>> (not knowing how big it is).
>
>In that situation, there are only three reliable ways to detect when 
>arbitrary binary data has finished being sent:
>
>1) the server sends the data length beforehand (which it sounds like from 
>yor descriptions that it is not)
>
>2) the server sends a unique terminator byte/sequence to mark the end of the 
>image data.  You would have to look for that terminator while reading so you 
>don't accidentally receive too much data if another packet follows the image 
>data right away.
>
>3) the server disconnects the connection at the end of the file.
>
>If the same socket connection is being used for both types of data 
>transmissions, where your custom communication packets can arrive after the 
>image is finished (without having to reconnect to the server), then #3 is 
>not a viable option.  It has to be doing either #1 or #2 to ensure the 
>integrity of the communication.
>
>If the server is not doing any of those options, then the only other thing I 
>can think of is to see if the image data itself has its own header that 
>specifies the data size, assuming the same image format is being used each 
>time.
>
>
>Gambit 
>
>

Thanks!!!
Great points. I got most of it to work :-)
The only major issue for me that's remaining is the multithreading. The main server (TIdTCPServer) component is part of the TService application, and in it's main Execute thread, it is started like this:

iln_rcv_socket->Active = true;
while (!Terminated) {
  ServiceThread->ProcessRequests(true);
}
iln_rcv_socket->Active = false;

This seems to be working ok, however, it appears that I can only get the first connection to come in ok, and the rest after that fail.

I also added TIdAntiFreeze component to the TService application, but I'm not sure if all the properties between TIdTCPServer and TIdAntiFreeze are set ok.

For TIdTCPServer I have:
ListenQueue: 200 (max for Win 2003 Server as I understand)
ReuseSocket: true
TerminateWait: 65535 (???)

For TIdAntiFreeze:
ApplicationHasPriority: true (???)
IdleTimeout: 250
OnlyWhenIdle: true

Also, I assume that all storage in OnExecute handler of the TIdTCPServer is local to the each thread.

"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>news:479e2c75$1@newsgroups.borland.com...
>
>> The main server (TIdTCPServer) component is part of the TService
>> application, and in it's main Execute thread, it is started like this:
><snip>
>
>Get rid of that OnExecute handler altogether.  TService already runs such a 
>loop internally for you when no OnExecute handler is assigned.  Instead, 
>activate the server in the OnStart event, and deactivate it in the service's 
>OnStop/Shutdown events, ie:
>
>    void __fastcall TMyService::ServiceStart(TObject *Sender, bool &Started)
>    {
>        iln_rcv_socket->Active = true;
>        Started = true;
>    }
>
>    void __fastcall TMyService::ServiceStop(TObject *Sender, bool &Stopped)
>    {
>        ServiceShutdown(Sender);
>        Stopped = true;
>    }
>
>    void __fastcall TMyService::ServiceShutdown(TObject *Sender)
>    {
>        iln_rcv_socket->Active = false;
>    }
>
>
>> This seems to be working ok, however, it appears that I can only
>> get the first connection to come in ok, and the rest after that fail.
>
>TIdTCPServer works fine in a service environment.  If you are having 
>problems with connections, then your TIdTCPServer event handler code is 
>likely buggy to begin with.
>
>> I also added TIdAntiFreeze component to the TService application
>
>TIdAntiFreeze is useless in a service.  It operates only in the context of 
>the main thread, and acts as a no-op in worker threads.  TService does not 
>run in the main thread.
>
>> Also, I assume that all storage in OnExecute handler of the TIdTCPServer
>> is local to the each thread.
>
>What "storage" are you referring to exactly?
>
>
>Gambit 
>
>

Thanks!!! This is great! It's the lack of documentation that's driving me nuts :-) but I'm chugging along. I have a few other "variables" in the picture (like a third-party imaging libs), so anything is possible as far as "bugs" go.
I have lots of info to go on for now. Thanks again!

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:479e2c75$1@newsgroups.borland.com...

> The main server (TIdTCPServer) component is part of the TService
> application, and in it's main Execute thread, it is started like this:
<snip>

Get rid of that OnExecute handler altogether.  TService already runs such a 
loop internally for you when no OnExecute handler is assigned.  Instead, 
activate the server in the OnStart event, and deactivate it in the service's 
OnStop/Shutdown events, ie:

    void __fastcall TMyService::ServiceStart(TObject *Sender, bool &Started)
    {
        iln_rcv_socket->Active = true;
        Started = true;
    }

    void __fastcall TMyService::ServiceStop(TObject *Sender, bool &Stopped)
    {
        ServiceShutdown(Sender);
        Stopped = true;
    }

    void __fastcall TMyService::ServiceShutdown(TObject *Sender)
    {
        iln_rcv_socket->Active = false;
    }


> This seems to be working ok, however, it appears that I can only
> get the first connection to come in ok, and the rest after that fail.

TIdTCPServer works fine in a service environment.  If you are having 
problems with connections, then your TIdTCPServer event handler code is 
likely buggy to begin with.

> I also added TIdAntiFreeze component to the TService application

TIdAntiFreeze is useless in a service.  It operates only in the context of 
the main thread, and acts as a no-op in worker threads.  TService does not 
run in the main thread.

> Also, I assume that all storage in OnExecute handler of the TIdTCPServer
> is local to the each thread.

What "storage" are you referring to exactly?


Gambit 

"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>news:479e2c75$1@newsgroups.borland.com...
>
>> The main server (TIdTCPServer) component is part of the TService
>> application, and in it's main Execute thread, it is started like this:
><snip>
>
>Get rid of that OnExecute handler altogether.  TService already runs such a 
>loop internally for you when no OnExecute handler is assigned.  Instead, 
>activate the server in the OnStart event, and deactivate it in the service's 
>OnStop/Shutdown events, ie:
>
>    void __fastcall TMyService::ServiceStart(TObject *Sender, bool &Started)
>    {
>        iln_rcv_socket->Active = true;
>        Started = true;
>    }
>
>    void __fastcall TMyService::ServiceStop(TObject *Sender, bool &Stopped)
>    {
>        ServiceShutdown(Sender);
>        Stopped = true;
>    }
>
>    void __fastcall TMyService::ServiceShutdown(TObject *Sender)
>    {
>        iln_rcv_socket->Active = false;
>    }
>
>
>> This seems to be working ok, however, it appears that I can only
>> get the first connection to come in ok, and the rest after that fail.
>
>TIdTCPServer works fine in a service environment.  If you are having 
>problems with connections, then your TIdTCPServer event handler code is 
>likely buggy to begin with.
>
>> I also added TIdAntiFreeze component to the TService application
>
>TIdAntiFreeze is useless in a service.  It operates only in the context of 
>the main thread, and acts as a no-op in worker threads.  TService does not 
>run in the main thread.
>
>> Also, I assume that all storage in OnExecute handler of the TIdTCPServer
>> is local to the each thread.
>
>What "storage" are you referring to exactly?
>
>
>Gambit 
>
>

Just two quick questions:
1. By "storage", I mean, variables declared in the OnExecute handler, which is a method of the TService class (when the TIdTCPServer is added to the service form, that's how it gets declared). I assume all variables there are "thread-safe".
2. I see some posts about "broken" Indy libraries in Borland C++ Builder 2007.
My version says: 10.0.0.17. Is it ok? Should I upgrade?

Thanks again!!!

"Peter" <piotrtrzeciak@yahoo.com> wrote:
>
>"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>>
>>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>>news:479e2c75$1@newsgroups.borland.com...
>>
>>> The main server (TIdTCPServer) component is part of the TService
>>> application, and in it's main Execute thread, it is started like this:
>><snip>
>>
>>Get rid of that OnExecute handler altogether.  TService already runs such a 
>>loop internally for you when no OnExecute handler is assigned.  Instead, 
>>activate the server in the OnStart event, and deactivate it in the service's 
>>OnStop/Shutdown events, ie:
>>
>>    void __fastcall TMyService::ServiceStart(TObject *Sender, bool &Started)
>>    {
>>        iln_rcv_socket->Active = true;
>>        Started = true;
>>    }
>>
>>    void __fastcall TMyService::ServiceStop(TObject *Sender, bool &Stopped)
>>    {
>>        ServiceShutdown(Sender);
>>        Stopped = true;
>>    }
>>
>>    void __fastcall TMyService::ServiceShutdown(TObject *Sender)
>>    {
>>        iln_rcv_socket->Active = false;
>>    }
>>
>>
>>> This seems to be working ok, however, it appears that I can only
>>> get the first connection to come in ok, and the rest after that fail.
>>
>>TIdTCPServer works fine in a service environment.  If you are having 
>>problems with connections, then your TIdTCPServer event handler code is 
>>likely buggy to begin with.
>>
>>> I also added TIdAntiFreeze component to the TService application
>>
>>TIdAntiFreeze is useless in a service.  It operates only in the context of 
>>the main thread, and acts as a no-op in worker threads.  TService does not 
>>run in the main thread.
>>
>>> Also, I assume that all storage in OnExecute handler of the TIdTCPServer
>>> is local to the each thread.
>>
>>What "storage" are you referring to exactly?
>>
>>
>>Gambit 
>>
>>
>
>Just two quick questions:
>1. By "storage", I mean, variables declared in the OnExecute handler, which is a method of the TService class (when the TIdTCPServer is added to the service form, that's how it gets declared). I assume all variables there are "thread-safe".
>2. I see some posts about "broken" Indy libraries in Borland C++ Builder 2007.
>My version says: 10.0.0.17. Is it ok? Should I upgrade?
>
>Thanks again!!!

An additional observation:
It appears that my code "crashes" when more then one connection is being established from the client and the IOHandler's InputBuffer gets stepped on. Is this buffer a separate one for each thread? Is there any property that's controlling it? Thanks!

"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>news:479f51f8$1@newsgroups.borland.com...
>
>> It appears that my code "crashes" when more then one connection is
>> being established from the client and the IOHandler's InputBuffer gets
>> stepped on.
>
>Then you are using it wrong.  Please show your actual code.
>
>> Is this buffer a separate one for each thread?
>
>For each connection, yes.
>
>
>Gambit 
>
>

Here is the code:

void __fastcall Tlens_srvc::iln_rcv_socketExecute(TIdContext *AContext)
{

	ImageData		*imageData;


	imageData = (ImageData *)malloc(sizeof(ImageData));

	imageData->INITC = RawToBytes("\x00\xA1", 2);
	imageData->DISPLAY = RawToBytes("\x00\xA9", 2);
	imageData->EVN_MESSAGE = RawToBytes("\x00\xA7", 2);

	AContext->Connection->IOHandler->ReadBytes(imageData->rcv_buffer, 20, false);

	imageData->requested_length = 2;
	imageData->msg = AContext->Connection->IOHandler->ReadSmallInt(true);

	if (imageData->msg > 0) {
		imageData->length = imageData->msg;
	}
	else {
		// error
		free(imageData);
		return;
	}
	imageData->requested_length = imageData->length - 2;

	AContext->Connection->IOHandler->ReadBytes(imageData->rcv_buffer, imageData->requested_length, false);

	// This return is here only for testing to exit without reading more data.
    //return;

	//
	// Figure out what command we are receiving
	//
	if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length > 4) ) {
		// INITC as OINT, no reads, just write a response to the socket
		processOint(AContext, imageData);
	}
	else {
		if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length <=4) ) {
			// INITC as INIT, no reads, just write a response to the socket
			processInit(AContext, imageData);
		}
		else {
			if ( (imageData->rcv_buffer[0] == imageData->DISPLAY[0]) && (imageData->rcv_buffer[1] == imageData->DISPLAY[1]) ) {
				// DISPLAY... this is when we get an image and read more data from the socket
				imageData->received_length = imageData->length;
				receiveImage(AContext, imageData);
			}
			else {
				if ( (imageData->rcv_buffer[0] == imageData->EVN_MESSAGE[0]) && (imageData->rcv_buffer[1] == imageData->EVN_MESSAGE[1]) ) {
					// EVN_MESSAGE. Error messages will come down from the host here, no more data from socket, just interpret message
					getEvnMessage(AContext, imageData);
				}
				else {
					// Other command... no processing

				}
			}
		}
	}

	//
	// Clean up
	//
	free(imageData);

}

Now, I put in a return statement in the middle, just to see if I can get more than one connection, but it still fails.
For any "command" other than DISPLAY, it works fine, because they only come as single connections. If I get a series of DISPLAY commands, the first one usually goes ok (depends on the timing), but the second client connection kills the service.
The only other thing I can think of, is that I'm adding the TIdTCPServer component to a wrong place (TService), and that's why it gets "clobbered".
I have to admit, it took me less to write this whole app from scratch the first time around, than to get Indy working :-( (but I think it's just a lack of docs and experience).
Thanks again.

"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>news:479f4835$1@newsgroups.borland.com...
>
>> By "storage", I mean, variables declared in the OnExecute handler,
>> which is a method of the TService class (when the TIdTCPServer is
>> added to the service form, that's how it gets declared). I assume all
>> variables there are "thread-safe".
>
>Like any other function, variables declared inside an event handler are 
>local to that handler only.
>
>> I see some posts about "broken" Indy libraries in Borland C++ Builder
>> 2007. My version says: 10.0.0.17. Is it ok? Should I upgrade?
>
>10.0.17 is a very old version.  The current version is 10.2.3.
>
>
>Gambit 
>
>

Hmmm... that's what comes with BC++ Builder 2007. Does CodeGear have an updated install, or I have to get the latest dev version from Indy? Is there a good writeup of the upgrade steps?

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:479f4835$1@newsgroups.borland.com...

> By "storage", I mean, variables declared in the OnExecute handler,
> which is a method of the TService class (when the TIdTCPServer is
> added to the service form, that's how it gets declared). I assume all
> variables there are "thread-safe".

Like any other function, variables declared inside an event handler are 
local to that handler only.

> I see some posts about "broken" Indy libraries in Borland C++ Builder
> 2007. My version says: 10.0.0.17. Is it ok? Should I upgrade?

10.0.17 is a very old version.  The current version is 10.2.3.


Gambit 

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:479f51f8$1@newsgroups.borland.com...

> It appears that my code "crashes" when more then one connection is
> being established from the client and the IOHandler's InputBuffer gets
> stepped on.

Then you are using it wrong.  Please show your actual code.

> Is this buffer a separate one for each thread?

For each connection, yes.


Gambit 

"Peter" <piotrtrzeciak@yahoo.com> wrote:
>
>"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>>
>>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>>news:479f4835$1@newsgroups.borland.com...
>>
>>> By "storage", I mean, variables declared in the OnExecute handler,
>>> which is a method of the TService class (when the TIdTCPServer is
>>> added to the service form, that's how it gets declared). I assume all
>>> variables there are "thread-safe".
>>
>>Like any other function, variables declared inside an event handler are 
>>local to that handler only.
>>
>>> I see some posts about "broken" Indy libraries in Borland C++ Builder
>>> 2007. My version says: 10.0.0.17. Is it ok? Should I upgrade?
>>
>>10.0.17 is a very old version.  The current version is 10.2.3.
>>
>>
>>Gambit 
>>
>>
>
>Hmmm... that's what comes with BC++ Builder 2007. Does CodeGear have an updated install, or I have to get the latest dev version from Indy? Is there a good writeup of the upgrade steps?
>

Please, keep in mind that I have BC++ Builder 2007 Pro (and not RAD), so I don't seem to be able to recompile any source.

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:479f7070$1@newsgroups.borland.com...

> Here is the code:

Other than error checking that is missing, I see nothing else in that code 
that would cause the kind of problems you are describing.

> ImageData *imageData;

What does the declaration of ImageData actually look like?

> imageData = (ImageData *)malloc(sizeof(ImageData));

malloc() can return NULL, which you are not checking for.  In fact, since 
you always free() the ImageData instance before exiting, there is no point 
in dynamically allocating it at all.  You could declare it on the stack 
instead and let the compiler manage it for you.

> imageData->INITC = RawToBytes("\x00\xA1", 2);
> imageData->DISPLAY = RawToBytes("\x00\xA9", 2);
> imageData->EVN_MESSAGE = RawToBytes("\x00\xA7", 2);

Why are you treating the command IDs as 2-byte buffers instead of 2-byte 
integers?  That would make your processing much cleaner.  You could get rid 
of the INITC, DISPLAY, and EVN_MESSAGE members altogether and just use 
compile-time constants instead.

> imageData->requested_length = imageData->length - 2;

You are not checking to make sure that requested_length is greater than 0 
before reading and processing the buffer.

> // This return is here only for testing to exit without reading more data.

That would be a memory leak since you don't free the ImageData instance 
before returning.

> if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
> (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length > 
> 4) ) {
> // INITC as OINT, no reads, just write a response to the socket
> processOint(AContext, imageData);
> }
> else {
> if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
> (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length 
> <=4) ) {
> // INITC as INIT, no reads, just write a response to the socket
> processInit(AContext, imageData);
> }

Those if statements are a bit redundant.  You could clean that up as follows 
(more so if you get rid of the INITC buffer and use integers instead):

    if( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
(imageData->rcv_buffer[1] == imageData->INITC[1]) )
    {
        if( imageData->length > 4 )
        {
            // INITC as OINT, no reads, just write a response to the socket
            processOint(AContext, imageData);
        }
        else
        {
            // INITC as INIT, no reads, just write a response to the socket
            processInit(AContext, imageData);
        }
    }

> Now, I put in a return statement in the middle, just to see if I can get
> more than one connection, but it still fails.

You need to be more specific about what "it fails" actually means.

> For any "command" other than DISPLAY, it works fine, because they
> only come as single connections. If I get a series of DISPLAY commands,
> the first one usually goes ok (depends on the timing), but the second 
> client
> connection kills the service.

Then most likely your processing and responding code, which you have no 
shown yet, is not managing the data/connection properly and corrupting the 
service in some way.

> The only other thing I can think of, is that I'm adding the TIdTCPServer
> component to a wrong place (TService), and that's why it gets "clobbered".

Putting a TIdTCPServer inside the TService works perfectly fine when used 
properly.


Gambit 

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:479f7225$1@newsgroups.borland.com...

> Hmmm... that's what comes with BC++ Builder 2007.

Yes, I realize that, and I've already commented several times before about 
why that happened.

> Does CodeGear have an updated install

No.

> I have to get the latest dev version from Indy?

Yes.

> Is there a good writeup of the upgrade steps?

Not really.


Gambit 

"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>news:479f7225$1@newsgroups.borland.com...
>
>> Hmmm... that's what comes with BC++ Builder 2007.
>
>Yes, I realize that, and I've already commented several times before about 
>why that happened.
>
>> Does CodeGear have an updated install
>
>No.
>
>> I have to get the latest dev version from Indy?
>
>Yes.
>
>> Is there a good writeup of the upgrade steps?
>
>Not really.
>
>
>Gambit 
>
>

Not very encouraging :-) 
I'll try to work with whatever is there, hoping for an update from CodeGear.
I got the newest version of Indy, managed to recompile it (System, Core, and Protocols), but I can't get it into BC++, because it claims the packages are not "design time"?
Just venting I guess...

"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>news:479f7070$1@newsgroups.borland.com...
>
>> Here is the code:
>
>Other than error checking that is missing, I see nothing else in that code 
>that would cause the kind of problems you are describing.
>
>> ImageData *imageData;
>
>What does the declaration of ImageData actually look like?
>
>> imageData = (ImageData *)malloc(sizeof(ImageData));
>
>malloc() can return NULL, which you are not checking for.  In fact, since 
>you always free() the ImageData instance before exiting, there is no point 
>in dynamically allocating it at all.  You could declare it on the stack 
>instead and let the compiler manage it for you.
>
>> imageData->INITC = RawToBytes("\x00\xA1", 2);
>> imageData->DISPLAY = RawToBytes("\x00\xA9", 2);
>> imageData->EVN_MESSAGE = RawToBytes("\x00\xA7", 2);
>
>Why are you treating the command IDs as 2-byte buffers instead of 2-byte 
>integers?  That would make your processing much cleaner.  You could get rid 
>of the INITC, DISPLAY, and EVN_MESSAGE members altogether and just use 
>compile-time constants instead.
>
>> imageData->requested_length = imageData->length - 2;
>
>You are not checking to make sure that requested_length is greater than 0 
>before reading and processing the buffer.
>
>> // This return is here only for testing to exit without reading more data.
>
>That would be a memory leak since you don't free the ImageData instance 
>before returning.
>
>> if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
>> (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length > 
>> 4) ) {
>> // INITC as OINT, no reads, just write a response to the socket
>> processOint(AContext, imageData);
>> }
>> else {
>> if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
>> (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length 
>> <=4) ) {
>> // INITC as INIT, no reads, just write a response to the socket
>> processInit(AContext, imageData);
>> }
>
>Those if statements are a bit redundant.  You could clean that up as follows 
>(more so if you get rid of the INITC buffer and use integers instead):
>
>    if( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
>(imageData->rcv_buffer[1] == imageData->INITC[1]) )
>    {
>        if( imageData->length > 4 )
>        {
>            // INITC as OINT, no reads, just write a response to the socket
>            processOint(AContext, imageData);
>        }
>        else
>        {
>            // INITC as INIT, no reads, just write a response to the socket
>            processInit(AContext, imageData);
>        }
>    }
>
>> Now, I put in a return statement in the middle, just to see if I can get
>> more than one connection, but it still fails.
>
>You need to be more specific about what "it fails" actually means.
>
>> For any "command" other than DISPLAY, it works fine, because they
>> only come as single connections. If I get a series of DISPLAY commands,
>> the first one usually goes ok (depends on the timing), but the second 
>> client
>> connection kills the service.
>
>Then most likely your processing and responding code, which you have no 
>shown yet, is not managing the data/connection properly and corrupting the 
>service in some way.
>
>> The only other thing I can think of, is that I'm adding the TIdTCPServer
>> component to a wrong place (TService), and that's why it gets "clobbered".
>
>Putting a TIdTCPServer inside the TService works perfectly fine when used 
>properly.
>
>
>Gambit 
>
>

Well... I stripped all the code that's not relevant (there is error checking and other things there :-)). And certain code "ugliness" comes from the initial version that was written based on network traces.
The only thing I can improve is to get rid of memory management (malloc) and let it do it automatically, but to be honest, I do not think that's the fix here.
I'll keep looking.
Thanks again!

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:47a09758$1@newsgroups.borland.com...

> I'll try to work with whatever is there, hoping for an update from 
> CodeGear.

It is not CodeGear's responsibility to release updates for Indy.  They just 
bundle whatever is currently available and working at the time a new IDE 
version is released.  We're currently working on a new version of Indy for 
inclusion into the Tiburon IDE later this year.  In the meantime, dilay 
snapshots are all that are available as updates for existing IDEs.

> I got the newest version of Indy, managed to recompile it (System, Core,
> and Protocols), but I can't get it into BC++, because it claims the 
> packages
> are not "design time"?

The IndySystem..., IndyCore..., and IndyProtocols... packages are run-time 
packages, not design-time packages.  They cannot be installed into the IDE. 
Indy has separate dclCore... and dclProtocols... design-time packages for 
that.


Gambit 

Peter wrote:

>>> Is there a good writeup of the upgrade steps?

Check Google for this thread. It might help.

Newsgroups: borland.public.cppbuilder.internet.socket
Subject: Update Indy10 in C++Builder 2007
Date: Sun, 16 Dec 2007 18:10:35 -0500

"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>news:479f7070$1@newsgroups.borland.com...
>
>> Here is the code:
>
>Other than error checking that is missing, I see nothing else in that code 
>that would cause the kind of problems you are describing.
>
>> ImageData *imageData;
>
>What does the declaration of ImageData actually look like?
>
>> imageData = (ImageData *)malloc(sizeof(ImageData));
>
>malloc() can return NULL, which you are not checking for.  In fact, since 
>you always free() the ImageData instance before exiting, there is no point 
>in dynamically allocating it at all.  You could declare it on the stack 
>instead and let the compiler manage it for you.
>
>> imageData->INITC = RawToBytes("\x00\xA1", 2);
>> imageData->DISPLAY = RawToBytes("\x00\xA9", 2);
>> imageData->EVN_MESSAGE = RawToBytes("\x00\xA7", 2);
>
>Why are you treating the command IDs as 2-byte buffers instead of 2-byte 
>integers?  That would make your processing much cleaner.  You could get rid 
>of the INITC, DISPLAY, and EVN_MESSAGE members altogether and just use 
>compile-time constants instead.
>
>> imageData->requested_length = imageData->length - 2;
>
>You are not checking to make sure that requested_length is greater than 0 
>before reading and processing the buffer.
>
>> // This return is here only for testing to exit without reading more data.
>
>That would be a memory leak since you don't free the ImageData instance 
>before returning.
>
>> if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
>> (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length > 
>> 4) ) {
>> // INITC as OINT, no reads, just write a response to the socket
>> processOint(AContext, imageData);
>> }
>> else {
>> if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
>> (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length 
>> <=4) ) {
>> // INITC as INIT, no reads, just write a response to the socket
>> processInit(AContext, imageData);
>> }
>
>Those if statements are a bit redundant.  You could clean that up as follows 
>(more so if you get rid of the INITC buffer and use integers instead):
>
>    if( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
>(imageData->rcv_buffer[1] == imageData->INITC[1]) )
>    {
>        if( imageData->length > 4 )
>        {
>            // INITC as OINT, no reads, just write a response to the socket
>            processOint(AContext, imageData);
>        }
>        else
>        {
>            // INITC as INIT, no reads, just write a response to the socket
>            processInit(AContext, imageData);
>        }
>    }
>
>> Now, I put in a return statement in the middle, just to see if I can get
>> more than one connection, but it still fails.
>
>You need to be more specific about what "it fails" actually means.
>
>> For any "command" other than DISPLAY, it works fine, because they
>> only come as single connections. If I get a series of DISPLAY commands,
>> the first one usually goes ok (depends on the timing), but the second 
>> client
>> connection kills the service.
>
>Then most likely your processing and responding code, which you have no 
>shown yet, is not managing the data/connection properly and corrupting the 
>service in some way.
>
>> The only other thing I can think of, is that I'm adding the TIdTCPServer
>> component to a wrong place (TService), and that's why it gets "clobbered".
>
>Putting a TIdTCPServer inside the TService works perfectly fine when used 
>properly.
>
>
>Gambit 
>
>

So... here is an interesting outcome. I execute the service, attach it to CodeGear debugger, send some data, and it works, yet the same thing refuses to "service" more than one socket connection, when executed normally (without the debugger). So, my next question is:
What, if any, maintenance must be performed on the instance of the IIdTCPServer? Context? Connection? IOHandler? What kind of open/close statement must be performed, and when? Are there any special properties that must be set, in order to run the code concurrently?
Any input, as always, greatly appreciated!!!

"Peter" <piotrtrzeciak@yahoo.com> wrote:
>
>"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>>
>>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>>news:479f7070$1@newsgroups.borland.com...
>>
>>> Here is the code:
>>
>>Other than error checking that is missing, I see nothing else in that code 
>>that would cause the kind of problems you are describing.
>>
>>> ImageData *imageData;
>>
>>What does the declaration of ImageData actually look like?
>>
>>> imageData = (ImageData *)malloc(sizeof(ImageData));
>>
>>malloc() can return NULL, which you are not checking for.  In fact, since 
>>you always free() the ImageData instance before exiting, there is no point 
>>in dynamically allocating it at all.  You could declare it on the stack 
>>instead and let the compiler manage it for you.
>>
>>> imageData->INITC = RawToBytes("\x00\xA1", 2);
>>> imageData->DISPLAY = RawToBytes("\x00\xA9", 2);
>>> imageData->EVN_MESSAGE = RawToBytes("\x00\xA7", 2);
>>
>>Why are you treating the command IDs as 2-byte buffers instead of 2-byte 
>>integers?  That would make your processing much cleaner.  You could get rid 
>>of the INITC, DISPLAY, and EVN_MESSAGE members altogether and just use 
>>compile-time constants instead.
>>
>>> imageData->requested_length = imageData->length - 2;
>>
>>You are not checking to make sure that requested_length is greater than 0 
>>before reading and processing the buffer.
>>
>>> // This return is here only for testing to exit without reading more data.
>>
>>That would be a memory leak since you don't free the ImageData instance 
>>before returning.
>>
>>> if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
>>> (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length > 
>>> 4) ) {
>>> // INITC as OINT, no reads, just write a response to the socket
>>> processOint(AContext, imageData);
>>> }
>>> else {
>>> if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
>>> (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length 
>>> <=4) ) {
>>> // INITC as INIT, no reads, just write a response to the socket
>>> processInit(AContext, imageData);
>>> }
>>
>>Those if statements are a bit redundant.  You could clean that up as follows 
>>(more so if you get rid of the INITC buffer and use integers instead):
>>
>>    if( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
>>(imageData->rcv_buffer[1] == imageData->INITC[1]) )
>>    {
>>        if( imageData->length > 4 )
>>        {
>>            // INITC as OINT, no reads, just write a response to the socket
>>            processOint(AContext, imageData);
>>        }
>>        else
>>        {
>>            // INITC as INIT, no reads, just write a response to the socket
>>            processInit(AContext, imageData);
>>        }
>>    }
>>
>>> Now, I put in a return statement in the middle, just to see if I can get
>>> more than one connection, but it still fails.
>>
>>You need to be more specific about what "it fails" actually means.
>>
>>> For any "command" other than DISPLAY, it works fine, because they
>>> only come as single connections. If I get a series of DISPLAY commands,
>>> the first one usually goes ok (depends on the timing), but the second 
>>> client
>>> connection kills the service.
>>
>>Then most likely your processing and responding code, which you have no 
>>shown yet, is not managing the data/connection properly and corrupting the 
>>service in some way.
>>
>>> The only other thing I can think of, is that I'm adding the TIdTCPServer
>>> component to a wrong place (TService), and that's why it gets "clobbered".
>>
>>Putting a TIdTCPServer inside the TService works perfectly fine when used 
>>properly.
>>
>>
>>Gambit 
>>
>>
>
>So... here is an interesting outcome. I execute the service, attach it to CodeGear debugger, send some data, and it works, yet the same thing refuses to "service" more than one socket connection, when executed normally (without the debugger). So, my next question is:
>What, if any, maintenance must be performed on the instance of the IIdTCPServer? Context? Connection? IOHandler? What kind of open/close statement must be performed, and when? Are there any special properties that must be set, in order to run the code concurrently?
>Any input, as always, greatly appreciated!!!
>

Another interesting finding:
When I do a very simple data send to the server, the sequence I get is:
1. onConnect handler is invoked (no code, just log entry)
2. onExecute handler is invoked (runs ok)
3. onExecute handler is invokes AGAIN. I assume on the same socket connection. Exits out, because there is no data in the pipe (error code kicks in :-))
4. onDisconnect handler gets executed.
So... why is onExecute handler being invoked twice on the same socket connection?

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:47a2265f$1@newsgroups.borland.com...

> So... here is an interesting outcome.

For future reference, please trim your replies.

> I execute the service, attach it to CodeGear debugger, send some
> data, and it works, yet the same thing refuses to "service" more than
> one socket connection, when executed normally (without the debugger).

Then please show ALL of your code, or at least make a minimal test project 
that reproduces the same problem.  There has to be something in your code 
that is serializing/deadlocking the threads.  By itself, TIdTCPServer can 
service multiple clients at the same time without problem.

> What, if any, maintenance must be performed on the instance of the 
> IIdTCPServer?

None by you.  It is all handled internally by Indy for you.

> Are there any special properties that must be set, in order to run the
> code concurrently?

No.


Gambit 

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:47a23116$1@newsgroups.borland.com...

> When I do a very simple data send to the server, the sequence I get is:
> 1. onConnect handler is invoked (no code, just log entry)
> 2. onExecute handler is invoked (runs ok)
> 3. onExecute handler is invokes AGAIN.

That is normal behavior.

> I assume on the same socket connection.

Yes.  Use the TIdContext parameter to verify that.

> why is onExecute handler being invoked twice on the same socket 
> connection?

Because it is supposed to.  TIdTCPServer calls OnExecute in a continuous 
loop for the lifetime of the connection.  You have your OnExecute code set 
up to read a single command and then exit.  The server will loop around, 
call OnExecute again, read the next command (waiting as needed, raising an 
exception if error/disconnect occurs), exit, and repeat.  This is the 
preferred way to use TIdTCPServer, so you are fine.


Gambit 

>> why is onExecute handler being invoked twice on the same socket 
>> connection?
>
>Because it is supposed to.  TIdTCPServer calls OnExecute in a continuous 
>loop for the lifetime of the connection.  You have your OnExecute code set 
>up to read a single command and then exit.  The server will loop around, 
>call OnExecute again, read the next command (waiting as needed, raising an 
>exception if error/disconnect occurs), exit, and repeat.  This is the 
>preferred way to use TIdTCPServer, so you are fine.
>
>
>Gambit 
>
>
That's great info! Since, the "commands" in my case come from this old and not very well documented system, I figured I only get to the end of OnExecute handler, when the connection is closed by the host/client.
It is evidently not so.
So, what I did, is I added:
AContext->Connection->Disconnect();
at the very end of the handler to make sure that the connection is dropped after servicing it's content.
However, I see that sometimes it does not cause the OnDisconnect handler to fire, and the server does not accept any more connections after that (almost like it does not yeld to the main thread).
Is there any additional handling that must be done to any of the components (AContext, or IOHandler)?

P.S. I would love to post the full source, but I would have to provide the insides on how the ImagePlus sends its data, which is no fun :-)

Thanks!

"Remy Lebeau \(TeamB\)" <no.spam@no.spam.com> wrote:
>
>"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
>news:47a23116$1@newsgroups.borland.com...
>
>> When I do a very simple data send to the server, the sequence I get is:
>> 1. onConnect handler is invoked (no code, just log entry)
>> 2. onExecute handler is invoked (runs ok)
>> 3. onExecute handler is invokes AGAIN.
>
>That is normal behavior.
>
>> I assume on the same socket connection.
>
>Yes.  Use the TIdContext parameter to verify that.
>
>> why is onExecute handler being invoked twice on the same socket 
>> connection?
>
>Because it is supposed to.  TIdTCPServer calls OnExecute in a continuous 
>loop for the lifetime of the connection.  You have your OnExecute code set 
>up to read a single command and then exit.  The server will loop around, 
>call OnExecute again, read the next command (waiting as needed, raising an 
>exception if error/disconnect occurs), exit, and repeat.  This is the 
>preferred way to use TIdTCPServer, so you are fine.
>
>
>Gambit 
>
>

See... this is what I do not understand (excuse my ignorance here).
Shouldn't OnExecute be invoked only once per client connection?
1. A clinet connects.
2. The listener thread spawns a new thred and executes OnExecute handler in it.
3. Data exchange is performed within the OnExecute handler.
4. Either the client or the server closes the connection, and the thread ends.
5. The listener thread spawns a NEW thread for a new connection.
As such, I assume there should be a clear OnConnect/OnExecute/OnDisconnect sequence.
If I'm coming into the OnExecute for the second time with the same connection, how would I know where in the data I am?

To approach it from a different perspective and if the above is not true, how would I force the connection to be dropped completely when at the end of the OnExecute method?
Right now (as I shown in a different post), I have:

if (AContext->Connection->Connected()) {
    	AContext->Connection->Disconnect();
}

and the code inside of the if is always executed, but I do not get the OnDisconnect handler every time.
If the OnDisconnect handler is not invoked for some reason, the listener thread does not allow for any more connection anymore, and it does not yeld to the main application either (service is unstoppable).

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:47a369a7$1@newsgroups.borland.com...

> Shouldn't OnExecute be invoked only once per client connection?

No.  Most server code is written to loop its client handling, so the default 
behavior of OnExecute was designed to loop automatically for you so your 
handling code could be simplified.  This is also important for things like 
Indy's CommandHandlers, where the server has to handle the looping 
internally for you anyway, otherwise its internal handling breaks if you try 
to hook into it.  Also, in Indy 10, the connection handling is actually 
separated from the thread handling in order to support additional threading 
models (fibers, completion ports, etc), so you don't even have direct access 
to the actual thread anymore.

> 1. A clinet connects.
> 2. The listener thread spawns a new thred and executes OnExecute handler 
> in it.
> 3. Data exchange is performed within the OnExecute handler.
> 4. Either the client or the server closes the connection, and the thread 
> ends.
> 5. The listener thread spawns a NEW thread for a new connection.

All of that is exactly what TIdTCPServer actually does.  The only difference 
between what you understand and what Indy really does is in how #3 does its 
work.  Which do you think is easier for you:

1)
    procedure Execute(connection);
    begin
        while (not thread is terminated) and (connection is connected) do
        begin
            try
                repeat
                    read a command
                    process the command
                    send a reply
                until connection is disconnected;
            except
                disconnect connection
                terminate thread;
                exit;
            end;
        end;
    end;

2)
    procedure Execute(connection);
    begin
        read a command
        process the command
        send a reply
    end;

> As such, I assume there should be a clear OnConnect/OnExecute/OnDisconnect 
> sequence.

There already is, which is essentially as follows:

    try
        OnConnect;
        try
            repeat
                OnExecute;
            until Terminated;
        finally
            OnDisconnect;
        end;
    except
        Terminate;
    end

If you really want to do your own looping in OnExecute, you certainly can do 
that.  Simply disconnect the socket before exiting:

    procedure Execute()
    begin
        try
            while connected do
            begin
                read a command
                process the command
                send a reply
            end;
        finally
            disconnect
        end;
    end;

> If I'm coming into the OnExecute for the second time with the same 
> connection, how would I know where in the data I am?

Because you are supposed to exit the first OnExecute afte processing the 
first command, ready to process the next command when OnExecute is 
reentered, using the Data property of TIdPeerThread/TIdContext to persist 
any data you need between events (which you need to do anyway if you want to 
pass values between OnConnect/OnExecute/OnDisconnect.

> how would I force the connection to be dropped completely
> when at the end of the OnExecute method?

Just do so.  There is nothing stopping you from calling Disconnect() inside 
the OnExecute event.  The next time OnExecute is called, your 
reading/writing will raise an EIdConnClosedGracefully exception.  Simply 
allow that exception to escape back into the server (which you should be 
doing any EIdException-derived exception anyway) and it will release the 
socket and shut down the thread accordingly.

> the code inside of the if is always executed

Connected() can return true even if the socket is not actually connected 
anymore.  Most noticely, if there is still unread data in the connection's 
InputBuffer.  This is by design.

> I do not get the OnDisconnect handler every time.

OnDisconnect is not triggered until the thread is being shut down, whether 
the socket is still active or not.  That is the main reason why 
EIdException-derived exceptions must be allowed to reach the server's 
internal exception handlers, so it can track socket status correctly and 
manage the thread accordingly.  If you block the exceptions in your own 
exceptions handlers, the thread can continue to run endlessly until the 
server is deactivated.


Gambit 

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:47a33b83$1@newsgroups.borland.com...

> So, what I did, is I added:
> AContext->Connection->Disconnect();
> at the very end of the handler to make sure that the connection is
> dropped after servicing it's content. However, I see that sometimes
> it does not cause the OnDisconnect handler to fire

When used properly, yes it will.  The rest of your OnExecute code is 
probably blocking Indy's internal notifications from reaching the server's 
internal handlers correctly so it can stop the thread accordingly.

> the server does not accept any more connections after that (almost like
> it does not yeld to the main thread).

As long as the server's MaxConnections limit has not been reached, 
TIdTCPServer can continue servicing new clients just fine while old client 
threads are still running.


Gambit