Bug 2234 - Mono doesn't swallow some exceptions, unlike .NET
Summary: Mono doesn't swallow some exceptions, unlike .NET
Status: RESOLVED FIXED
Alias: None
Product: Class Libraries
Classification: Mono
Component: Windows.Forms ()
Version: 2.10.x
Hardware: PC Windows
: --- normal
Target Milestone: Untriaged
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2011-11-28 17:28 UTC by Thomas Goldstein
Modified: 2012-06-01 18:25 UTC (History)
2 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Test case (17.91 KB, application/zip)
2011-11-28 17:30 UTC, Thomas Goldstein
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Thomas Goldstein 2011-11-28 17:28:52 UTC
Under certain circumstances, .NET swallows exceptions, but Mono doesn't.
See test case.

Scenario:
- Add a ComboBox on a Form
- Add a SelectedIndexChanged event handler method, and throw an exception in it
- Set the ComboBox DataSource

Result: Mono crashes, while .NET silently swallows the exception.
Note that .NET does *not* swallow the exception if the user goes and selects another ComboBox item. It only happens when setting the DataSource, indirectly firing SelectedIndexChanged.

Not sure what's happening here. Is this documented behavior, or some oddity?
Comment 1 Thomas Goldstein 2011-11-28 17:30:09 UTC
Created attachment 964 [details]
Test case
Comment 2 Robert Wilkens 2012-05-26 22:00:32 UTC
I am not able to look into this at moment,  but i can confirm it...

In Windows, if you run the sample program in visual studio, the exception is indeed raised and caught by Visual Studio.  However, as Mr. Goldstein points out, when run outside of Visual Studio, it behaves as described in Windows -- That is, Windows does "eat" the exception, where mono doesn't.  Windows does raise the exception outside of visual studio if the selection changes though.

This sounds like an easy fix -- throw a dummy try/catch around the firing of that event under the conditions described above.

The next few days don't look good on my availability on researching the change and testing it and generating a unit test.  But i'll try to get back to this next week and see if i can get it done.

If someone else wants to do this before i get back to it (i.e. if i forget about it), feel free.
Comment 3 Thomas Goldstein 2012-05-27 02:29:44 UTC
Thanks for looking into it.

But if you do add a try / catch around it, the exception wouldn't be caught by MonoDevelop, and so the exception wouldn't be brought to the developer's attention, unlike with .NET.

So I'm not sure what's the best solution. Maybe only set the try / catch if (!Debugger.IsAttached), or something like that?
Comment 4 Robert Wilkens 2012-05-27 08:56:11 UTC
At first i didn't know if Debugger.IsAttached was real or you made that up -- so I grepped through the mono source, and didn't find too many references to it, but i did find it in:

./mcs/class/corlib/System.Diagnostics/ChangeLog:
        * Debugger.cs: Implement Debugger.IsAttached

So apparently, it's there in System.Diagnostics...

I realize you may have already known that, but as long as i found it, i wanted to document it here for when I get back to this.  I'm hoping for the user this doesn't require additionally referencing something like System.Diagnostics.dll because that would break things, but i will try to remember to check that (it should be obvious if i test the sample you provided, which i don't think references it).

I'm guessing something like this should work:
if (Debugger.IsAttached){
     fire event
} else {
     try{ 
           fire event
     } catch (Exception ex)
     {
           //either ignore event, or maybe print it to console 
           //since this is WinForms it won't be primarily displayed
           //i.e.
           Console.WriteLine(ex.Message+"\n"+ex.StackTrace);
     }
}
Comment 5 Thomas Goldstein 2012-05-27 13:01:29 UTC
I could be wrong, but I don't think Mono devs usually write to the console for things like that. And if we had to, Debug.Write may be a better idea, to avoid doing it in release mode. Other than that, I had the same thing in mind.

We'd need someone to review this, though, because it may not be the right solution.
Comment 6 Robert Wilkens 2012-05-27 20:56:29 UTC
I really didn't think printing to console was a good idea to be honest, that's why i prefaced it with a "maybe" ;-)..  I was looking for advice..  Thanks for feedback.

