Locking on async calls

  • Thread starter Thread starter Shak
  • Start date Start date
S

Shak

Hi all,

I'm trying to write a thread-safe async method to send a message of the form
(type)(contents). My model is as follows:

private void SendMessage(int type, string message)
{
//lets send the messagetype via async
NetworkStream ns = client.GetStream(); //assume client globally
accessible
byte[] bytes = BitConverter.GetBytes(type);
ns.BeginWrite(bytes, 0, bytes.Length, CompleteSend, message); //only
called here in whole application
}

private void CompleteSend(IAsyncResult ar)
{
String message = (string)ar.AsyncState;
NetworkStream ns = client.GetStream();
BinaryWriter writer = new BinaryWriter(ns);
lock (ns)
{
//really need exclusive lock on ns here, since two sends may hit
endwrite at the same time.
ns.EndWrite(ar); // type sent
writer.Write(message); // string sent
writer.Write(DateTime.Now); // extra info sent
}
}

Questions:

1) Is the above the best way to achieve what I want? I could set up a thread
which builds the bytes for the whole message in a memory stream, and then
send that in one go via a blocking ns.Write(), but the above seems more
elegant (and possibly more interesting).

2) Is the lock in a reasonable place? I can't place it after the call to
EndWrite(), since that could allow more than one message type to be sent
before any message content (I'm presuming data actually gets sent when
EndWrite is called. If not, I'm not certain of how you can get concurrency
between multiple async calls). However, I'm afraid of what would happen if
EndWrite was to block, possibly say due to another thread's call to
BeginWrite.

3) Generally, can you have multiple calls to an async Begin* method before
calling the corresponding End* methods?

Thanks!

Shak
 
Hi Shak, again,

EndWrite doesn't require synchronization since your passing it an
IAsyncResult object reference. In other words, multiple BeginAsync calls
each have a corresponding IAsyncResult that is used to end each of the
calls. It's up to the BeginWrite method to call CompleteSend with the
appropriate IAsyncResult object.

You need synchronization across the entire ns.BeginWrite method, from
ns.BeginWrite to ns.EndWrite since you don't want one Thread writing the
'type' and then another Thread writing a different 'type' before the first
Thread gets a change to write the message.

void SendMessage(...)
{
lock (this)
// I would use an AutoResetEvent instead of locking
{
// TODO: synchronous write to socket
}
}
 
Thanks for that (and for all the attention!).

Although now I'm stumped. I don't want calls to SendMessage to tie up the
main thread; that's why I call the async as soon as possible.

I could move the contents of this method to a new thread, synchronising as
appropriate on the network stream.

Questions:

1) SendMessage is a selfcontained non-polling method that seemed, based on
the answers I got in another thread, a prime candidate for calling
asynchronously. I'm now struggling to find a use for async calls in my
application at all!

2) What are the costs invovled with creating a new thread over working
synchornosly? If SendMessage repeatedly gets called, I may spend more time
creating threads than making them do work!

3) Why would you use an AutoResetEvent instead of a lock in the sample code
below?

Shak

Dave Sexton said:
Hi Shak, again,

EndWrite doesn't require synchronization since your passing it an
IAsyncResult object reference. In other words, multiple BeginAsync calls
each have a corresponding IAsyncResult that is used to end each of the
calls. It's up to the BeginWrite method to call CompleteSend with the
appropriate IAsyncResult object.

You need synchronization across the entire ns.BeginWrite method, from
ns.BeginWrite to ns.EndWrite since you don't want one Thread writing the
'type' and then another Thread writing a different 'type' before the first
Thread gets a change to write the message.

void SendMessage(...)
{
lock (this)
// I would use an AutoResetEvent instead of locking
{
// TODO: synchronous write to socket
}
}

Shak said:
Hi all,

I'm trying to write a thread-safe async method to send a message of the
form
(type)(contents). My model is as follows:

private void SendMessage(int type, string message)
{
//lets send the messagetype via async
NetworkStream ns = client.GetStream(); //assume client globally
accessible
byte[] bytes = BitConverter.GetBytes(type);
ns.BeginWrite(bytes, 0, bytes.Length, CompleteSend, message); //only
called here in whole application
}

private void CompleteSend(IAsyncResult ar)
{
String message = (string)ar.AsyncState;
NetworkStream ns = client.GetStream();
BinaryWriter writer = new BinaryWriter(ns);
lock (ns)
{
//really need exclusive lock on ns here, since two sends may hit
endwrite at the same time.
ns.EndWrite(ar); // type sent
writer.Write(message); // string sent
writer.Write(DateTime.Now); // extra info sent
}
}

Questions:

1) Is the above the best way to achieve what I want? I could set up a
thread which builds the bytes for the whole message in a memory stream,
and then send that in one go via a blocking ns.Write(), but the above
seems more elegant (and possibly more interesting).

