In my current project I am working with Boo/Rhino DSL (what a great thing(s) by the way).
In digging in the code I came across the following piece of code:
engine.Cache.WriteLock( () =>
{
engine.Storage.NotifyOnChange(urls, delegate(string invalidatedUrl)
{
engine.Cache.Remove(invalidatedUrl);
if (!standAloneCompilation.Contains(invalidatedUrl))
standAloneCompilation.Add(invalidatedUrl);
});
});
the intention here is pretty clear: the engine.Cache
has to be protected from race condition when a url is removed from it. The problem I see here is that what is really protected is a call to the Storage.NotifyOnChange
- not the Cache.Remove
.
And all NotifyOnChange
does is taking the supplied delegate and attach it as an event handler to the 'FileWatcher' it creates. So instead of protecting the Cache.Remove
the write lock here protects creating the the FileWatcher, and leaves the Cache.Remove
unprotected.
I have great respect to both Boo and Rhino, which makes me wonder - am missing something here? or the write lock should be really moved inside the delegate?
Here is the NotifyOnChange code if you are wondering:
public virtual void NotifyOnChange(IEnumerable<string> urls, Action<string> action)
{
lock (pathToFileWatchers)
{
string[] commonPaths = GatherCommonPaths(urls);
foreach (string path in commonPaths)
{
FileSystemWatcher watcher;
if(pathToFileWatchers.TryGetValue(path, out watcher)==false)
{
pathToFileWatchers[path] = watcher = new FileSystemWatcher(path, FileNameFormat);
watcher.EnableRaisingEvents = true;
}
watcher.Changed += delegate(object sender, FileSystemEventArgs e)
{
actio开发者_C百科n(e.FullPath);
};
}
}
}
精彩评论