Bug 497175 - Exceptions thrown in a method that is invoked via Control.Invoke are not propagated to the caller
Summary: Exceptions thrown in a method that is invoked via Control.Invoke are not prop...
Status: RESOLVED FIXED
Alias: None
Product: Mono: Class Libraries
Classification: Mono
Component: Windows.Forms (show other bugs)
Version: unspecified
Hardware: Other All
: P5 - None : Minor
Target Milestone: ---
Assignee: Mono Bugs
QA Contact: Mono Bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-22 08:30 UTC by Tom Spink
Modified: 2009-08-13 15:37 UTC (History)
2 users (show)

See Also:
Found By: Development
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments
A patch to introduce exception propagation to Control.Invoke (2.16 KB, patch)
2009-05-01 07:46 UTC, Tom Spink
Details | Diff
A better patch to introduce exception propagation to Control.Invoke (2.33 KB, patch)
2009-08-09 10:46 UTC, Tom Spink
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Spink 2009-04-22 08:30:55 UTC
Description of Problem:
When Control.Invoke is used to invoke a method, if that method throws an exception it does not get propagated back to the caller - rather an unhandled exception occurs.  The Microsoft implementation does propagate this exception, which can then be caught with in a try..catch block.

To fix this, AsyncMethodResult.EndInvoke should throw the exception (if any was generated), and this will automatically fix Control.Invoke, and indeed any handlers in async callbacks using Control.EndInvoke.

I've created a patch to introduce this functionality, but am unsure as to where to post it for review.

Steps to reproduce the problem:
1. Create a method that throws an exception.
2. Call that method using Control.Invoke, and wrap it in a try..catch block


Actual Results:
An unhandled exception.

Expected Results:
A caught exception.

How often does this happen? 
Always.
Comment 1 Carlos Alberto Cortez 2009-04-27 16:20:37 UTC
Post here the patch, and I will review it. Thanks!
Comment 2 Tom Spink 2009-05-01 07:46:19 UTC
Created attachment 289405 [details]
A patch to introduce exception propagation to Control.Invoke
Comment 3 Robert Jordan 2009-08-07 19:57:39 UTC
The first "throw ex;" in the patch must be a "throw;"
Comment 4 Carlos Alberto Cortez 2009-08-08 03:56:03 UTC
Besides to the comment by Robert, I think that in:


@@ -513,7 +525,14 @@

                        try {
                                AsyncMethodResult result = data.Result;
-                               object ret = data.Method.DynamicInvoke (data.Args);
+                               object ret;
+                               try {
+                                       ret = data.Method.DynamicInvoke (data.Args);
+                               } catch (Exception ex) {
+                                       result.CompleteWithException (ex);
+                                       return;
+                               }
+                               
                                result.Complete (ret);

We could move the AsyncMethodResult line out of the try block, so we can use a catch block to call result.CompleteWithException, instead of having two try blocks - which I don't like very much.

Besides that, the patch is fine. 

Thanks!
Comment 5 Tom Spink 2009-08-09 10:46:40 UTC
Created attachment 311470 [details]
A better patch to introduce exception propagation to Control.Invoke

Okay, so I've updated the patch to introduce those changes - thanks for reviewing guys!
Comment 6 Carlos Alberto Cortez 2009-08-13 15:37:41 UTC
I applied your patch in rev 139853. Thanks!

Carlos.