c# - Task.ContinueWith 中的嵌套锁 - 安全，还是玩火？

Question

Windows 服务:从要在配置文件中监视的目录列表生成一组 FileWatcher 对象，具有以下要求:

文件处理可能很耗时 - 事件必须在它们自己的任务线程上处理

保留事件处理程序任务的句柄以在 OnStop() 事件中等待完成。

跟踪上传文件的哈希值；如果没有不同，请不要重新处理

保留文件哈希以允许 OnStart() 在服务关闭时处理上传的文件。

永远不要多次处理一个文件。

(关于#3，我们确实会在没有变化的情况下获取事件……最显着的原因是 FileWatchers 的重复事件问题)

为了做这些事情，我有两本词典——一本用于上传的文件，一本用于任务本身。这两个对象都是静态的，我需要在添加/删除/更新文件和任务时锁定它们。简化代码:

public sealed class TrackingFileSystemWatcher : FileSystemWatcher {

    private static readonly object fileWatcherDictionaryLock = new object();
    private static readonly object runningTaskDictionaryLock = new object();

    private readonly Dictionary<int, Task> runningTaskDictionary = new Dictionary<int, Task>(15);
    private readonly Dictionary<string, FileSystemWatcherProperties>  fileWatcherDictionary = new Dictionary<string, FileSystemWatcherProperties>();

    //  Wired up elsewhere
    private void OnChanged(object sender, FileSystemEventArgs eventArgs) {
        this.ProcessModifiedDatafeed(eventArgs);
    }

    private void ProcessModifiedDatafeed(FileSystemEventArgs eventArgs) {

        lock (TrackingFileSystemWatcher.fileWatcherDictionaryLock) {

            //  Read the file and generate hash here

            //  Properties if the file has been processed before
            //  ContainsNonNullKey is an extension method
            if (this.fileWatcherDictionary.ContainsNonNullKey(eventArgs.FullPath)) {

                try {
                    fileProperties = this.fileWatcherDictionary[eventArgs.FullPath];
                }
                catch (KeyNotFoundException keyNotFoundException) {}
                catch (ArgumentNullException argumentNullException) {}
            }
            else {  
                // Create a new properties object
            }


            fileProperties.ChangeType = eventArgs.ChangeType;
            fileProperties.FileContentsHash = md5Hash;
            fileProperties.LastEventTimestamp = DateTime.Now;

            Task task;
            try {
                task = new Task(() => new DatafeedUploadHandler().UploadDatafeed(this.legalOrg, datafeedFileData), TaskCreationOptions.LongRunning);
            }
            catch {
              ..
            }

            //  Only lock long enough to add the task to the dictionary
            lock (TrackingFileSystemWatcher.runningTaskDictionaryLock) {
                 try {
                    this.runningTaskDictionary.Add(task.Id, task);  
                }
                catch {
                  ..
                }    
            }


            try {
                task.ContinueWith(t => {
                    try {
                        lock (TrackingFileSystemWatcher.runningTaskDictionaryLock) {
                            this.runningTaskDictionary.Remove(t.Id);
                        }

                        //  Will this lock burn me?
                        lock (TrackingFileSystemWatcher.fileWatcherDictionaryLock) {
                            //  Persist the file watcher properties to
                            //  disk for recovery at OnStart()
                        }
                    }
                    catch {
                      ..
                    }
                });

                task.Start();
            }
            catch {
              ..
            }


        }

    }

}

请求锁定 ContinueWith() 中的 FileSystemWatcher 集合有什么影响？当委托(delegate)在同一个对象的锁中定义时？我希望它没问题，即使任务开始、完成并在 ProcessModifiedDatafeed() 之前进入 ContinueWith() |释放锁，任务线程将简单地挂起，直到创建线程释放锁。但我想确保我没有踩到任何延迟执行的地雷。

查看代码，我可能能够更快地释放锁，避免出现问题，但我还不确定……需要查看完整代码才能确定。

更新

为了阻止越来越多的“这段代码很糟糕”的评论，我有很好的理由来捕捉我所做的异常，并且捕捉到如此多的异常。这是一个带有多线程处理程序的 Windows 服务，它可能不会崩溃。曾经。如果这些线程中的任何一个有未处理的异常，它就会这样做。

此外，这些异常(exception)被写入 future 的防弹。我在下面的评论中给出的例子是为处理程序添加一个工厂......正如今天编写的代码一样，永远不会有空任务，但如果工厂没有正确实现，代码可能会抛出异常.是的，这应该在测试中发现。然而，我的团队中有初级开发人员......“可能。不会。崩溃。” (此外，如果有未处理的异常，它必须正常关闭，允许当前运行的线程完成 - 我们使用 main() 中设置的未处理的异常处理程序来完成)。我们将企业级监视器配置为在事件日志中出现应用程序错误时发送警报——这些异常将记录并标记我们。该方法是经过深思熟虑和讨论的决定。

每个可能的异常都经过仔细考虑并选择归入两类之一 - 适用于单个数据馈送并且不会关闭服务的那些(大多数)，以及那些表明明显的编程或其他错误从根本上呈现代码对所有数据馈送都没用。例如，我们选择在无法写入事件日志时关闭服务，因为这是我们指示数据馈送未得到处理的主要机制。异常是在本地捕获的，因为本地上下文是唯一可以做出继续决定的地方。此外，允许异常冒泡到更高级别 (1) 违反了抽象的概念，并且 (2) 在工作线程中没有意义。

我对反对处理异常的人数之多感到惊讶。如果我每次尝试都有一角钱..catch(Exception){do nothing} 我明白了，你会在永恒的剩余时间里得到五分钱的零钱。我会争辩说如果对 .NET 框架的调用或您自己的代码抛出异常，您需要考虑会导致该异常发生的场景并明确决定应该如何处理它。我的代码在 IO 操作中捕获了 UnauthorizedExceptions，因为当我考虑可能发生的情况时，我意识到添加新的 datafeed 目录需要向服务帐户授予权限(默认情况下不会有权限)。

我很欣赏 build 性的意见......只是请不要用广泛的“这很糟糕”画笔批评简化的示例代码。代码并不糟糕 - 它是防弹的，而且必然如此。

1 如果 Jon Skeet 不同意，我只会争论很长时间

Answer 1

首先，您的问题是:在 ContinueWith 中请求锁定本身不是问题。如果你打扰你在另一个锁块中这样做 - 只是不要。您的延续将异步执行，在不同的时间，不同的线程。

现在，代码本身是有问题的。为什么在几乎不能抛出异常的语句周围使用许多 try-catch 块？例如这里:

 try {
     task = new Task(() => new DatafeedUploadHandler().UploadDatafeed(this.legalOrg, datafeedFileData), TaskCreationOptions.LongRunning);
 }
 catch {}

你只是创建任务 - 我无法想象这什么时候会抛出。与 ContinueWith 相同的故事。这里:

this.runningTaskDictionary.Add(task.Id, task); 

您可以检查此类 key 是否已存在。但即使这样也没有必要，因为 task.Id 是您刚刚创建的给定任务实例的唯一 ID。这个:

try {
    fileProperties = this.fileWatcherDictionary[eventArgs.FullPath];
}
catch (KeyNotFoundException keyNotFoundException) {}
catch (ArgumentNullException argumentNullException) {}

更糟。你不应该使用异常 - 不要捕获 KeyNotFoundException 而是在 Dictionary 上使用适当的方法(如 TryGetValue)。

因此，首先，删除所有 try catch 块，并为整个方法使用一个，或者将它们用于可能真正抛出异常的语句，否则您无法处理这种情况(并且您知道如何处理抛出的异常)。

然后，您处理文件系统事件的方法不太可扩展和可靠。许多程序在将更改保存到文件时会在很短的时间间隔内生成多个更改事件(对于同一文件也有多个事件按顺序进行的其他情况)。如果你只是开始处理每个事件的文件，这可能会导致不同类型的麻烦。因此，您可能需要限制给定文件的事件，并且仅在上次检测到更改后的特定延迟后才开始处理。不过，这可能有点高级。

不要忘记尽快获取文件的读锁，以便其他进程在您处理文件时无法更改文件(例如，您可能会计算文件的 md5，然后有人更改文件，然后您开始正在上传 - 现在您的 md5 无效)。其他方法是记录上次写入时间和上传时间 - 获取读取锁并检查文件是否在两者之间没有更改。

更重要的是一次可以有很多变化。假设我非常快速地复制了 1000 个文件 - 您不想用 1000 个线程一次开始上传它们。您需要处理一个文件队列，并使用多个线程从该队列中获取项目。这样，成千上万的事件可能会同时发生，您的上传仍然可以可靠地工作。现在，您为每个更改事件创建新线程，在那里您立即开始上传(根据方法名称) - 这将在严重的事件负载下失败(以及在上述情况下)。