Except all that @JAD said, which I totally agree with. I will give me five cents also after taking your comment in mind.
I want to cancel the other pings when one of the ping responses is successful. This should be the default behaviour - so I don't want to expose this implementation detail to the other clients.
First and foremost, there is a much cleaner and concise way of canceling your Parallel.ForEach. Check ParallelLoopState.Break method, I believe it does exactly the same thing you want to achieve without polluting your class with any unnecessary to business logic stuff - like CancellationTokenSource or ParallelOptions.
Second, you probably forgot to put Ping class in using statement. Ping class inherits IDisposable type which means that it will be a good practice to put it in using statement.
Third, I don't mind having fancy stuff like volatile field, but if I want to use something that fancy I want my code to "scream out loud" - "Hey, this makes sense here and it's probably the only way of achieving it." However, this is not the case here. It is not obvious to me why this property is static in the first place (perhaps the example is not good enough) also I don't see why it is volatile as well.
Although the volatile keyword can help you in thread safety in certain situations, it is not a solution to all of your thread concurrency issues. You should know that marking a variable or an object as volatile does not mean you don’t need to use the lock keyword. The volatile keyword is not a substitute for the lock keyword. It is only there to help you avoid data conflicts when you have multiple threads trying to access the same data.1
You are never accessing _result in your multithreaded code (unless I am wrong), you only try to assign it. With the same success, you can remove volatile keyword and the code will work as intended (again, unless I am missing something) because you are not changing the state. In other words, you don't care if _result is set to true one time, two times or three times and in what order simply because this is your business logic - you only need to know if at least one host is available.
With all the comments above the code looks like this
using System; using System.Net.NetworkInformation; using System.Threading.Tasks; namespace ConsoleApp1 { class Program { private bool _result = false; private readonly string[] _hostsToPing = { "netflix.com", "google.com", "reddit.com" }; private bool IsOneOfTheHostsAvailable() { Parallel.ForEach(_hostsToPing, (host, state) => { if (state.ShouldExitCurrentIteration) return; // <-- personal preference // I just hate .NET world putting brackets and new lines for nothing using (var ping = new Ping()) { var reply = ping.Send(host); if (reply?.Status == IPStatus.Success) { _result = true; Console.WriteLine($"Pinged {host}"); state.Break(); } } }); return _result; // you can even further remove _result by simply returning // IsCompleted property of ParallelLoopResult but many will argue this is more // readable so it's up to you } private static void Main() { var program = new Program(); var oneOfTheHostsIsAvailable = program.IsOneOfTheHostsAvailable(); Console.WriteLine($"One of the hosts is available: {oneOfTheHostsIsAvailable}"); } } }
In the end, I will try to emphasize one thing - Don't use features you don't fully understand. This might not be the case for you but maybe the case for others, it is better that you do something in 50 lines of code but fully understands it than coding it in 5-6 lines with fancy stuff but then don't know why it works. Time will pass, you will gain more experience and you will eventually come to the place where you will do it in 5-6 lines, but don't try to force this process.
