Async callback vs events vs ?

[Jonathan]

Hi all,

I have an application separated into visual, logic and data components
(c# dlls).


UserControl.dll
================

The visual component is a user control which implements an event sink
raised by the hosting application.

This sink is provided with application data via a parameter. The data
may not necessarily change between events.


Logic.dll
==========

The logic layer contains custom a business entity which maintains the
application data state. It can therefore determine when something has
changed.

It is "refreshed" via a call from the UserControl within its event handler.

If the state changes it is required to grab more information from a
database via a data layer.

Once this has been done, it signals the UserControl via an event that
new data has been obtained which needs displaying.


Data.dll
========

The data layer contains simple code to grab data from a database and,
once done, raises an event to which the Logic.dll subscribes.


This all works - but not in the way i had intended.



I (incorrectly) assumed that the UserControl would call a method in the
Logic.dll to refresh its data and return from this immediately to
service the UI and keep things responsive.

What actually happens is the call doesn't return until all subsequent
calls have completed (from the Logic.dll to Data.dll). Things are
happening synchronously and I might as well wait for the method calls to
complete instead of subscribing to and raising events.

I have considered a couple of ways to redesign to use aynchronous calls
(BeginInvoke/EndInvoke) either with a callback or no callback but I am
not sure whether this is the best or only way to address this.


If you have any guidance regarding best practices in this area I would
be grateful for your input.

Regards
Jon

 

[Peter Duniho]

[...]
I (incorrectly) assumed that the UserControl would call a method in the
Logic.dll to refresh its data and return from this immediately to
service the UI and keep things responsive.

What actually happens is the call doesn't return until all subsequent
calls have completed (from the Logic.dll to Data.dll). Things are
happening synchronously and I might as well wait for the method calls to
complete instead of subscribing to and raising events.

I have considered a couple of ways to redesign to use aynchronous calls
(BeginInvoke/EndInvoke) either with a callback or no callback but I am
not sure whether this is the best or only way to address this.

If you have any guidance regarding best practices in this area I would
be grateful for your input.

The main advantage here for an asynchronous implementation would be that
it would ensure that your UI is always responsive. Depending on your
"database", it may be that the whole updating process is lengthy, during
which time the UI can't redraw or respond to user input (if appropriate).

There are different ways to make the implementation asynchronous. A very
simple approach might be to keep essentially the design you have now, but
have your UserControl sub-class execute the method that calls your
Logic.dll in a different thread. Then that method would handle any
necessary updating when the initial call to Logic.dll returns by calling
Control.Invoke() to cause any UI changes.

IMHO, the only "best practice" that really applies is to have the UI not
become blocked waiting on some lengthy processing. That implies an
asynchronous implementation, but how you do that is entirely up to you.
Of course, another "best practice" is to keep things simple, and in this
case one of the simplest things to do would be to change what you've
already got as little as possible.

But, if you want a more complicated API, you can put the threading
implementation into Logic.dll instead of having your UserControl sub-class
handle that, so that the event handlers always return immediately, but if
there is processing that needs to be done as a result of an event being
raised, the handler starts the necessary asynchronous processing.

Pete

 

[Jonathan]

[...]
I (incorrectly) assumed that the UserControl would call a method in
the Logic.dll to refresh its data and return from this immediately to
service the UI and keep things responsive.

What actually happens is the call doesn't return until all subsequent
calls have completed (from the Logic.dll to Data.dll). Things are
happening synchronously and I might as well wait for the method calls
to complete instead of subscribing to and raising events.

I have considered a couple of ways to redesign to use aynchronous
calls (BeginInvoke/EndInvoke) either with a callback or no callback
but I am not sure whether this is the best or only way to address this.

If you have any guidance regarding best practices in this area I would
be grateful for your input.

The main advantage here for an asynchronous implementation would be that
it would ensure that your UI is always responsive. Depending on your
"database", it may be that the whole updating process is lengthy, during
which time the UI can't redraw or respond to user input (if appropriate).

There are different ways to make the implementation asynchronous. A
very simple approach might be to keep essentially the design you have
now, but have your UserControl sub-class execute the method that calls
your Logic.dll in a different thread. Then that method would handle any
necessary updating when the initial call to Logic.dll returns by calling
Control.Invoke() to cause any UI changes.

IMHO, the only "best practice" that really applies is to have the UI not
become blocked waiting on some lengthy processing. That implies an
asynchronous implementation, but how you do that is entirely up to you.
Of course, another "best practice" is to keep things simple, and in this
case one of the simplest things to do would be to change what you've
already got as little as possible.

But, if you want a more complicated API, you can put the threading
implementation into Logic.dll instead of having your UserControl
sub-class handle that, so that the event handlers always return
immediately, but if there is processing that needs to be done as a
result of an event being raised, the handler starts the necessary
asynchronous processing.

Pete

Pete,

Thanks for your thoughts. You are right on the money regarding what I
require and give some useful suggestions regarding how to do it.

I've taken a little while to look a bit deeper into the nitty gritty of
such async methods and found myself at a bit of a roadblock...

If I could just explain briefly what I have got so far then detail the
problems I seem to be having (maybe just my lack of understanding...)

I call a method on the logic component asynchronously from a UserControl
button click event...

private void btnAsynch_Click(object sender, EventArgs e)
{
MyLogic = new LogicClass();
MyLogicDelegate myLogicDelegate = new
MyLogicDelegate(MyLogic.LogicLongMethod);

MyLogic.LogicEvent += new EventHandler<LogicEventArgs>(MyLogic_LogicEvent);
btnAsynch.Enabled = false;
IAsyncResult resLog = myLogicDelegate.BeginInvoke(MyLogicCallback,null);
}

....which, on completion, calls back into the following method...

private void MyLogicCallback(IAsyncResult r)
{

AsyncResult result = r as AsyncResult;
MyDataDelegate caller = result.AsyncDelegate as MyLogicDelegate;
if (result.EndInvokeCalled == false)
{
try
{
caller.EndInvoke(r);
MethodInvoker updateForm = delegate
{
label2.Text = "Completed Asynch call via MyLogic";
btnAsynch.Enabled = true;
};
if (this.InvokeRequired) this.Invoke(updateForm); else updateForm();
}
catch (Exception e)
{
MessageBox.Show(e.Message, "blah bla blah");
}
}
}

....I also have an event which is handled in this method...

private void MyLogic_LogicEvent(object sender, LogicEventArgs e)
{
label1.Text = "MyLogic has raised an event";
}


The async call runs on a thread from the ThreadPool and, consequently,
the callback aso runs on this thread so I need to be careful about
updating the GUI.

The event was part of my original intention to "fire and forget" and
grab a lot of information from the LogicEventArgs.

I suppose I could use the IAsyncResult to pack this information in but -
and here is where I lose the plot - all of this is still running under
the thread from the threadpool as far as i can tell. This seems to make
life complicated and dangerous since there is still a lot I need to do
with the information returned including updating the GUI, calling more
methods, ...

How do I escape from this situation?

I hope this makes sense to you - feel free to put me straight on the
concepts I am obviously floundering with

Regards
Jon

 

[Peter Duniho]

[...]
I suppose I could use the IAsyncResult to pack this information in but -
and here is where I lose the plot - all of this is still running under
the thread from the threadpool as far as i can tell. This seems to make
life complicated and dangerous since there is still a lot I need to do
with the information returned including updating the GUI, calling more
methods, ...

How do I escape from this situation?

Define "escape". If by that you mean "avoid it altogether", you can't
without abandoning the idea of asynchronous processing altogether itself.
Asynchronous necessarily implies a thread other than your main GUI thread;
updating the GUI from some other thread necessarily implies using
Control.Invoke() or Control.BeginInvoke() to execute code that actually
updates the GUI.

The main thing when calling Control.Invoke() or Control.BeginInvoke() is
to make sure you aren't capturing variables in anonymous methods that may
change in the course of the threading. But even there, assuming those
variables are used only in the one async thread, then calling
Control.Invoke() will ensure that, because it blocks until the delegate
has actually been invoked; that thread _can't_ change the values, even if
they happen to be non-local, because the thread's just waiting for the GUI
to finish being updated.

With Control.BeginInvoke() you need to be more careful, but that mainly
means copying values from non-local variables into captured local
variables that you know won't be changed anywhere in that method.

That said, the code you posted looks mostly workable. The only thing you
probably need to really fix is the lack of a call to Control.Invoke() in
your event handler ("MyLogic_LogicEvent"). I would expect everything else
to run fine as-is.

Which is not to say it's how I'd write it myself. Other changes I'd
make:

-- Don't bother with the System.Runtime.Remoting.Messaging.AsyncResult
class. IAsyncResult provides everything you need; just pass your delegate
instance as the state object to the call to BeginInvoke(), and then
retrieve that from the IAsyncResult.AsyncState property.

-- Related to the above, there's no need to check the EndInvokeCalled
property; your callback will only be called once per invocation, and you
should write the code to only call EndInvoke() in that callback. Thus,
EndInvokeCalled will (should) always be "false".

-- Also, there's no need to check InvokeRequired. It's a legacy
property that really has no need any longer (and barely had a need in the
past). The call to Control.Invoke() does the exact same check, and will
simply invoke the delegate directly if InvokeRequired is false. There's
practically no additional overhead even if it does sometimes happen that
InvokeRequired might be "false", and of course in this case that's just
not going to happen anyway.

But note that those changes are all mostly stylistic; they have little
bearing on the _correctness_ of the code.

I hope this makes sense to you - feel free to put me straight on the
concepts I am obviously floundering with

For someone who's floundering, you sure seem to already understand pretty
much everything that's going on.

Pete

 

[Jonathan]

[...]
I suppose I could use the IAsyncResult to pack this information in but
- and here is where I lose the plot - all of this is still running
under the thread from the threadpool as far as i can tell. This seems
to make life complicated and dangerous since there is still a lot I
need to do with the information returned including updating the GUI,
calling more methods, ...

How do I escape from this situation?

Define "escape". If by that you mean "avoid it altogether", you can't
without abandoning the idea of asynchronous processing altogether
itself. Asynchronous necessarily implies a thread other than your main
GUI thread; updating the GUI from some other thread necessarily implies
using Control.Invoke() or Control.BeginInvoke() to execute code that
actually updates the GUI.

The main thing when calling Control.Invoke() or Control.BeginInvoke() is
to make sure you aren't capturing variables in anonymous methods that
may change in the course of the threading. But even there, assuming
those variables are used only in the one async thread, then calling
Control.Invoke() will ensure that, because it blocks until the delegate
has actually been invoked; that thread _can't_ change the values, even
if they happen to be non-local, because the thread's just waiting for
the GUI to finish being updated.

With Control.BeginInvoke() you need to be more careful, but that mainly
means copying values from non-local variables into captured local
variables that you know won't be changed anywhere in that method.

That said, the code you posted looks mostly workable. The only thing
you probably need to really fix is the lack of a call to
Control.Invoke() in your event handler ("MyLogic_LogicEvent"). I would
expect everything else to run fine as-is.

Which is not to say it's how I'd write it myself. Other changes I'd
make:

-- Don't bother with the
System.Runtime.Remoting.Messaging.AsyncResult class. IAsyncResult
provides everything you need; just pass your delegate instance as the
state object to the call to BeginInvoke(), and then retrieve that from
the IAsyncResult.AsyncState property.

-- Related to the above, there's no need to check the
EndInvokeCalled property; your callback will only be called once per
invocation, and you should write the code to only call EndInvoke() in
that callback. Thus, EndInvokeCalled will (should) always be "false".

-- Also, there's no need to check InvokeRequired. It's a legacy
property that really has no need any longer (and barely had a need in
the past). The call to Control.Invoke() does the exact same check, and
will simply invoke the delegate directly if InvokeRequired is false.
There's practically no additional overhead even if it does sometimes
happen that InvokeRequired might be "false", and of course in this case
that's just not going to happen anyway.

But note that those changes are all mostly stylistic; they have little
bearing on the _correctness_ of the code.

I hope this makes sense to you - feel free to put me straight on the
concepts I am obviously floundering with

For someone who's floundering, you sure seem to already understand
pretty much everything that's going on.

Pete

Pete,

Much obliged again for your thoughts and thanks for your reassurance!

I don't have anyone to bounce off at work for C# stuff and it's
sometimes difficult - even when you think you know something - to be
completely sure that you are correct in your understanding.


Re the event handler:
I left the event handler code as it was since I anticipated abandoning
this mechanism in favour of callbacks. I read an anecdote somewhere
(sorry, cannot cite this) that got me a little worried:

"this is not the only important point I disagree with in that article.
For example, I think that it's almost always going to be a bad idea to
use asynchronous delegate invocation to raise events. This would be a
total disaster if you ever used the event source in a Windows Forms
application, for example."

If this is a red herring and there are really no issues with such a
mechanism, I'll follow your recommendation for this.

Your other observations are very useful - I kind of "get it" now
regarding what's going on in the background which is important to see
*why* things need to be done the way they do...

Thanks again!

Regards
Jon

 

[Peter Duniho]

[...]
Re the event handler:
I left the event handler code as it was since I anticipated abandoning
this mechanism in favour of callbacks. I read an anecdote somewhere
(sorry, cannot cite this) that got me a little worried:

"this is not the only important point I disagree with in that article.
For example, I think that it's almost always going to be a bad idea to
use asynchronous delegate invocation to raise events. This would be a
total disaster if you ever used the event source in a Windows Forms
application, for example."

If this is a red herring and there are really no issues with such a
mechanism, I'll follow your recommendation for this.

It seems to me that the warning is overblown. "Disaster" is such a strong
word.

It's true that raising events asynchronously introduces cross-thread
issues that wouldn't exist if everything ran on the same thread. But that
just means you need to be careful. It doesn't mean that it's disastrous
to design the code that way. And it's absolutely necessary to design the
code that way if you want asynchronous behavior.

That last point is important: there is practically no difference between
using an event and using a "callback" (which would necessarily have to be
a delegate). In fact, an event has to be declared in terms of a delegate
type, and it's just a special way of encapsulating delegates. If you are
going to use an asynchronous design, and if part of that design requires a
callback of _any_ form to the client (whether that's a simple delegate or
an event), and if the client is a Forms application and needs to perform
some kind of GUI update in response to the callback, then you'll have to
deal with that issue by using Control.Invoke() or Control.BeginInvoke(),
no matter what.

Hope that helps.

Pete