2) Is the lock in a reasonable place? I can't place it after the call to
EndWrite(), since that could allow more than one message type to be sent
before any message content (I'm presuming data actually gets sent when
EndWrite is called. If not, I'm not certain of how you can get
concurrency between multiple async calls). However, I'm afraid of what
would happen if EndWrite was to block, possibly say due to another
thread's call to BeginWrite.

3) Generally, can you have multiple calls to an async Begin* method
before calling the corresponding End* methods?

Thanks!

Shak
Click to expand...
Click to expand...
 
Hi Shak,

Inline:
1) SendMessage is a selfcontained non-polling method that seemed, based on
the answers I got in another thread, a prime candidate for calling
asynchronously. I'm now struggling to find a use for async calls in my
application at all!
Click to expand...

I mentioned in the other thread how you must synchronize access to your code
that writes to the socket so that the full stream of data pertaining to a
single, logical message is written before another caller can write to the
socket. A logical message includes the 'type', as in your example code, and
the message bits. Synchronization exists so you don't have two messages
mixed together. How could the server figure out which bits belong to which
message? Your SendMessage method can be called asynchronously to free up
the UI but should be synchronized internally so that all callers are queued
and no mixed messages are transmitted to the server.

The IAsyncResult object tells the underlying delegate which Thread the End*
method should terminate. You don't need to synchronize calls to a method
for which synchronization is already handled internally, however you must
synchronize the entire SendMessage routine as an atomic operation to prevent
mixed messages from being transmitted to the server.
2) What are the costs invovled with creating a new thread over working
synchornosly? If SendMessage repeatedly gets called, I may spend more time
creating threads than making them do work!
Click to expand...

Costs?

I'm suggesting that you execute SendMessage asynchronously from the UI
Thread but execute the code within the method synchronously as with my
locking example. Calling another method asynchronously from within
SendMessage doesn't make sense since SendMessage is already being called
asynchronously. Am I on point here?

You might be thinking in a very linear fashion. Try thinking more OO and
ask yourself this: If I encapsulate all of the code to handle communication
over a single socket, between a client and server, how many threads will I
need for one instance of that object? Your answer should be one. You
already know that only one message may be sent at a time since you can't
sign each packet using TCP. If you want to use the object to handle
communication for multiple, simealtaneous requests by calls from an external
object you must still synchronize access to the code, internally. If
multiple Threads call a method that writes a complete message to the socket,
including a message type and message data, you must queue the threads so
that only one may execute at any given time. From within your object you
don't need asynchronous calls if the object is being used asynchronously by
callers.
3) Why would you use an AutoResetEvent instead of a lock in the sample
code below?
Click to expand...

A. Performance
WaitHandles outperform locking.

B. Scaling
In my first attempt to write a Socket app, a while back, I used locking
because it was quick and easy. However, as the complexity of the app grew
locking became impossible to use and I had to rewrite the synchronization
code to use a WaitHandle. There proved to be many dead-locks when I used
locking and with WaitHandles dead-locking was much easier to avoid when the
code got really complex. AutoResetEvent allowed me to make a remoting
framework that uses a single Socket with support for two-way, asynchronous
calling between a client and server, effectively avoiding firewall and
routing issues on the client-side and I only needed one AutoResetEvent
object to do it. Now, I'm not saying that it can't be done with locking but
that might be like saying, "Visual Studio could have been written in
JScript" (although probably not that drastic of a difference :)

In short, it's easy to use a WaitHandle for this particular situation, it
will perform better, and may save you trouble in the future.

HTH
 
Dave,

Dave Sexton said:
Hi Shak,

Inline:

The IAsyncResult object tells the underlying delegate which Thread the
End* method should terminate. You don't need to synchronize calls to a
method for which synchronization is already handled internally, however
you must synchronize the entire SendMessage routine as an atomic operation
to prevent mixed messages from being transmitted to the server.
Click to expand...

I think my assumption that nothing happens between the Begin* and End* calls
is what was tripping me up here. I thought all Begin* did was to kick off
the callback on a new thread. Looking at that now, it's the totally wrong
way of thinking about it.
Costs?

I'm suggesting that you execute SendMessage asynchronously from the UI
Thread but execute the code within the method synchronously as with my
locking example. Calling another method asynchronously from within
SendMessage doesn't make sense since SendMessage is already being called
asynchronously. Am I on point here?
Click to expand...

Well, I'm writing the networking stuff for others (and myself) who may
prefer the object containing the connection to handle all the async calls
for them - ie call the send methods on the object synchronously. I'm not
sure it makes a difference really, since whether you make the thread
internally or externally to the connection object it'll have to be created
at some point.

