这是同步代码块的安全方法吗？

Posted 2023-04-13

【中文标题】这是同步代码块的安全方法吗？

【英文标题】：Is this a safe way to synchronize a block of code?

【发布时间】：2019-08-16 14:26:46

【问题描述】：

在我们的应用程序中，有一段代码会持续读取和调整文件。只是为了让您了解正在发生的事情：

public void run() 
    try 
        while(true)  //Yeah, I know...
            Path currentFileName = getNextFile();
            String string = readFile(currentFileName);
            Files.deleteFile(currentFileName);
            string = string.replaceAll("Hello", "Blarg");
            writeFile(currentFileName);
        
     catch (Exception e) 
        System.err.println("It's all ogre now.");
        e.printStackTrace(System.err);
    

我们代码中的其他地方有一个方法，它可能——但通常——不会在与上述代码相同的线程上运行，我们用它来退出应用程序。

private void shutdown() 
    if(fileReader != null)
        fileReader = null;

    System.exit(0); //Don't blame me, I didn't write this code

很明显，这段代码中存在潜在的竞争条件，如果在检索文件和写回文件之间调用shutdown()，则可能会导致文件完全丢失。这显然是不受欢迎的行为。

此代码存在一千个问题（超出了我在此处显示的范围），但我需要解决的主要问题是处理文件的不良行为可能在中途被切断而没有追索权。我提出的解决方案包括简单地将 while 循环包装在 synchronized 块中，并在 shutdown 中的 System.exit 调用周围放置一个块。

所以我修改后的代码如下所示：

private Object monitor = new Object();
public void run() 
    try 
        while(true) 
            synchronized(monitor) 
                Path currentFileName = getNextFile();
                String string = readFile(currentFileName);
                Files.deleteFile(currentFileName);
                string = string.replaceAll("Hello", "Blarg");
                writeFile(currentFileName);
            
        
     catch (Exception e) 
        System.err.println("It's all ogre now.");
        e.printStackTrace(System.err);
    


private void shutdown() 
    synchronized(monitor) 
        if(fileReader != null)
            fileReader = null;

         System.exit(0);
    

我最担心的是System.exit(0); 通话，我不确定通话幕后的整体行为。是否存在System.exit 的副作用会释放monitor 上的锁定的风险，从而有可能在System.exit 导致JVM 停止之前部分执行run 中的循环内容？或者这段代码会保证执行不会在处理单个文件的过程中尝试关闭？

注意：在一些闲散的程序员介入替代方案之前，我想指出，我在这里放的是大约 4000 行代码的截断版本，所有代码都隐藏在一个类中。是的，这太可怕了。是的，这让我后悔自己选择的职业。我不是在这里寻找这个问题的替代解决方案，我只是想确定这个特定的解决方案是否有效，或者是否有一些严重的缺陷会阻止它按我的预期工作。

【问题讨论】：

如何在shutdown 中设置一个布尔值并在run 中检查它以查看JVM 是否即将退出？

多线程做读/写的事情吗？

（另外：在System.exit 之前将变量设置为null 可能是不必要的）。

@GhostCat 我没有接受答案，因为没有一个答案试图证明他们的主张，比如引用文档或实施细节。我不愿意接受这样的问题的答案，如果仅仅运行代码并不能证明解决方案的合法性，如果答案只声称“这会起作用！”没有证实这一说法。

【参考方案1】：

或者此代码是否会保证执行不会在处理单个文件的过程中尝试关闭？

此代码保证在处理单个文件的过程中不会发生关闭在此处启动。也许很明显，您的代码中的其他地方可能会调用System.exit，而您对此没有任何保护。

您可能需要考虑不调用 preventing System.exit，然后让您的代码正常关闭（即通过正常完成 main 方法）。

【讨论】：

如果简单地消除System.exit 是一个真正的选择，我很乐意接受。不幸的是，除非我获得完全修改代码的许可，否则我无法做出这个决定——而就目前而言，这不太可能。

【参考方案2】：

如果你真的有多个线程调用不同的方法，像这样使用synchronized 实际上是一个聪明的主意，因为它可以处理“多线程”的事情。

您可以考虑缩小第一个块的范围：

Path currentFileName = getNextFile();
String string = readFile(currentFileName);
synchronized(monitor) 

读取文件本身应该不是问题。 （当然，除非您的代码必须保证getNextFile() 返回的路径得到完全处理）。

【参考方案3】：

如果代码在synchronized 块中执行并且块在同一个对象上同步，并且在while 循环中的synchronized 块中调用的方法完全在与它们的调用者相同的线程，则文件相关的进程不会被System.exit 的调用中断。

也就是说，它看起来确实像一个有争议的补丁，只是稍微改进了有争议的代码。

还可能存在更具体的饥饿风险，因为显示的 while 循环似乎只是尽可能快地向这些文件操作发送垃圾邮件，因此在退出时获取锁的尝试可能不会成功。

探索的一般方向是将无限 while 循环转换为 ScheduledExecutorService + Runnable，每隔 x 时间使用自己的监视器执行一次，以防止对同一文件的重叠操作，并在调用 shutdown 方法时优雅地终止它。

【讨论】：

“它看起来确实像一个有争议的补丁，只是稍微改进了有争议的代码。” 你没有半点错误。如果我们有资金彻底检修并重做整个程序，我会改为这样做。

【参考方案4】：

您可以使用关闭挂钩。来自 javadocs：

关闭挂钩只是一个已初始化但未启动的线程。当虚拟机开始其关闭序列时，它将以某种未指定的顺序启动所有已注册的关闭挂钩并让它们同时运行。

https://docs.oracle.com/javase/7/docs/api/java/lang/Runtime.html#addShutdownHook(java.lang.Thread)

由此，您可以从文件类中提供一个关闭挂钩，如下所示：

public Thread getShutdownHook() 
    return new Thread(() -> 
        synchronized (monitor) 
            // gracefully handle the file object
        
    );

这将在调用Runtime.getRuntime().exit() 时调用（由System.exit() 调用）。由于它也在监视器对象上同步，如果文件正在被其他线程使用，则关闭挂钩将阻塞，直到它空闲为止。

【讨论】：

这个答案对我的问题没有帮助，因为“文件对象的优雅处理”不是我要修复的缺陷。我要解决的是 run 的虚假执行，当调用 System.shutdown(0) 时，它可能会或可能不会在其内部循环的迭代中进行。

【参考方案5】：

即使您处理并发System.exit()，仍然存在其他可能导致执行突然中断的原因，例如断电或硬件错误。

所以你最好不要在新版本写入之前删除文件。执行以下操作：

Path currentFileName = getNextFile();
Path tmpFileName = currentFileName+suffix();
String string = readFile(currentFileName);
string = string.replaceAll("Hello", "Blarg");
writeFile(tmpFileName);
Files.move(tmpFileName, currentFileName);

【讨论】：

正如我在问题中强调的那样，这段代码只是实际发生的事情的精简版。 实际上发生的事情包括 1) 从消息队列读取，2) 从数据库读取（然后写入），3) 进行 Web 服务调用，4) 在多个联网设备上执行文件 I/O .虽然您是正确的，此代码可能因电源故障等其他原因而失败，但解决该问题绝不是我要解决的问题的有效解决方案。