I think ignoring any exceptions in this case makes sense (you're either loading options first time, or changing it, so a handler which is expecting a user change is more likely to fail for an unimportant reason) - and i agree that getting approval from others makes sense too, but to be honest I worry if by comparing it to visual studio behavior and .net behavior i'm violating any copyright issues if i implement the same behavior in mono.  I remember in the early days of mono they were very careful about saying if you ever used a mono decompiler, they don't want you touching the mono source code, this isn't quite the same thing as touching the decompiler, we're not copying code, but we are copying functionality without necessarily referring to the open .net specifications, if there is such a beast of a document.
Comment 7 Robert Wilkens 2012-05-27 21:24:36 UTC
oops.. replace mono decompiler in above with ildasm (intermediate language disassembler).  To be honest, i've used that exactly once about 10 years ago, and only because the instructions in the .net book i had at the time said to do it as part of the tutorial, but i didn't really study the output so i don't think that disqualifies me now, 10 years later, from contributing.
Comment 8 Thomas Goldstein 2012-05-28 03:26:05 UTC
I do not know whether this particular behavior (swallowed exception) is documented or not, but I haven't seen it. Mimicking / reverse engineering .NET is okay, as long as we don't look into their code.
Comment 9 Robert Wilkens 2012-05-28 09:04:42 UTC
I have made the change at least as far as running standalone earlier this morning, and it works outside of the debugger.  But for testing under the debugger, I'm having all sorts of headaches trying to get monodevelop to _run_ under the development version of mono.  I know from past experience i need to 'gacutil /i' things like gdk/gtk/glib/gnome/mono.addins which i think i've done, but it's still not running so i must be missing something.  I'll figure it out eventually, but it may be a day or two.  Like yesterday, i'm not expecting to be home during the day today, but not sure yet.
Comment 10 Robert Wilkens 2012-05-28 09:52:22 UTC
Ok, I've now tested inside of monodevelop and it works as expected (that is it crashes inside the debugger).  At least it does if i press F5 to run it it - it crashes at the appropriate spot.  However, for some reason monodevelop (regardless of which version is run 2.x or 3.x) doesn't seem to have a top menu, so i can't click, for example, the run menu, instead i had to press F5 which did the same thing.  That sounds like another bug, though, because i have a mono source tree without my patch in it, and it does the same thing (no menu) without me changing any code.
Comment 11 Robert Wilkens 2012-05-29 07:33:53 UTC
Last night at the last minute i submitted a patch to github - and actually went forward and submitted a pull reuqest.  Someone should review it, in theory, before it is committed.  I verified as much as it doesn't cause any additional failures in the unit tests (there were 3 failueres before, and still are now; my new tests don't fail with this patch applied).

In the patch, i included two unit tests.

If it's accepted, great, the issue reported here is solved.  If not, my feelings won't be hurt.  I tried.

If you want to see code changes, and follow the pull request progress  (or leave a comment) - see https://github.com/mono/mono/pull/306 

I notice this is request #306 and the last pull request i submitted was 305 and that's already been put through.
Comment 12 Thomas Goldstein 2012-05-29 14:18:32 UTC
Thanks for your work, Robert. Let's wait and see.
Comment 13 Robert Wilkens 2012-05-31 22:19:12 UTC
FYI - It looks like the change was merged into the main branch, so it was accepted I believe.
Comment 14 Thomas Goldstein 2012-06-01 01:47:53 UTC
Cool. Marking as resolved, then.
Comment 15 Thomas Goldstein 2012-06-01 01:55:42 UTC
I just saw the patch. For what it's worth, I would have moved the content of each code path to a sub method, to avoid redundancy and making sure both paths always get changed in sync.

       if (Debugger.IsAttached){
         SetSelectedIndex();
       } else {
         try{
           SetSelectedIndex();
         } catch {
           //ignore exceptions here per 
           //bug 2234
         }

If anyone thinks it's desirable, I can take care of it.
Comment 16 Robert Wilkens 2012-06-01 06:35:45 UTC
I was thinking of doing that, but for some reason i thought that would've added too much code (realistically, that wouldn't have affected performance).  But you're right, it would've kept the logic portion of that code more consistent.  If you want to do this, you're welcome to, but as i recall i left a big comment in the code with a note about that and i'd say delete at least part of that comment too (or reword it).
Comment 17 Thomas Goldstein 2012-06-01 18:25:24 UTC
Okay, I'll do that.

By the way, I'm kind of confused by the behavior of .NET. The exception is indeed visible when debugging through Visual Studio, but not when using SharpDevelop... Not sure what's the reason for this difference.

But what's important is that Mono now matches .NET when running applications out of the IDE.