I guess my concern is this: I've changed my recieve method to now loop
instead of having a new call each recieve. This requires a single thread, so
is pretty neat and efficient. However, I'll be creating a new thread for
each send, and my understanding is that thread creation, whether inside or
outside the connection object, is expensive. My initial hunch was that using
BeginWrite etc would be an efficient way to introduce asynchronous calling,
but in retrospect I'm thinking it's as costly as creating a thread manually
anyway. My messages are pretty small, so creating a thread to send a message
may hold up the UI more than the actual sending of the message (or is that
way off the mark?).
You might be thinking in a very linear fashion. Try thinking more OO and
ask yourself this: If I encapsulate all of the code to handle
communication over a single socket, between a client and server, how many
threads will I need for one instance of that object? Your answer should
be one.
Click to expand...

Hmm. Assume all my threads are created internally by the connection object
atm. At the moment I have a single thread looping on ns.Read() to handle
incoming messages. On building of a complete message, it then creates a new
thread to deal with that message (which at the moment is to simply fire an
event). This is the same model used for the server listening for incoming
connections (a loop thread that creates another thread per cycle). There is
no synchronising on the reading thread loop since there'll only be one call
to ns.Read() at a time by that single thread.

Messages could be sent by the UI (say) at arbitrary times, and in order not
to hold up the UI, a thread is created by the communication object for each
request. So there could be more than one sending thread dealing with a send
at a time, if the UI asks fast enough. So I'll synchronise around the
sending code there, using a WaitHandle (I assume that Read and Write on a
networkstream don't interfere with each other. Do they? If so I'll have to
sync around the read too).

