I have a tidtcpserver component that takes incoming messages processes some data and sends a response back to the user. My question is how do I block the process so that the next user doesn't overwrite the AContext process when the onexecute event fires and I have yet to complete processing of the previous message? It seems like you have to somehow block the event until the acontext pointer has at least been written to a queue. Is there a better way to do this?
al wrote:
> I have 2 questions. 1) Is it possible to create the MessageChannel
> object in the currently identified coding process such that these
> calls are protected?
Of course. And I already told you how to do it.
> The .Flock calls in the addconnection method contain
> the waitforsingleobject function to prevent other users from accessing
> the public list structure that holds the newclient record in a public tlist.
There was no such code in the snippets you showed earlier.
> This is for my knowledge to fix my current problems.
Which are what example?
> So I have been looking for samples to help me find sample coding to
> conform to my number 2 request. I found an sufficient example in the
> current message by Bardt Kent.
Don't rely on Bardt's example. He's having a lot of problems with it, and
it is not a trivial example to follow. I have posted many simpler examples
before, in these forums, in the Indy AToZed forums, and on StackOverflow.
> My question for number 2 (the example code) is in your response you
> indicate missing a couple of lock and unlock calls. Since the prior to
> the loop he uses the locklist method and at the end he calls the unlock
> list function I thought that was sufficient. Please indicate what should
> be added to make the lock calls work in Bardt example as required.
> Also, The new class is assigned to the tidtcpserver VIA the idtcpserver1.contextclass
> property. How does this new class referenced by the tidtcpserver in
> events like the onconnect event?
When a new client connects, TIdTCPServer creates a new instance of the class
specified in the ContextClass property, and then passes that object around
to the event. The ContextClass is declared as "class of TIdServerContext",
so any TIdServerContext-derived class type can be used.
> Do you have to cast the acontext variable to access the properties and
> methods in the onconnect event handler?
Yes.
> Lastly It seems that you are indicating that the methods like my
> addconnection, as defined earlier in this thread, be assigned directly
> to the derived tidservercontect class?
I did not say, or imply, any such thing. But you could certainly do that
if you want, and have them access the MessageClient internally as needed.
--
Remy Lebeau (TeamB)
> {quote:title=Remy Lebeau (TeamB) wrote:}{quote}
> al wrote:
>
> > MessageChannel is of type TMsgFramework class.
>
> That is not what I asked. But you answered my question below.
>
> > AddConnection is a procedure of MessageChannel.(TMsgFrameWork class )
> > - Are you saying that a new instance of AddConnection is automatically
> > created for each onconnect event.
>
> There are no "instances" of function calls. But yes, each thread that calls
> AddConnection() will run independantly of any other thread that is calling
> AddConnection(), even at the same time. Function parameters and local variables
> are specific to the calling thread, they do not overlap. So you only have
> to protect access to external data that is shared across thread boundaries.
> For instance, if each thread is calling AddConnection() on the same MessageChannel
> object, then you need to protect access to data members of that object.
>
> > The MsgChannel is not instantiated in the onconnect event of the
> > tidtcpserver. It is instantiated in the formcreate event of the application.
>
> This is the answer to my earlier question. Then yes, it is a singleton object
> that is being shared across thread boundaries. So you have to protect its
> data members from concurrent access.
>
> > Should a separate instance of MessageChannel be manually created for each
> connected PC?
>
> That depends on what MessageChannel actually does.
MessageChannel is a class that has the method addconnection which creates an extended record of additional information about each connection to the server. Additional information will be invormation like the version of the client application, user id, height and width of the screen, etc.
I have 2 questions. 1) Is it possible to create the MessageChannel object in the currently identified coding process such that these calls are protected? The .Flock calls in the addconnection method contain the waitforsingleobject function to prevent other users from accessing the public list structure that holds the newclient record in a public tlist. This is for my knowledge to fix my current problems.
2) For the long term fix : How do I derive a new class from tidservercontext and extend it accord to your prior suggestion? Can you reference some sample code or an external project sample?
So I have been looking for samples to help me find sample coding to conform to my number 2 request. I found an sufficient example in the current message by Bardt Kent.My question for number 2 (the example code) is in your response you indicate missing a couple of lock and unlock calls. Since the prior to the loop he uses the locklist method and at the end he calls the unlock list function I thought that was sufficient. Please indicate what should be added to make the lock calls work in Bardt example as
required. Also, The new class is assigned to the tidtcpserver VIA the idtcpserver1.contextclass property. How does this new class referenced by the tidtcpserver in events like the onconnect event? Do you have to cast the acontext variable to access the properties and methods in the onconnect event handler? Lastly It seems that you are indicating that the methods like my addconnection, as defined earlier in this thread, be assigned directly to the derived tidservercontect class?
>
> --
> Remy Lebeau (TeamB)
Edited by: al nickels on Jan 22, 2015 5:01 AM
al wrote:
> MessageChannel is of type TMsgFramework class.
That is not what I asked. But you answered my question below.
> AddConnection is a procedure of MessageChannel.(TMsgFrameWork class )
> - Are you saying that a new instance of AddConnection is automatically
> created for each onconnect event.
There are no "instances" of function calls. But yes, each thread that calls
AddConnection() will run independantly of any other thread that is calling
AddConnection(), even at the same time. Function parameters and local variables
are specific to the calling thread, they do not overlap. So you only have
to protect access to external data that is shared across thread boundaries.
For instance, if each thread is calling AddConnection() on the same MessageChannel
object, then you need to protect access to data members of that object.
> The MsgChannel is not instantiated in the onconnect event of the
> tidtcpserver. It is instantiated in the formcreate event of the application.
This is the answer to my earlier question. Then yes, it is a singleton object
that is being shared across thread boundaries. So you have to protect its
data members from concurrent access.
> Should a separate instance of MessageChannel be manually created for each
connected PC?
That depends on what MessageChannel actually does.
--
Remy Lebeau (TeamB)
> {quote:title=Remy Lebeau (TeamB) wrote:}{quote}
> al wrote:
>
> > The procedure below is the connect procedure from the tidtcpserver
> > please observe comments or concerns within the 2 procedures;
>
> The OnConnect, OnDisconnect, OnExecute, and OnException events are all triggered
> in the context of a client worker thread. The OnListenException event is
> triggered in the context of a listener socket worker thread.
>
> > MessageChannel.AddConnection( aContext ) ;
>
> What is MessageChannel? I assume is is a global singleton object, correct?
MessageChannel is of type TMsgFramework class.
>
> Your AddConnection() code is MOSTLY thread-safe, as it is mostly operating
> only on local variables and the passed-in AContext object. The only thing
> I see it doing that is not thread-safe is incrementing wcConnCtrl.VarWord
> variable, which is apparently a shared variable. You could use an atomic
> increment to deal with that, via InterlockedIncrement() or TInterlocked.Increment(),
> or wrap it with a mutex/criticalsection.
AddConnection is a procedure of MessageChannel.(TMsgFrameWork class ) - Are you saying that a new instance of AddConnection is automatically created for each onconnect event. The MsgChannel is not instantiated in the onconnect event of the tidtcpserver. It is instantiated in the formcreate event of the application. Should a separate instance of MessageChannel be manually created for each connected PC?
>
> > { Is this a safe call or should it be under control of a critical
> > section or mutex right after the begin statement?
>
> A mutex/criticalsection is fine, but right after the 'begin' statement would
> not be the correct place to use it.
>
> > It seems as though a second user would change the value of the
> > AContext in this procedure while I am executing the code causing
> > catastrophic or unreliable results.
>
> No, it would not. Each thread has its own independant call stack. Yes,
> AddConnection() could potentially be running multiple times simultaneously,
> one in each thread. But they will not affect each other's parameter values
> like you are thinking.
>
> > If a critical section was needed here would you place it before or
> > within this procedure?
>
> Within, and only where actually needed.
>
> > What happens if a lengthy processing routing was done here would the
> > other users have to wait
>
> If you are holding on to a lock during that lengthy routine, other clients
> would wait when they try to obtain the same lock to perform their own lengthy
> operations. Which is why locks should be as small and isolated as possible,
> and keep them locked for as short a time as possible. Don't lock access
> to anything that does not require locking.
>
> > { This routine adds a connection to the clientlist the alias or user
> > is added later in another call / function of the class }
>
> Are you aware that TIdTCPServer already maintains its own thread-safe list
> of connected clients? Why are you duplicating that effort manually?
>
Yes I am aware of this threadsafe list but I never considered using it in that manner. I'm not as good as you are. Isn't that obvious, but I am getting better.
> > New( NewClient ) ;
>
> Rather than use the TIdContext.Data property, I would suggest deriving a
> new class from TIdServerContext, add whatever fields/methods/propeties you
> want to it, and then set the TIdTCPServer.ContextClass property at runtime
> before activating the server. Just a suggestion, not a requirement. It
> will make your field management a little easier (for example, creating your
> MsgInfoStatus TList object in the TIdServerContext constructor instead of
> the OnConnect event, and freeing it in the destructor instead of the OnDisconnect
> event).
I will have to explore that during a revamp of the code.
>
> --
> Remy Lebeau (TeamB)
al wrote:
> The procedure below is the connect procedure from the tidtcpserver
> please observe comments or concerns within the 2 procedures;
The OnConnect, OnDisconnect, OnExecute, and OnException events are all triggered
in the context of a client worker thread. The OnListenException event is
triggered in the context of a listener socket worker thread.
> MessageChannel.AddConnection( aContext ) ;
What is MessageChannel? I assume is is a global singleton object, correct?
Your AddConnection() code is MOSTLY thread-safe, as it is mostly operating
only on local variables and the passed-in AContext object. The only thing
I see it doing that is not thread-safe is incrementing wcConnCtrl.VarWord
variable, which is apparently a shared variable. You could use an atomic
increment to deal with that, via InterlockedIncrement() or TInterlocked.Increment(),
or wrap it with a mutex/criticalsection.
> { Is this a safe call or should it be under control of a critical
> section or mutex right after the begin statement?
A mutex/criticalsection is fine, but right after the 'begin' statement would
not be the correct place to use it.
> It seems as though a second user would change the value of the
> AContext in this procedure while I am executing the code causing
> catastrophic or unreliable results.
No, it would not. Each thread has its own independant call stack. Yes,
AddConnection() could potentially be running multiple times simultaneously,
one in each thread. But they will not affect each other's parameter values
like you are thinking.
> If a critical section was needed here would you place it before or
> within this procedure?
Within, and only where actually needed.
> What happens if a lengthy processing routing was done here would the
> other users have to wait
If you are holding on to a lock during that lengthy routine, other clients
would wait when they try to obtain the same lock to perform their own lengthy
operations. Which is why locks should be as small and isolated as possible,
and keep them locked for as short a time as possible. Don't lock access
to anything that does not require locking.
> { This routine adds a connection to the clientlist the alias or user
> is added later in another call / function of the class }
Are you aware that TIdTCPServer already maintains its own thread-safe list
of connected clients? Why are you duplicating that effort manually?
> New( NewClient ) ;
Rather than use the TIdContext.Data property, I would suggest deriving a
new class from TIdServerContext, add whatever fields/methods/propeties you
want to it, and then set the TIdTCPServer.ContextClass property at runtime
before activating the server. Just a suggestion, not a requirement. It
will make your field management a little easier (for example, creating your
MsgInfoStatus TList object in the TIdServerContext constructor instead of
the OnConnect event, and freeing it in the destructor instead of the OnDisconnect
event).
--
Remy Lebeau (TeamB)
> {quote:title=Remy Lebeau (TeamB) wrote:}{quote}
> al wrote:
>
> > I have a tidtcpserver component that takes incoming messages processes
> > some data and sends a response back to the user. My question is how
> > do I block the process so that the next user doesn't overwrite the
> > AContext process when the onexecute event fires and I have yet to
> > complete processing of the previous message?
>
> TIdTCPServer is a multi-threaded component. Each client runs within its
> own worker thread. The OnExecute event is triggered in the context of those
> worker threads. While one client is blocked doing something, other clients
> are free to do other things. If your OnExecute code is doing everything
> in the context of the calling thread only, then all is good. But if it is
> doing something that has to cross thread boundaries, you have to provide
> thread-safe access to those operations. Whether that be accessing the main
> thread UI, reading/writing data that is shared by multiple threads, etc.
>
> > It seems like you have to somehow block the event until the acontext pointer
> > has at least been written to a queue. Is there a better way to do this?
>
> I cannot answer that without seeing what you are doing in the first place,
> and why you think it is not safe for multiple clients to do it at the same
> time.
> --
> Remy Lebeau (TeamB
The procedure below is the connect procedure from the tidtcpserver please observe comments or concerns within the 2 procedures;
procedure TForm1.SvrMsgCommConnect(AContext: TIdContext);
begin
{ is the code in this procedure the only protected section ?
what happens when another user connects and this routine is called?
I am assuming this is the portion that is threaded and protected by the tidtcpserver class.
the AddConnection call below is not a tthread method, it is a simple class function detailed below }
MessageChannel.AddConnection( aContext ) ;
end;
procedure TMsgFrameWork.AddConnection( AContext: TIdContext);
Var
NewClient : PClientRec ;
LockData : PLockData ;
Begin
{ Is this a safe call or should it be under control of a critical section or mutex right after the begin statement?
It seems as though a second user would change the value of the AContext in this procedure while I am executing the code causing catastrophic or unreliable results.
If a critical section was needed here would you place it before or within this procedure?
What happens if a lengthy processing routing was done here would the other users have to wait or would you call a threaded routing from here? This is just a curious matter for a case such as a file transfer or something. }
New( LockData ) ;
LockData.ProcName := 'AddConnection' ;
LockData.MsgStructure := lsUser ;
LockData.COID := 0 ;
LockData.ID := -9 ;
LockData.ProcTime := Now ;
LockData.IPAddr := Acontext.Connection.Socket.Binding.PeerIP ;
{ This routine adds a connection to the clientlist the alias or user is added
later in another call / function of the class }
Inc( wcConnCtrl.VarWord, 1 ) ;
AContext.Connection.IOHandler.WriteLn( Format( XMLHead + XMLTail, ['SPS_ConnectRQ', 'ResponseType="[0]" MSG="0" Status="Messenger: Status = Ready" ConnectTime="' + DateTimeToStr( Now ) + '"','SPS_ConnectRQ'] ) );
New( NewClient ) ;
try
NewClient.ClientIP := Acontext.Connection.Socket.Binding.PeerIP ;
NewClient.Connected := Now ;
NewClient.LastMsg := Now ;
NewClient.Session := aContext ;
NewClient.CoID := 0 ;
NewClient.LogEntNameID := 0 ;
NewClient.LogStatus := 7 ;
NewClient.LogEntity := Nil ;
NewClient.AliasStatus := 0 ;
NewClient.AliasEntNameID := 0 ;
NewClient.NotifSent := 0 ;
newClient.AliasEntity := Nil ;
NewClient.MsgCnt := 0 ;
NewClient.MsgState := msNone ;
NewClient.MsgInfoStatus := TList.Create ;
AContext.Data := Pointer( NewClient ) ;
except
dispose( newclient ) ;
end;
end;
Please know how much I appreciate your help.
al wrote:
> I have a tidtcpserver component that takes incoming messages processes
> some data and sends a response back to the user. My question is how
> do I block the process so that the next user doesn't overwrite the
> AContext process when the onexecute event fires and I have yet to
> complete processing of the previous message?
TIdTCPServer is a multi-threaded component. Each client runs within its
own worker thread. The OnExecute event is triggered in the context of those
worker threads. While one client is blocked doing something, other clients
are free to do other things. If your OnExecute code is doing everything
in the context of the calling thread only, then all is good. But if it is
doing something that has to cross thread boundaries, you have to provide
thread-safe access to those operations. Whether that be accessing the main
thread UI, reading/writing data that is shared by multiple threads, etc.
> It seems like you have to somehow block the event until the acontext pointer
> has at least been written to a queue. Is there a better way to do this?
I cannot answer that without seeing what you are doing in the first place,
and why you think it is not safe for multiple clients to do it at the same
time.
--
Remy Lebeau (TeamB)
Mega Search
Sign Up 
