use background thread to wait for address changes on OSX#41768
use background thread to wait for address changes on OSX#41768wfurt merged 3 commits intodotnet:masterfrom
Conversation
| // and forces quit even when foreground threads are running. | ||
| NetworkChange.NetworkAddressChanged += _addressHandler; | ||
| { | ||
| }; |
There was a problem hiding this comment.
Hmm. I know we spoke about it offline, but seeing it now I realize this could negatively interact with other tests. Do any other tests validate the behavior when going from zero to one handlers, validate the behavior when the last handler is removed, etc.?
There was a problem hiding this comment.
I did not see any. All the tests are primitive in sense that they only verify that handler can be set but they do not test actual address changes as that is difficult to do in CI.
src/System.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.OSX.cs
Show resolved
Hide resolved
| // Register without unregistering. | ||
| // This should not block process exit. If it does, this test will pass | ||
| // but we would fail to exit test run at the end. | ||
| // We cannot test this via RemoteInvoke() as that call System.Exit() |
There was a problem hiding this comment.
| // We cannot test this via RemoteInvoke() as that call System.Exit() | |
| // We cannot test this via RemoteInvoke() as that calls Environment.Exit() |
There was a problem hiding this comment.
Indeed it even says
// Use Exit rather than simply returning the exit code so that we forcibly shut down
// the process even if there are foreground threads created by the operation that would
// end up keeping the process alive potentially indefinitely.
There was a problem hiding this comment.
I know I added this explicitly a while back, but I can't remember now why. Ideally there wouldn't be any foreground threads and this logic could go away. Worth looking into subsequently.
| @@ -181,6 +181,7 @@ private static unsafe void CreateAndStartRunLoop() | |||
| } | |||
| } | |||
| s_runLoopThread = new Thread(RunLoopThreadStart); | |||
There was a problem hiding this comment.
Nit: It is not necessary to keep s_runLoopThread in a static. It is never used for anything. This can be a local variable.
* use background thread to wait for address changes on OSX * remove RemoteExecutor * update comment
* use background thread to wait for address changes on OSX * remove RemoteExecutor * update comment
…fx#41768) * use background thread to wait for address changes on OSX * remove RemoteExecutor * update comment Commit migrated from dotnet/corefx@08bf1fe
we should not block process exit.
fixes #41740