Runtime: NetworkAddressChanged event hangs process on Mac

[ryanbrandenburg]

Repro steps

1. Install 3.0 RTM
2. dotnet new console
3. Replace Program.cs with the following:

```C#
using System;

namespace MacMinimal
{
class Program
{
static void Main(string[] args)
{
Console.WriteLine("Hello World!");
System.Net.NetworkInformation.NetworkChange.NetworkAddressChanged += (sender, e) => {

        };
    }
}

}
```

Expected Result

The process should exit.

Actual Result

The process does not exit.

Details

Thread dumps indicate that this line is the problem:

https://github.com/dotnet/corefx/blob/d3911035f2ba3eb5c44310342cc1d654e42aa316/src/System.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.OSX.cs#L184

We believe that this Thread should be marked as background so it doesn't block the process exiting.

Real world cases

We found this when we noticed that applications which use AppInsights don't close when they would normally exit on a Mac. This isn't noticed normally because the normal workflow for AspNetCore apps is that when you want the server to exit you send Ctrl-C.

CC @danmosemsft @karelz @davidfowl

[karelz]

@wfurt can you please take a look?

[danmosemsft]

It should be background. I looked at all the othre managed threads we start, and found two others that look like they should be set to background as well.

https://github.com/dotnet/corefx/blob/ec1ab377f9dfa2daa379df4eb22fbe73ebd3aa69/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs#L180
https://github.com/dotnet/corefx/blob/ec1ab377f9dfa2daa379df4eb22fbe73ebd3aa69/src/System.Net.Requests/src/System/Net/TimerThread.cs#L477

[wfurt]

For the timer, we set IsBackground = true inside of ThreadProc. For the SystemProcess, it seems like we explicitly call Join(). In this case I think user is in charge when they execute new process. I posted PR for NetworkAddressChanged. That change is simple and it should be easy to port to maintenance branches. It also hangs with 2.1 so it is not 3.0 regression.
Workaround would be to unregister handler using NetworkAddressChanged -= but that may not be practical for lambda functions.

[danmosemsft]

For the SystemProcess, it seems like we explicitly call Join(). In this case I think user is in charge when they execute new process.

I think in some cases ShellExecuteExW can jam and in that case you may well want Ctrl-C to work. For example, if your file exists but the extension is not registered with an app, the shell will pop a dialog:
c# // c:\\temp\\foo.baz exists var psi = new ProcessStartInfo() { UseShellExecute = true, FileName = @"c:\\temp\\foo.baz" }; psi.ErrorDialog = true; var p = new Process(); p.StartInfo = psi; var t = System.Threading.Thread.CurrentThread; p.Start(); // pops dialog

Having said the above, despite the ShellExecuteExW thread definitely not being Background, Ctrl-C does seem to work. I tried to debug it, but when I do that Ctrl-C doesn't work. @jkotas can you educate me why ?

I think it is best to just mark this thread background rather than rationalizing about whether it is ever needed.

[danmosemsft]

(You are right about TimerThread. I had assumed IsBackground could not be set after the thread starts, just like setting the apartment)

[jkotas]

I tried to debug it, but when I do that Ctrl-C doesn't work

Debugger tries to intercept Ctrl-C. The behavior that you are seeing may be a bug or a feature - I do not know.

[danmosemsft]

I guess I meant - in the absence of debugger, why does Ctrl-C terminate successfully when this managed thread is waiting and it's not Background.

[jkotas]

The default Ctrl-C handler behavior is to exit the process. (E.g. from https://docs.microsoft.com/en-us/windows/console/setconsolectrlhandler : Initially, the handler list for each process contains only a default handler function that calls the ExitProcess function.)

[danmosemsft]

Oh - my bad - I thoroughly confused myself due to mention of Ctrl-C at the top. Ctrl-C always works, background or not.

My original observation stands -- if I run "process.Start" it may block process exit even if I run it on a background thread: this is because if UseShellExecute causes us to spawn a thread to do the potentially indefinite call into ShellExecuteEx, we do not mark it as background. We should mark that spawned thread as background so this does not hang.

[wfurt]

I can try to craft some test with example above. That can also be separate change to keep this small for back-porting.

[danmosemsft]

Just noticed this got closed by the master fix. @ryanbrandenburg do you believe this should be backported to 3.1? The risk is very low.

[davidfowl]

Yep

[danmosemsft]

Alright, I threw up a port. https://github.com/dotnet/corefx/pull/41960

[danmosemsft]

ported