UNC path exists monitor

Question

This is my first attempt at trying to write a "monitor" class to determine if a UNC path is available. I need the monitor to not block the main thread and to also event when the UNC is toggled UP/DOWN so that I can respond to that event.

I'm looking for:

1. Best practice suggestions
2. Am I doing this properly? Is there a more efficient way?
3. Will there be threads left alive? will I eat into the pool until it starves?
4. Other comments suggestions related to this code or programming context

public class NetworkPathMonitor 
{
    public event EventHandler<PathAvailabilityChangedArgs> PathAvailabilityChanged;
    public event EventHandler Started;
    public event EventHandler Stopping;
    public event EventHandler Stopped;
    public event EventHandler CheckingStatus;

    protected virtual void OnStopping(object sender, EventArgs e)
    {
        EventHandler handler = Stopping;
        if (handler != null)
            handler(sender, e);
    }
    protected virtual void OnCheckingStatus(object sender, EventArgs e)
    {
        EventHandler handler = CheckingStatus;
        if (handler != null)
            handler(sender, e);
    }
    protected virtual void OnStopped(object sender, EventArgs e)
    {
        EventHandler handler = Stopped;
        if (handler != null)
            handler(sender, e);
    }
    protected virtual void OnStarted(object sender, EventArgs e)
    {
        EventHandler handler = Started;
        if (handler != null)
            handler(sender, e);
    }
    protected virtual void OnPathAvailabilityChanged(object sender, PathAvailabilityChangedArgs e)
    {
        EventHandler<PathAvailabilityChangedArgs> handler = PathAvailabilityChanged;
        if (handler != null)
            handler(sender, e);
    }

    private PathState CurrentState { get; set; }

    private CancellationTokenSource TokenSource { get; set; }

    private AutoResetEvent IsStopping { get; set; }

    public string PathToMonitor { get; set; }

    public int Interval { get; set; }

    public NetworkPathMonitor(string pathToMonitor, int interval, PathState expectedState = PathState.Up)
    {
        this.CurrentState = expectedState;
        this.Interval = interval;
        this.PathToMonitor = pathToMonitor;
    }

    public void Stop()
    {
        OnStopping(this, EventArgs.Empty);
        TokenSource.Cancel();
    }

    public void Start()
    {

        IsStopping = new AutoResetEvent(false);
        TokenSource = new CancellationTokenSource();

        var t = Task.Factory.StartNew(() =>
        {
            OnStarted(this, EventArgs.Empty);
            TimeSpan waitInterval = TimeSpan.FromMilliseconds(Interval);
            while (!IsStopping.WaitOne(waitInterval))
            {
                if (this.TokenSource.IsCancellationRequested)
                {
                    //Is this how to properly stop the signal?
                    IsStopping.Set();
                    IsStopping.Dispose();
                    OnStopped(this, EventArgs.Empty);
                    break;
                }

                OnCheckingStatus(this, EventArgs.Empty);
                CheckPathStatus();
            }
        }, TokenSource.Token);

    }


    private void CheckPathStatus()
    {
        //this is synchronous. We just have to wait for it to timeout.
        var currentState = Directory.Exists(PathToMonitor) ? PathState.Up : PathState.Down;

        var shouldEvent = currentState != CurrentState;
        CurrentState = currentState;

        if (shouldEvent)
        {
            OnPathAvailabilityChanged(this, new PathAvailabilityChangedArgs() { Path = PathToMonitor, PathState = CurrentState });
        }

    }
}


public enum PathState
{
    Up,
    Down
}
public class PathAvailabilityChangedArgs : EventArgs
{
    public string Path { get; set; }
    public PathState PathState { get; set; }
    public PathAvailabilityChangedArgs() { }
    public PathAvailabilityChangedArgs(string path)
    {
        Path = path;
    }
}

Answer 1

The implementation you have will forever consume one thread from the ThreadPool. That's not the end of the world, but not ideal either. An easy solution would be to use an overload of Task.Factory.StartNew that accepts a TaskCreationOptions argument, and specify TaskCreationOptions.LongRunning. That will let the system handle it appropriately. Another option would be starting a new thread yourself.

In the task delegate (the code inside StartNew()) I would put a global try/catch block. Any exceptions that might be thrown in there will cause your process to be killed. At the very least you'd want to log those somewhere just so you have some idea what's going on.

I think you're going to Dispose() the IsStopping event on the first time through the loop, then try to use it on the second time through, which will probably cause an exception. I would just remove the AutoResetEvent entirely and use CancellationTokenSource instead. You can use TokenSource.Token.WaitHandle.WaitOne() to wait on instead of the AutoResetEvent. This also has the benefit that when Stop() is called, any pending wait will end immediately.

I would make the Interval property a TimeSpan instead of an int. It's just more clear to users what it means - they don't have to wonder whether it's seconds, milliseconds, or what.

Answer 2

Just to add to breishl's answer:

• don't use private properties, just use fields instead (private CancellationTokenSource TokenSource { get; set; } => private CancellationTokenSource _tokenSource;)
• AutoResetEvent is not needed as already noted, but even now it's used incorrectly, you should have used ManualResetEvent instead (again, not needed here)
• you can also wait (await) with cancellationToken using Task.Delay(TimeSpan, CancellationToken) method.