Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: when debugger encounters a not-unhandled exception, it stops and shows "Dns.cs file not found" (Dns being the BCL class used) #17083

Closed
knocte opened this issue Aug 28, 2019 · 19 comments · Fixed by #17106
Assignees
Labels

Comments

@knocte
Copy link
Contributor

@knocte knocte commented Aug 28, 2019

I believe this is a regression because I haven't seen this behaviour in previous versions of VisualStudio for Mac (now using 8.2.4; not sure the exact version when this started happening, possibly when Mono 6.0 was bundled).

Steps to reproduce:

  1. Open VS4Mac, create new project/solution, Console type, language: C#.
  2. Use this simple snippet:
using System;
using System.Linq;
using System.Collections.Generic;
using System.Net.Sockets;
using System.Threading.Tasks;

namespace TestResolveCSharpConsole
{
    class MainClass
    {
        public static void Main(string[] args)
        {
            try
            {
                var results = ResolveAsync().GetAwaiter().GetResult();
            }
            catch (SocketException sockEx)
            {
                Console.WriteLine("correctly handled");
            }
        }

        public static async Task<List<string>> ResolveAsync()
        {
            var addresses = await System.Net.Dns.GetHostAddressesAsync("foo.bar.baz");
            return new List<string>(addresses.Select(addr => addr.ToString()));
        }
    }
}
  1. Compile it. (If you run it outside VS4Mac, result should just be the printing of the string "correctly handled".)
  2. Run it with the debugger inside VS4Mac.

Expected results:
Should just run it and not stop, finish execution at the end like it's the case when being run outside of VS4Mac.

Current results:
Debugger seems to bomb when a SocketException is thrown, even if it's properly handled with a try-catch (this is the first bug). Then it tries to open the source code of the BCL where this exception happened (Dns.cs), telling the weird error "Source Not Found" to the user.

See screenshot:
Screenshot 2019-08-28 at 3 56 16 PM

VS bug #973786, VS bug #989287

@jstedfast

This comment has been minimized.

Copy link
Member

@jstedfast jstedfast commented Sep 25, 2019

Pretty sure this is caused by the new "SourceLink" feature added recently by @nosami :-\

@jstedfast jstedfast assigned nosami and unassigned jstedfast Sep 25, 2019
@nosami

This comment has been minimized.

Copy link
Member

@nosami nosami commented Sep 25, 2019

