锁定 lambda 事件处理程序

Question

我需要同步一个包含异步部分的操作序列。 该方法查看图像缓存并在图像存在时返回图像（实际上调用回调）。否则它必须从服务器下载它。下载操作是异步的，并在完成时触发一个事件。

这是（简化的）代码。

private Dictionary<string, Bitmap> Cache;

public void GetImage(string fileName, Action<Bitmap> onGetImage)
{
    if (Cache.ContainsKey(fileName))
    {
        onGetImage(Cache[fileName]);
    }
    else
    {
        var server = new Server();
        server.ImageDownloaded += server_ImageDownloaded;
        server.DownloadImageAsync(fileName, onGetImage); // last arg is just passed to the handler
    }
}

private void server_ImageDownloaded(object sender, ImageDownloadedEventArgs e)
{
    Cache.Add(e.Bitmap, e.Name);
    var onGetImage = (Action<Bitmap>)e.UserState;
    onGetImage(e.Bitmap);
}

问题：如果两个线程几乎同时调用GetImage，它们都会调用服务器并尝试将相同的图像添加到缓存中。我应该做的是在 GetImage 的开头创建锁并在 server_ImageDownloaded 处理程序结束时释放它。

显然，这对于 lock 构造是不可行的，并且没有意义，因为在任何情况下都很难确保释放锁。

现在我认为我可以做的是使用 lambda 而不是事件处理程序。这样我就可以锁定整个部分：

我必须在 DownloadImage 方法的开头锁定 Cache 字典，并仅在 ImageDownloaded 事件处理程序结束时释放它。

private Dictionary<string, Bitmap> Cache;

public void GetImage(string fileName, Action<Bitmap> onGetImage)
{
    lock(Cache)
    {
        if (Cache.ContainsKey(fileName))
        {
            onGetImage(Cache[fileName]);
        }
        else
        {
            var server = new Server();
            server.ImageDownloaded += (s, e) =>
            {
                Cache.Add(e.Bitmap, e.Name);
                onGetImage(e.Bitmap);
            }
            server.DownloadImageAsync(fileName, onGetImage); // last arg is just passed to the handler
        }
    }
}

这样安全吗？还是在GetImage执行后立即释放锁，让lambda表达式解锁？

有没有更好的方法来解决这个问题？

---

解决方案

最后，解决方案是所有答案和 cmets 的混合，不幸的是，我无法将所有答案都标记为答案。所以这是我的最终代码（为了清楚起见，删除了一些空检查/错误案例/等）。

private readonly object ImageCacheLock = new object();
private Dictionary<Guid, BitmapImage> ImageCache { get; set; }
private Dictionary<Guid, List<Action<BitmapImage>>> PendingHandlers { get; set; }

public void GetImage(Guid imageId, Action<BitmapImage> onDownloadCompleted)
{
    lock (ImageCacheLock)
    {
        if (ImageCache.ContainsKey(imageId))
        {
            // The image is already cached, we can just grab it and invoke our callback.
            var cachedImage = ImageCache[imageId];
            onDownloadCompleted(cachedImage);
        }
        else if (PendingHandlers.ContainsKey(imageId))
        {
            // Someone already started a download for this image: we just add our callback to the queue.
            PendingHandlers[imageId].Add(onDownloadCompleted);
        }
        else
        {
            // The image is not cached and nobody is downloading it: we add our callback and start the download.
            PendingHandlers.Add(imageId, new List<Action<BitmapImage>>() { onDownloadCompleted });
            var server = new Server();
            server.DownloadImageCompleted += DownloadCompleted;
            server.DownloadImageAsync(imageId);
        }
    }
}

private void DownloadCompleted(object sender, ImageDownloadCompletedEventArgs e)
{
    List<Action<BitmapImage>> handlersToExecute = null;
    BitmapImage downloadedImage = null;

    lock (ImageCacheLock)
    {
        if (e.Error != null)
        {
            // ...
        }
        else
        {
            // ...
            ImageCache.Add(e.imageId, e.bitmap);
            downloadedImage = e.bitmap;
        }

        // Gets a reference to the callbacks that are waiting for this image and removes them from the waiting queue.
        handlersToExecute = PendingHandlers[imageId];
        PendingHandlers.Remove(imageId);
    }

    // If the download was successful, executes all the callbacks that were waiting for this image.
    if (downloadedImage != null)
    {
        foreach (var handler in handlersToExecute)
            handler(downloadedImage);
    }
}

Answer 1

lambda 表达式被转换为一个锁内的委托，但是 lambda 表达式的 body 将不会自动获取Cache 监视器的锁，当代表被执行。所以你可能想要：

server.ImageDownloaded += (s, e) =>
{
    lock (Cache)
    {
        Cache.Add(e.Bitmap, e.Name);
    }
    onGetImage(e.Bitmap);
}

Answer 2

这里还有一个潜在的问题。这段代码：

if (Cache.ContainsKey(fileName))
{
    onGetImage(Cache[fileName]);
}

如果其他线程在您调用ContainsKey 之后但在执行下一行之前从缓存中删除了图像，它将崩溃。

如果您在并发线程可以读写的多线程上下文中使用Dictionary，那么您需要使用某种锁来保护每个访问。 lock 很方便，但ReaderWriterLockSlim 会提供更好的性能。

我还建议您将上面的代码重新编码为：

Bitmap bmp;
if (Cache.TryGetValue(fileName, out bmp))
{
    onGetImage(fileName);
}

如果您运行的是 .NET 4.0，那么我强烈建议您考虑使用 ConcurrentDictionary。

Answer 3

您为什么不保留一个正在下载的图像文件名的集合，并将线程代码设为：

public void GetImage(string fileName, Action<Bitmap> onGetImage) 
{ 
    lock(Cache) 
    { 
        if (Cache.ContainsKey(fileName)) 
        { 
            onGetImage(Cache[fileName]); 
        } 
        else if (downloadingCollection.contains(fileName))
        {
            while (!Cache.ContainsKey(fileName))
            {
                System.Threading.Monitor.Wait(Cache)
            }
            onGetImage(Cache[fileName]); 
        }
        else 
        { 
           var server = new Server(); 
           downloadCollection.Add(filename);
           server.ImageDownloaded += (s, e) => 
           { 
              lock (Cache)
              {
                  downloadCollection.Remove(filename);
                  Cache.Add(e.Bitmap, e.Name); 
                  System.Threading.Monitor.PulseAll(Cache);
              }
              onGetImage(e.Bitmap);

           } 
           server.DownloadImageAsync(fileName, onGetImage); // last arg is just passed to the handler 
        } 
    } 
}

这或多或少是标准监视器模式，或者如果您将 lambda 表达式重构为像 GetImage 这样的成员函数。你真的应该这样做。这将使监控逻辑更容易推理。