patterncsharpMinor
UNC path exists monitor
Viewed 0 times
monitorpathuncexists
Problem
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:
```
public class NetworkPathMonitor
{
public event EventHandler 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 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
I'm looking for:
- Best practice suggestions
- Am I doing this properly? Is there a more efficient way?
- Will there be threads left alive? will I eat into the pool until it starves?
- Other comments suggestions related to this code or programming context
```
public class NetworkPathMonitor
{
public event EventHandler 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 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
Solution
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
In the task delegate (the code inside
I think you're going to
I would make the
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.Context
StackExchange Code Review Q#19868, answer score: 3
Revisions (0)
No revisions yet.