So quite a few more threads than just one :(. I can't allow the processing
of incoming/outgoing messages to block the sending of new messages or
reciept of new ones. I'm not sure how to just do this with the single
thread.
You already know that only one message may be sent at a time since you
can't sign each packet using TCP. If you want to use the object to handle
communication for multiple, simealtaneous requests by calls from an
external object you must still synchronize access to the code, internally.
If multiple Threads call a method that writes a complete message to the
socket, including a message type and message data, you must queue the
threads so that only one may execute at any given time. From within your
object you don't need asynchronous calls if the object is being used
asynchronously by callers.


In short, it's easy to use a WaitHandle for this particular situation, it
will perform better, and may save you trouble in the future.

HTH
Click to expand...

It does, thanks. I think I'm almost there!

Shak
 
Hi Shak,

Inline:
I think my assumption that nothing happens between the Begin* and End*
calls is what was tripping me up here. I thought all Begin* did was to
kick off the callback on a new thread. Looking at that now, it's the
totally wrong way of thinking about it.
Click to expand...

You got it. AsyncCallback is a notification that the operation has ended.
IAsyncResult may be used to monitor the process on another Thread or end the
operation (which will block if the operation hasn't completed).
Well, I'm writing the networking stuff for others (and myself) who may
prefer the object containing the connection to handle all the async calls
for them - ie call the send methods on the object synchronously. I'm not
sure it makes a difference really, since whether you make the thread
internally or externally to the connection object it'll have to be created
at some point.
Click to expand...

The recommended model for handling synchronous and asynchronous calling
within one object is to have three methods:

IAsyncResult BeginDoStuff(?, AsyncCallback callBack, object state);
? DoStuff(?);
? EndDoStuff(IAsyncResult result);

Your obviously already familiar with this model. You can implement it in
your connection object using a strong-typed delegate's BeginInvoke and
EndInvoke methods.
Your caller passes in an AsyncCallback delegate to be invoked when the
process has ended. The caller must call EndDoStuff on your class and pass
in the appropriate IAsyncResult to obtain the return value and prevent
memory leaks (1.1 framework; not sure if memory leaks are a problem in 2.0
but I bet they are so always call End*)

The other option for callers is to call DoStuff directly for a synchronous
call.
I guess my concern is this: I've changed my recieve method to now loop
instead of having a new call each recieve. This requires a single thread,
so is pretty neat and efficient. However, I'll be creating a new thread
for each send, and my understanding is that thread creation, whether
inside or outside the connection object, is expensive. My initial hunch
was that using BeginWrite etc would be an efficient way to introduce
asynchronous calling, but in retrospect I'm thinking it's as costly as
creating a thread manually anyway. My messages are pretty small, so
creating a thread to send a message may hold up the UI more than the
actual sending of the message (or is that way off the mark?).
Click to expand...

You'd need to test performance on both to determine which is faster, with or
without the spawing of new Threads, since there are a few factors that play
into it although I doubt creating new Threads is going to be a performance
hit at all if you are creating, say, a chat program. Anyway, you could use
the ThreadPool for sending messages (as in my Begin* End* example above the
strong-typed delegate will use the ThreadPool) since you won't be blocking
indefinitely and you won't be making nested async ThreadPool Thread calls.
It's pretty effecient.
Hmm. Assume all my threads are created internally by the connection object
atm. At the moment I have a single thread looping on ns.Read() to handle
incoming messages. On building of a complete message, it then creates a
new thread to deal with that message (which at the moment is to simply
fire an event). This is the same model used for the server listening for
incoming connections (a loop thread that creates another thread per
cycle). There is no synchronising on the reading thread loop since
there'll only be one call to ns.Read() at a time by that single thread.
Messages could be sent by the UI (say) at arbitrary times, and in order
not to hold up the UI, a thread is created by the communication object for
each request. So there could be more than one sending thread dealing with
a send at a time, if the UI asks fast enough. So I'll synchronise around
the sending code there, using a WaitHandle (I assume that Read and Write
on a networkstream don't interfere with each other. Do they? If so I'll
have to sync around the read too).
Click to expand...

You are correct that read and write on the socket do not interfere with each
other.
So quite a few more threads than just one :(. I can't allow the processing
of incoming/outgoing messages to block the sending of new messages or
reciept of new ones. I'm not sure how to just do this with the single
thread.
Click to expand...

I wrote one thread per socket, which is what you are refering to exactly.
If you want to handle multiple client connections from one server object
then you are correct that you will need multiple sockets and therefore
multiple threads, but still one thread per socket. My point was that you
don't need to make asynchronous calls from within the content of one
client-server connection to read from the socket.
Here's a simple model that illustrates the use of one Thread per Socket for
reading data, to what I've been refering:

Server --> Listen for connections async (Client-acceptance Thread)
Client --> Connects to server
Server --> Accepts client and spawns Client-Listener Thread
Server --> Responds to client that connection was successful
Server --> Raises event to notify objects of client connection
Server --> Polls Socket for incoming data only
Client --> Sends message
Server --> Reads message
Server --> Raises event to notify objects of received message

If the server writes back to the client, then the server becomes the client
and the client becomes the server. This is not just semantics, you will
need the server and client code on both 'sides'. The only difference here
is that the client will not have an Acceptance Thread listening for incoming
connections. The Remoting framework requires another socket for callback
invocations for reasons that I'm sure you'll run into if your attempting to
use a single socket. For me, it took a while to get it working using one
Socket for two-way, asynchronous communication but it's possible if you want
to keep trying!

HTH
 
Dave, final post on this since I've already taken up a lot of your time!
The recommended model for handling synchronous and asynchronous calling
within one object is to have three methods ....
The other option for callers is to call DoStuff directly for a synchronous
call.
Click to expand...

I've provided a Send method that launches a threadpool thread to call
DoStuff - caller's in this context don't really need to know the result of a
send, it's kind of fire and forget. Anyway, Send therefore returns
immiediately while the threadpool thread does the actual sending. This seems
to be equivalent to setting up the Async mechanism above. Is that a correct
assessment? I mean, a single Send method, to the people using the object to
send messages, is easier to deal with than Begin* and End* calls...
You'd need to test performance on both to determine which is faster, with
or without the spawing of new Threads, since there are a few factors that
play into it although I doubt creating new Threads is going to be a
performance hit at all if you are creating, say, a chat program. Anyway,
you could use the ThreadPool for sending messages (as in my Begin* End*
example above the strong-typed delegate will use the ThreadPool) since you
won't be blocking indefinitely and you won't be making nested async
ThreadPool Thread calls. It's pretty effecient.
Click to expand...

Oh. That's exactly what I'm doing now! Super.
You are correct that read and write on the socket do not interfere with
each other.
Click to expand...

Thank heavens. I was wondering how to synchronise around the blocking
ns.Read() method without holding up Send()s!
I wrote one thread per socket, which is what you are refering to exactly.
Click to expand...

One looping permanent thread but possibly many others dealing with sending
and processing. Perhaps we mean the same thing...
If the server writes back to the client, then the server becomes the
client and the client becomes the server. This is not just semantics, you
will need the server and client code on both 'sides'. The only difference
here is that the client will not have an Acceptance Thread listening for
incoming connections.
Click to expand...

That's exactly how it's turned out to be now. My "server" spawns the same
client objects that the "clients" do.
The Remoting framework requires another socket for callback invocations
for reasons that I'm sure you'll run into if your attempting to use a
single socket. For me, it took a while to get it working using one Socket
for two-way, asynchronous communication but it's possible if you want to
keep trying!
Click to expand...

Yes, that and the relative heavyweightness of Remoting is what prompted me
to create my own system. I think what I've ended up with is much better than
to be honest :D.

Again, thanks for all the help!

Shak
 
Hi Shak,

Glad to help.

It sounds like a single method rolling the Begin and End methods into one is
fine, however make sure you call EndInvoke on the delegate. Of course, if
you are using ThreadPool.QueueUserWorkItem then that's all you need.
 
Back
Top