java - 为什么同步不起作用？

Question

我正在尝试编写一种方法，该方法向设备请求输入，然后接受响应，所有这些都是原子操作。

这是我的代码(query 方法才是真正应该关注的):

public class DeviceConnection implements Runnable{
    //For query
    static int test = 0;

    //For writeline
    static PrintWriter out = null; //(initialized in constructor)

    //Only important for readline
    static String[] systemMessage=new String[10];
    static int messageIn=0;
    static int messageOut=0;
    static boolean connected = false;
    static boolean endConnect = true;

    static PrintStream logWriter; //(initialized in constructor)
    static String serverName="";//(initialized in constructor)
    static int socketNum;//(initialized in constructor)

    /** REALLY ONLY NEED TO LOOK AT THIS METHOD
         * Atomic operation to ask for data from device
         * and get a response.
         * @param line -    query to be passed to device
         * @return response from device
         */
    public synchronized String query(String line){
        int temp = test;
        System.err.print("foo" + test);
        System.err.print(this);
        String response;
        writeLine(line);
        response = readLine();
        System.err.println("bar" + test + " should be " + temp);
        test = temp+1;
        return response;
    }


/**
     * Writes a query to the device. 
     *<p>
     * Does <b>not</b> get or handle any response
     * @param line -    query to be passed to device
     */
    public synchronized void writeLine(String line) {
        out.println(line + "\n");
    }

/**
     * Reads a response from device.
     *<p>
     * Should never be used outside of <code>query</code>
     * as this could create a race condition if another
     * thread is writing to the device at the same time.
     * @return
     */
    private synchronized String readLine() {

        String response;
        long start, stop;

        if (messageIn != messageOut) { // new message exists
            response = systemMessage[messageOut];
            messageOut = (messageOut + 1) % 10;
        } else {
            start = System.currentTimeMillis();
            try {
                if (connected) { // if connected wait for heatbeats
                    //wait(15000);
                    wait();
                    start = System.currentTimeMillis();
                } else { // if not connected ignore heartbeats
                    wait();
                    start = System.currentTimeMillis();
                }
            } catch (InterruptedException e) { return "Interrupted"; }
            stop = System.currentTimeMillis();

            if (stop - start < 12000) { // heart beats running at 5 s
                if (messageIn != messageOut) { // new message exists
                    response = systemMessage[messageOut];
                    messageOut = (messageOut + 1) % 10;
                } else {
                    return null;
                }
            } else { // heart beats lost
                response = "Heart beats lost";
                logWriter.println(response);
                if (connected) { // connection lost on client side
                    logWriter.println("Connection to " + serverName + 
                                      " lost on client side");
                    sleep(60000);
                    connect(serverName,socketNum);
                }
            }
        }
        return response;
    }
}

通常 query 方法运行良好，但有时我会得到如下输出:

foo59lib.DeviceConnection@7bd0bf6d(other System.out stuff printed here)
foo59lib.DeviceConnection@7bd0bf6dbar59 should be 59
bar60 should be 59

这怎么可能？对象上的方法没有锁定/同步吗？该对象显然与打印显示的相同，但不知何故两个 query 方法同时执行。

Answer 1

从 query 调用的 readLine 方法会调用 wait，从而释放锁，这将使另一个线程能够调用 query同时。

您应该始终使用条件变量在循环内调用wait，使用 if 来决定是否等待的模式是有缺陷的。一旦您的线程重新获取锁，它就需要检查当前状态。

Object#wait 的文档中解释了 wait 释放锁的情况。 :

The current thread must own this object's monitor. The thread releases ownership of this monitor and waits until another thread notifies threads waiting on this object's monitor to wake up either through a call to the notify method or the notifyAll method. The thread then waits until it can re-obtain ownership of the monitor and resumes execution.

As in the one argument version, interrupts and spurious wakeups are possible, and this method should always be used in a loop:

 synchronized (obj) {
     while (<condition does not hold>)
         obj.wait();
     ... // Perform action appropriate to condition
 }