更新资源同时避免竞争条件

Question

我正在为一个游戏客户端编写一个实用程序类。这个类有两个主要功能:连接到一个游戏服务器并检索当前的游戏版本，并检索一个“菜谱”，它基本上是序列化的数据，其中包含一个修订号特有的信息。“食谱”的细节与这个类的目的无关。

因为我的应用程序的一个实例可以支持多个游戏会话，每个会话位于不同的线程上，所以我需要这个类中的public方法是线程安全的。特别是，我关注forceUpdateRevision、getRevision和getRevision方法中的竞争条件。我对private对象进行了内部更新方法的同步，以确保只有单个线程可以同时更新，但我不确定这是否足以防止上述方法中的争用条件。我也不确定是否需要volatile修饰符在cachedRevision和cachedRecipe字段上。

例如，根据争用条件，我指的是两个线程在稍微不同的时间调用forceUpdateRevision方法，对于两个线程，updateRevision的结果是不同的。在这种情况下，我害怕不确定的行为。我假设一种解决方案是完全同步forceUpdateRevision方法，而不是底层的更新方法，但我希望最小化同步的程度，以避免通过一个方法调用锁定整个类，因为updateRevision和updateRecipe原则上完全独立于彼此。

可以让更有经验的线程安全和竞争条件的人查看我的代码，看看它是否会遇到可能的竞争条件问题吗？

为了完整起见，我已经包含了该类的整个源代码，尽管我认为分析内部更新方法的细节对我的问题没有多大帮助。

public final class Revision {

    public static final SocketAddress ADDRESS = new InetSocketAddress("game.server.address", 11111);
    public static final byte GAME_UPDATE = 1;
    public static final byte GOOD_RESPONSE = 0;
    private static final int REVISION_THRESHOLD = 150;
    private static final Object revisionLock = new Object();
    private static final Object recipeLock = new Object();
    private static volatile int cachedRevision = -1;
    private static volatile Recipe cachedRecipe;

    private Revision() {

    }

    public static void forceUpdateRevision() {
        int revision = updateRevision();
        if (revision != cachedRevision) {
            cachedRevision = revision;
            forceUpdateRecipe();
        }
    }

    public static int getRevision() {
        if (cachedRevision < 1) {
            forceUpdateRevision();
        }
        return cachedRevision;
    }

    public static void forceUpdateRecipe() {
        cachedRecipe = updateRecipe();
    }

    public static Recipe getRecipe() {
        if (cachedRecipe == null) {
            forceUpdateRecipe();
        }
        return cachedRecipe;
    }

    private static int updateRevision() {
        synchronized(revisionLock) {
            File revisionFile = new File("./data/revision.dat");
            int revision = 0;
            if (revisionFile.exists()) {
                DataInputStream in = null;
                try {
                    in = new DataInputStream(new FileInputStream(revisionFile));
                    revision = in.readShort();
                } catch (Exception ignored) {} 
                finally {
                    if (in != null) {
                        try {
                            in.close();
                        } catch (IOException ignored) {}
                    }
                }
            }
            int response = -1;
            while (revision <= REVISION_THRESHOLD) {
                Socket socket = new Socket();
                try {
                    socket.connect(ADDRESS);
                    socket.getOutputStream().write(new byte[] { 
                        GAME_UPDATE, 
                        (byte) ((revision >> 24) & 0xff),
                        (byte) ((revision >> 16) & 0xff), 
                        (byte) ((revision >> 8) & 0xff), 
                        (byte) revision });
                    response = socket.getInputStream().read();
                    if (response == GOOD_RESPONSE) {
                        break;
                    } else {
                        revision++;
                    }
                } catch (Exception ignored) {
                    break;
                } finally {
                    try {
                        socket.close();
                    } catch (IOException ignored) {}
                }
            }
            if (response != GOOD_RESPONSE) {
                return -1;
            }
            if (!createDataDirectory()) {
                return revision;
            }
            DataOutputStream out = null;
            try {
                out = new DataOutputStream(new FileOutputStream(revisionFile));
                out.writeShort((short) revision);
            } catch (Exception ignored) {} 
            finally {
                if (out != null) {
                    try {
                        out.close();
                    } catch (IOException ignored) {}
                }
            }
            return revision;
        }
    }

    private static Recipe updateRecipe() {
        synchronized(recipeLock) {
            File recipeFile = new File("./data/recipe.dat");
            if (!recipeFile.exists()) {
                return null;
            }
            DataInputStream in = null;
            try {
                in = new DataInputStream(new FileInputStream(recipeFile));
                byte[] data = new byte[(int) recipeFile.length()];
                in.readFully(data);
                return new Recipe(data);
            } catch (Exception ignored) {}
            finally {
                if (in != null) {
                    try {
                        in.close();
                    } catch (IOException ignored) {}
                }
            }
            return null;
        }
    }

    private static boolean createDataDirectory() {
        File data = new File("./data/");
        if (!data.exists()) {
            return data.mkdir();
        }
        return true;
    }
}

Answer 1

您的两个变量cachedRevision和cachedRecipe只能在同步块中访问--其他任何事情都肯定是代码的味道。仔细的分析也许能证明你是对的，但要特别小心。

将静态共享变量标记为易失性是很好的做法。将确保优化器不会做出在多线程时失败的假设。

我将在同步块中再次重复当前在同步块之外的测试。这与易失性声明一起，避免了两个线程在足够接近同一时间到达第一个测试时可能出现的争用条件。这可能是无害的，但也可能不是.

Answer 2

简单地看一下，这段代码中就出现了竞争条件。由于锁不正确(可能更糟)，您绝对可以调用您的updateRevision和updateRecipe方法。如果您必须对这些方法进行不必要的调用，它们将比添加适当的锁慢得多。

我不太清楚您的文件是如何使用/更新的--将这些文件的监视提取到单独的服务中可能是个好主意。每当其中一个文件更改时，它将自动更新，客户端将始终获得最最新的数据。您可以使用WatchService监视这些文件的更改：http://docs.oracle.com/javase/tutorial/essential/io/notification.html

每当客户端需要这些数据时，他们都会调用getter，如果文件只是更改，并且您的服务处于更新过程中，它可能会阻塞，但是很可能它几乎会立即返回最新的数据。这将取决于如何使用这些文件。这可能是个糟糕的主意哈哈..。

目标应该是尽量减少您的HDD和网络访问。从这些资源获取数据是很昂贵的。

关于这一点：

while (revision <= REVISION_THRESHOLD) {
  Socket socket = new Socket();
  try {
    socket.connect(ADDRESS);
    ...

如果您必须多次打开/关闭一个新套接字，您可能会占用一些宝贵的资源。这可能是服务器工作方式的一个工件，但如果可能的话，我将尝试连接一次并重用已经打开的连接，以避免对服务器进行重击。这只是个想法。