I am creating a piece of code that gets a webpage from a legacy system we have. In order to avoid excessive querying, I am caching the obtained URL. I am using Monitor.Enter, Monitor.Exit and double checking to avoid that request is issued twice, but when releasing the lock with Monitor.Exit, I am getting this exception:
System.Threading.SynchronizationLockException was caught
HResult=-2146233064
Message=Object synchronization method was called from an unsynchronized block of code.
Source=MyApp
StackTrace:
at MyApp.Data.ExProvider.<OpenFeature>d__0.MoveNext() in c:\Users\me\Documents\Visual Studio 2013\Projects\MyApp\MyApp\Data\ExProvider.cs:line 56
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
at MyApp.Data.ExProvider.<GetSupportFor>d__15.MoveNext() in c:\Users\me\Documents\Visual Studio 2013\Projects\MyApp\MyApp\Data\ExProvider.cs:line 71
InnerException:
The line 56 is the Monitor.Exit. This is the code that performs the operation:
private async Task<Stream> OpenReport(String report)
{
var file = _directory.GetFiles(report+ ".html");
if (file != null && file.Any())
return file[0].OpenRead();
else
{
try
{
Monitor.Enter(_locker);
FileInfo newFile = new FileInfo(Path.Combine(_directory.FullName, report + ".html"));
if (!newFile.Exists) // Double check
{
using (var target = newFile.OpenWrite())
{
WebRequest request = WebRequest.Create(BuildUrl(report));
var response = await request.GetResponseAsync();
using (var source = response.GetResponseStream())
source.CopyTo(target);
}
}
return newFile.OpenRead();
}
finally
{
Monitor.Exit(_locker);
}
}
}
So what is the problem with await and Monitor? Is it because it is not the same thread when Monitor.Enter than when Monitor.Exit?
You can't await a task inside a lock scope (which is syntactic sugar for Monitor.Enter and Monitor.Exit). Using a Monitor directly will fool the compiler but not the framework.
async-await has no thread-affinity like a Monitor does. The code after the await will probably run in a different thread than the code before it. Which means that the thread that releases the Monitor isn't necessarily the one that acquired it.
Either don't use async-await in this case, or use a different synchronization construct like SemaphoreSlim or an AsyncLock you can build yourself. Here's mine: https://stackoverflow.com/a/21011273/885318
In SendRequest however, I need to 'await', and thus I'm unable to use lock for some reason I didn't give much thought, so the solution to synchronize is to use Monitor.
Should have given it more thought. :)
There are two problems with using blocking locks with async code.
The first problem is that - in the general case - an async method may resume executing on a different thread. Most blocking locks are thread-affine, meaning that they must be released from the thread that owns them (the same thread that acquired the lock). It is this violation of Monitor thread-affinity that causes the SynchronizationLockException. This problem does not happen if the await captures an execution context (e.g., a UI context) and used that to resume the async method (e.g., on the UI thread). Or if you just got lucky and the async method happened to resume on the same thread pool thread.
However, even if you avoid the first problem, you still have a second problem: any arbitrary code can execute while an async method is "paused" at an await point. This is a violation of a cardinal rule of locking ("do not execute arbitrary code while holding a lock"). For example, thread-affine locks (including Monitor) are generally re-entrant, so even in the UI thread scenario, when your async method is "paused" (and holding the lock), other methods running on the UI thread can take the lock without any problems.
On Windows Phone 8, use SemaphoreSlim instead. This is a type that allows both blocking and asynchronous coordination. Use Wait for a blocking lock and WaitAsync for an asynchronous lock.
You may use interlocked class to simulate the lock statement, here is the code:
private async Task<Stream> OpenReport(String report)
{
var file = _directory.GetFiles(report + ".html");
if (file != null && file.Any())
return file[0].OpenRead();
else
{
object locker = _locker;
try
{
while (locker == null || Interlocked.CompareExchange(ref _locker, null, locker) != locker)
{
await Task.Delay(1);
locker = _locker;
}
FileInfo newFile = new FileInfo(Path.Combine(_directory.FullName, report + ".html"));
if (!newFile.Exists) // Double check
{
using (var target = newFile.OpenWrite())
{
WebRequest request = WebRequest.Create(BuildUrl(report));
var response = await request.GetResponseAsync();
using (var source = response.GetResponseStream())
source.CopyTo(target);
}
}
return newFile.OpenRead();
}
finally
{
_locker = locker;
}
}
}
