为什么同步不起作用?

Why is synchronized not working?

我正在尝试编写一个请求设备输入然后接受响应的方法,全部作为原子操作。

这是我的代码(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

这怎么可能?方法 locked/synchronized 不是对象上的吗?该对象显然与打印显示的相同,但不知何故同时执行了两个 query 方法。

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

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

wait 释放锁在 Object#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
 }