Haven't attempted to repro this yet, but it looks like 2 different bugs to me. The 1st bug is that the debugger shouldn't pause at all. The 2nd bug is that if the debugger does break for this reason, then SourceLink should have correctly found the Dns.cs file to download (provided this feature is even turned on, the bug report doesn't mention this)

@nosami

This comment has been minimized.

Copy link
Member

@nosami nosami commented Sep 25, 2019

@jstedfast with SourceLink enabled, the debugger correctly shows the location of the code that shows the exception

image

The issue here though is that the debugger shouldn't break at all, not SourceLink AFAICT

@nosami nosami assigned jstedfast and unassigned nosami Sep 25, 2019
@nosami

This comment has been minimized.

Copy link
Member

@nosami nosami commented Sep 25, 2019

The same bug happens with VSMac 8.2.3.5 before SourceLink was even introduced + Mono 6.4.0.198. Pretty sure that this is a regression in Mono.

@nosami

This comment has been minimized.

Copy link
Member

@nosami nosami commented Sep 25, 2019

@thaystg Please could you look at this and see if this is a bug in Mono? Thanks!

@knocte

This comment has been minimized.

Copy link
Contributor Author

@knocte knocte commented Sep 26, 2019

(provided this feature is even turned on, the bug report doesn't mention this)

It doesn't mention this because this bug was reported when stable channel only had the 8.2.6 version of VS4Mac. It's 8.3.x the one that introduces SourceLink, right? Because I haven't upgraded yet to 8.3.x and I don't see the setting in the debugger preferences page, so SourceLink must be disabled.

@knocte

This comment has been minimized.

Copy link
Contributor Author

@knocte knocte commented Sep 26, 2019

but it looks like 2 different bugs to me. The 1st bug is that the debugger shouldn't pause at all. The 2nd bug is that if the debugger does break for this reason, then SourceLink should

But what if there's no SourceLink(8.2.x) or SourceLink is disabled(8.3.x)? The risk of fixing the 1st bug first is that then it will hide the 2nd bug under the rug (this wouldn't mean that the 2nd bug is fixed, just less likely to happen). IMHO the 2nd bug should be fixed first.

@nosami

This comment has been minimized.

Copy link
Member

@nosami nosami commented Sep 26, 2019

@knocte there isn't a 2nd bug after all, only a first one :) SourceLink isn't connected to this issue. I repro'd using VSMac 8.1 / Mono 6.4.0.198 too

@knocte

This comment has been minimized.

Copy link
Contributor Author

@knocte knocte commented Sep 26, 2019

@knocte there isn't a 2nd bug after all, only a first one :) SourceLink isn't connected to this issue. I repro'd using VSMac 8.1 / Mono 6.4.0.198 too

I'm guessing that what you mean is that it's not SourceLink related, sure, but the 2nd bug still exists. Fixing only the 1st one might just "hide" it for this scenario, but could resurface in another one (e.g. if user configures debugger to stop in all exceptions, even handled ones?).

@nosami

This comment has been minimized.

Copy link
Member

@nosami nosami commented Sep 26, 2019

No. What I meant is that there isn't a 2nd bug.

The page that you are seeing is what happens by design when there is no source code for the current location irrespective of SourceLink.

@knocte

This comment has been minimized.

Copy link
Contributor Author

@knocte knocte commented Sep 26, 2019

when there is no source code for the current location

And shouldn't the source code of Mono be shipped by VS4Mac so that a dev can step into the BCL?

@nosami

This comment has been minimized.

Copy link
Member

@nosami nosami commented Sep 26, 2019

And shouldn't the source code of Mono be shipped by VS4Mac so that a dev can step into the BCL?

No. That was the whole point of adding SourceLink - so that we can stop adding source code in the various Mono pkgs.

@nosami

This comment has been minimized.

Copy link
Member

@nosami nosami commented Sep 26, 2019

The bug here is that the debugger stops on handled exceptions when it should not.

@knocte

This comment has been minimized.

Copy link
Contributor Author

@knocte knocte commented Sep 26, 2019

so that we can stop adding source code in the various Mono pkgs.

But Mono source code is not available through nuget. I thought SourceLink was integration with nuget? Can then VS4Mac8.3 show Dns.cs from Mono out of the box?

@nosami

This comment has been minimized.

Copy link
Member

@nosami nosami commented Sep 26, 2019

I thought SourceLink was integration with nuget?

No. SourceLink is a technology that isn't connected to NuGet but is used by many NuGet packages.

Can then VS4Mac8.3 show Dns.cs from Mono out of the box?

Yes, with these settings

image

@knocte

This comment has been minimized.

Copy link
Contributor Author

@knocte knocte commented Sep 26, 2019

Cool, understood!

@thaystg let me know if you need any help reproducing it. You might want to add more parallelisation to the testcase (as the snippet I posted was just my effort to convert the bug to a minimal repro; but I initially reproduced it with many more async parallel jobs running simultaneously, not just one).

@jstedfast jstedfast transferred this issue from mono/monodevelop Sep 26, 2019
@jstedfast jstedfast removed their assignment Sep 26, 2019
@marek-safar

This comment has been minimized.

Copy link
Member

@marek-safar marek-safar commented Sep 26, 2019

Is this really regression? It feels to me from the description the sourcelink made this more likely happen, right?

monojenkins added a commit to monojenkins/mono that referenced this issue Oct 7, 2019
…r catch generated by the owner thread, so we don't need to throw, we can continue and find the next catch. Fixes mono#17083
thaystg added a commit to thaystg/mono that referenced this issue Oct 7, 2019
…r catch generated by the method called in the owner thread, so we don't need to throw as unhandled exception, we can continue and find the next catch.

Fixes mono#17083

Backport of mono#17106.
monojenkins added a commit to monojenkins/mono that referenced this issue Oct 7, 2019
…r catch generated by the method called in the owner thread, so we don't need to throw as unhandled exception, we can continue and find the next catch.

Fixes mono#17083

Backport of mono#17106.
thaystg added a commit that referenced this issue Oct 7, 2019
…#17202)

* If there is a perform_wait_callback in the stack there will be another catch generated by the owner thread, so we don't need to throw, we can continue and find the next catch. Fixes #17083

* Reverting unit test changed on commit 405d521.

* Fixing assert when calling mono_jit_info_get_method if it was a trampoline.
marek-safar added a commit that referenced this issue Oct 9, 2019
…r catch generated by the method called in the owner thread, so we don't need to throw as unhandled exception, we can continue and find the next catch.

Fixes #17083

Backport of #17106.
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Oct 11, 2019
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Oct 16, 2019
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Oct 16, 2019
thaystg added a commit to thaystg/mono that referenced this issue Oct 17, 2019
… approach to fix this mono#17084, together.

The thing is when we are executing on hybrid suspend there is an extra try catch in the runtime_invoke_ method that is generated. And this extra try catch should be ignored, because this will generate more exceptions to debugger than expected, and remove part of the stack trace.

So my new try is ignore this try catch that is generated in runtime_invoke_ when hybrid suspend is being used. With this all the three issues are fixed.

Fixes mono#17084
thaystg added a commit to thaystg/mono that referenced this issue Oct 22, 2019
… approach to fix this mono#17084, together.

The thing is when we are executing on hybrid suspend the caller is executed inside try catch in the runtime_invoke_ method that is generated. And this try catch should be ignored for the debugger exception, otherwise it's the last try catch in main thread, but should always be considered to generate the stack trace.

Fixes mono#17084
monojenkins added a commit to monojenkins/mono that referenced this issue Oct 23, 2019
… approach to fix this mono#17084, together.

The thing is when we are executing on hybrid suspend the caller is executed inside try catch in the runtime_invoke_ method that is generated. And this try catch should be ignored for the debugger exception, otherwise it's the last try catch in main thread, but should always be considered to generate the stack trace.

Fixes mono#17084
monojenkins added a commit to monojenkins/mono that referenced this issue Oct 23, 2019
… approach to fix this mono#17084, together.

The thing is when we are executing on hybrid suspend the caller is executed inside try catch in the runtime_invoke_ method that is generated. And this try catch should be ignored for the debugger exception, otherwise it's the last try catch in main thread, but should always be considered to generate the stack trace.

Fixes mono#17084
thaystg added a commit that referenced this issue Oct 23, 2019
…d is enabled (#17478)

* Removing all fixes to fix #15203 and #17083, and trying a new approach to fix this #17084, together.

The thing is when we are executing on hybrid suspend the caller is executed inside try catch in the runtime_invoke_ method that is generated. And this try catch should be ignored for the debugger exception, otherwise it's the last try catch in main thread, but should always be considered to generate the stack trace.

Fixes #17084

* This test check the case that we were missing. On preemptive this test always runs perfectly, on Hybrid, never run well until this PR.

* Changing how call unit test.
Changing variable name.
marek-safar added a commit that referenced this issue Oct 24, 2019
* Removing all fixes to fix #15203 and #17083, and trying a new approach to fix this #17084, together.

The thing is when we are executing on hybrid suspend the caller is executed inside try catch in the runtime_invoke_ method that is generated. And this try catch should be ignored for the debugger exception, otherwise it's the last try catch in main thread, but should always be considered to generate the stack trace.

Fixes #17084

* This test check the case that we were missing. On preemptive this test always runs perfectly, on Hybrid, never run well until this PR.

* Changing how call unit test. Changing variable name.
marek-safar added a commit that referenced this issue Oct 24, 2019
* Removing all fixes to fix #15203 and #17083, and trying a new approach to fix this #17084, together.

The thing is when we are executing on hybrid suspend the caller is executed inside try catch in the runtime_invoke_ method that is generated. And this try catch should be ignored for the debugger exception, otherwise it's the last try catch in main thread, but should always be considered to generate the stack trace.

Fixes #17084

* This test check the case that we were missing. On preemptive this test always runs perfectly, on Hybrid, never run well until this PR.

* Changing how call unit test. Changing variable name.
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Oct 28, 2019
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Dec 3, 2019
Changes: mono/api-snapshot@fc50bc4...45a61d9

        $ git diff --shortstat fc50bc4f...45a61d93
         22 files changed, 775 insertions(+), 474 deletions(-)

Changes: mono/cecil@a6c8f5e...a6a7f5c

        $ git diff --shortstat a6c8f5e1...a6a7f5c0
         55 files changed, 818 insertions(+), 530 deletions(-)

Changes: mono/corefx@1f87de3...49f1c45

        $ git diff --shortstat e4f7102b...49f1c453
         38 files changed, 1171 insertions(+), 419 deletions(-)

Changes: mono/linker@ebe2a1f...e8d054b

        $ git diff --shortstat ebe2a1f4...e8d054bf
         137 files changed, 5360 insertions(+), 1781 deletions(-)

Changes: mono/mono@8946e49...18920a8

        $ git diff --shortstat 8946e49a...18920a83
         1811 files changed, 47240 insertions(+), 48331 deletions(-)

Changes: xamarin/xamarin-android-api-compatibility@a61271e...50a3c52

        $ git diff --shortstat a61271e0...50a3c52d
         1 file changed, 2 insertions(+), 791 deletions(-)

Fixes: #3619

Context: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1005448
Context: https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/967582
Context: dotnet/coreclr#26370
Context: dotnet/coreclr#26479
Context: dotnet/corefx#40455
Context: dotnet/corefx#40578
Context: mono/mono#7377
Context: mono/mono#12421
Context: mono/mono#12586
Context: mono/mono#14080
Context: mono/mono#14725
Context: mono/mono#14772
Context: mono/mono#15261
Context: mono/mono#15262
Context: mono/mono#15263
Context: mono/mono#15307
Context: mono/mono#15308
Context: mono/mono#15310
Context: mono/mono#15646
Context: mono/mono#15687
Context: mono/mono#15805
Context: mono/mono#15992
Context: mono/mono#15994
Context: mono/mono#15999
Context: mono/mono#16032
Context: mono/mono#16034
Context: mono/mono#16046
Context: mono/mono#16192
Context: mono/mono#16308
Context: mono/mono#16310
Context: mono/mono#16369
Context: mono/mono#16380
Context: mono/mono#16381
Context: mono/mono#16395
Context: mono/mono#16411
Context: mono/mono#16415
Context: mono/mono#16486
Context: mono/mono#16570
Context: mono/mono#16605
Context: mono/mono#16616
Context: mono/mono#16689
Context: mono/mono#16701
Context: mono/mono#16712
Context: mono/mono#16742
Context: mono/mono#16759
Context: mono/mono#16803
Context: mono/mono#16808
Context: mono/mono#16824
Context: mono/mono#16876
Context: mono/mono#16879
Context: mono/mono#16918
Context: mono/mono#16943
Context: mono/mono#16950
Context: mono/mono#16974
Context: mono/mono#17004
Context: mono/mono#17017
Context: mono/mono#17038
Context: mono/mono#17040
Context: mono/mono#17083
Context: mono/mono#17084
Context: mono/mono#17133
Context: mono/mono#17139
Context: mono/mono#17151
Context: mono/mono#17180
Context: mono/mono#17278
Context: mono/mono#17549
Context: mono/mono#17569
Context: mono/mono#17665
Context: mono/mono#17687
Context: mono/mono#17737
Context: mono/mono#17790
Context: mono/mono#17924
Context: mono/mono#17931
Context: https://github.com/mono/mono/issues/26758
Context: https://github.com/mono/mono/issues/37913
Context: xamarin/xamarin-macios#7005
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Dec 3, 2019
Changes: mono/api-snapshot@fc50bc4...45a61d9

        $ git diff --shortstat fc50bc4f...45a61d93
         22 files changed, 775 insertions(+), 474 deletions(-)

Changes: mono/cecil@a6c8f5e...a6a7f5c

        $ git diff --shortstat a6c8f5e1...a6a7f5c0
         55 files changed, 818 insertions(+), 530 deletions(-)

Changes: mono/corefx@1f87de3...49f1c45

        $ git diff --shortstat e4f7102b...49f1c453
         38 files changed, 1171 insertions(+), 419 deletions(-)

Changes: mono/linker@ebe2a1f...e8d054b

        $ git diff --shortstat ebe2a1f4...e8d054bf
         137 files changed, 5360 insertions(+), 1781 deletions(-)

Changes: mono/mono@8946e49...18920a8

        $ git diff --shortstat 8946e49a...18920a83
         1811 files changed, 47240 insertions(+), 48331 deletions(-)

Changes: xamarin/xamarin-android-api-compatibility@a61271e...50a3c52

        $ git diff --shortstat a61271e0...50a3c52d
         1 file changed, 2 insertions(+), 791 deletions(-)

Fixes: #3619

Context: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1005448
Context: https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/967582
Context: dotnet/coreclr#26370
Context: dotnet/coreclr#26479
Context: dotnet/corefx#40455
Context: dotnet/corefx#40578
Context: mono/mono#7377
Context: mono/mono#12421
Context: mono/mono#12586
Context: mono/mono#14080
Context: mono/mono#14725
Context: mono/mono#14772
Context: mono/mono#15261
Context: mono/mono#15262
Context: mono/mono#15263
Context: mono/mono#15307
Context: mono/mono#15308
Context: mono/mono#15310
Context: mono/mono#15646
Context: mono/mono#15687
Context: mono/mono#15805
Context: mono/mono#15992
Context: mono/mono#15994
Context: mono/mono#15999
Context: mono/mono#16032
Context: mono/mono#16034
Context: mono/mono#16046
Context: mono/mono#16192
Context: mono/mono#16308
Context: mono/mono#16310
Context: mono/mono#16369
Context: mono/mono#16380
Context: mono/mono#16381
Context: mono/mono#16395
Context: mono/mono#16411
Context: mono/mono#16415
Context: mono/mono#16486
Context: mono/mono#16570
Context: mono/mono#16605
Context: mono/mono#16616
Context: mono/mono#16689
Context: mono/mono#16701
Context: mono/mono#16712
Context: mono/mono#16742
Context: mono/mono#16759
Context: mono/mono#16803
Context: mono/mono#16808
Context: mono/mono#16824
Context: mono/mono#16876
Context: mono/mono#16879
Context: mono/mono#16918
Context: mono/mono#16943
Context: mono/mono#16950
Context: mono/mono#16974
Context: mono/mono#17004
Context: mono/mono#17017
Context: mono/mono#17038
Context: mono/mono#17040
Context: mono/mono#17083
Context: mono/mono#17084
Context: mono/mono#17133
Context: mono/mono#17139
Context: mono/mono#17151
Context: mono/mono#17180
Context: mono/mono#17278
Context: mono/mono#17549
Context: mono/mono#17569
Context: mono/mono#17665
Context: mono/mono#17687
Context: mono/mono#17737
Context: mono/mono#17790
Context: mono/mono#17924
Context: mono/mono#17931
Context: https://github.com/mono/mono/issues/26758
Context: https://github.com/mono/mono/issues/37913
Context: xamarin/xamarin-macios#7005
ManickaP pushed a commit to ManickaP/runtime that referenced this issue Jan 20, 2020
…o#17106)

* If there is a perform_wait_callback in the stack there will be another catch generated by the owner thread, so we don't need to throw, we can continue and find the next catch.
Fixes mono/mono#17083

* Reverting unit test changed on commit 405d521.

* Fixing assert when calling mono_jit_info_get_method if it was a trampoline.


Commit migrated from mono/mono@18fac0a
ManickaP pushed a commit to ManickaP/runtime that referenced this issue Jan 20, 2020
…d is enabled (mono/mono#17478)

* Removing all fixes to fix mono/mono#15203 and mono/mono#17083, and trying a new approach to fix this mono/mono#17084, together.

The thing is when we are executing on hybrid suspend the caller is executed inside try catch in the runtime_invoke_ method that is generated. And this try catch should be ignored for the debugger exception, otherwise it's the last try catch in main thread, but should always be considered to generate the stack trace.

Fixes mono/mono#17084

* This test check the case that we were missing. On preemptive this test always runs perfectly, on Hybrid, never run well until this PR.

* Changing how call unit test.
Changing variable name.


Commit migrated from mono/mono@8fda173
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.