I'm making a Windows service that watches a directory for new files to process them.
I hit a race condition between the file copying going on and me trying to process it. Going by a SO answer I wrote the following wait-and-retry method to make sure I can work with the file:
public static void WaitForFile(string file, CancellationToken token, int retryIntervalMillis = 200)
{
while (!token.IsCancellationRequested)
{
try
{
using (new FileStream(file, FileMode.Open, FileAccess.Read, FileShare.None))
return;
}
catch
{
Thread.Sleep(retryIntervalMillis);
}
}
throw new OperationCanceledException(token);
}
My question is: would this be a correct way to make this waiting cancellable? The token is cancelled to shut down the service, the idea is to allow for a clean (-ish) shutdown even in a case a bug means the file in question is inadvertently kept open indefinitely.
(Or if the reason the file can't be opened is some other, non-intermittent condition I haven't guarded against yet.)
For instance, is it okay to use the token this way in my method, and also expect the same token to cancel a BlockingCollection
's consuming enumerator?
My intent is to base the service around the expectation it can be hard-crashed at any time, so I'm fine with an exception taking out most of the call stack leading to it.
Edit:
These are changes that I have since made to the code
const int ErrorSharingViolation = -2147024864;
public static FileStream WaitForFile(string file,
CancellationToken token,
FileMode mode = FileMode.Open,
FileAccess access = FileAccess.Read,
Action<Stream> action = null,
int retryIntervalMillis = 200)
{
while (!token.IsCancellationRequested)
{
try
{
var stream = File.Open(file, mode, access, FileShare.None);
if (action != null)
{
using (stream)
{
action(stream);
}
}
return stream;
}
catch (IOException exception)
{
if (exception.HResult != ErrorSharingViolation)
{
throw;
}
else
{
Thread.Sleep(retryIntervalMillis);
}
}
}
throw new OperationCanceledException(token);
}
using
is there – the file is opened, then immediately closed again so the caller can open it. I've since changed it to actually invoke a callback instead to avoid the (in my use case unlikely) race condition where another process may open the file betweenWaitForFile()
closing and the caller opening it. – millimoose Jan 8 '14 at 18:16