Why are two synchronized blocks acting like I've provided different monitor objects, when both monitor fields reference the same object?

Question

I have written a class with an internal private class extending Thread. My outer class starts an instance of this thread, and the thread accesses fields of the outer class within a loop. However, external agents may call methods over the outer class that modify the fields of the outer class. These methods must be synchronized with the loop inside the thread, so that modifications don't interfere with the loop.

I had been synchronizing using a "synchronized" block on a field of the outer class (an Android SurfaceHolder), passing this object into the inner class and storing a reference to it as a field in the inner class, then synchronizing on that inner class field in the thread's loop. This, however, led to behaviors in which the outer class methods would run when the inner class should have been locking! I tried removing the internal field and having the internal class lock on the exact same field as the external class, and everything worked fine.

So, here's the question: I verified that the object pointed at by both the internal and external fields was the same object by checking the == operator and looking at the string representation, so why does the synchronized block behave in this situation as if I was using two different objects? Perhaps I have a fundamental misunderstanding of the way synchronized blocks work?

EDIT:

Alright, I'm getting a lot of downvotes, but the commenters seem to just want some more detail and I'm determined to understand what I'm not getting here. I will respond to each comment. Here, to start, is an example of what I'm doing:

class Outer {
    private Object lock;
    private Foo foo;        

    public Outer() {
        lock = new Object();

        // The thread is actually started in an Android callback method,
        // but I'll start it here as a demonstration
        InnerThread thread = new InnerThread(lock);
        thread.setRunning(true);
        thread.start();
    }

    private void modifyFoo() {
        synchronized(lock) {
            Log.d("outer", "locked outer");
            foo.bar();    // Has some effect on foo
        }
        Log.d("outer", "released outer");
    }

    private class InnerThread extends Thread {

        private volatile boolean running = false;
        private Object lock;

        public InnerThread(Object lock) {
            this.lock = lock;
        }

        private void setRunning(boolean running) {
            this.running = running;
        }

        @Override
        public void run() {
            while(running) {
                synchronized(lock) {
                    Log.d("inner", "locked inner");
                    foo.blah();    // Has some effect on foo
                }
                Log.d("inner", "released inner");
            }
        }

    }
}

The unexpected behavior is that when I call the modifyFoo() method with the thread running, I see the following log output:

locked inner
released inner
locked inner
released inner
locked inner
 locked outer
 released outer
released inner

One response indicated "you should never extend thread" and "you appear to have an object in a field and the outer object itself". First, I am extending Thread because it is the approach used in the Android SurfaceView example. I don't know how else to override the run() method; should I be passing a runnable into the thread? Second, as I thought I understood it, the lock fields in the inner and outer classes both hold references to the same instance, created on the line lock = new Object();. I am not asking how this should be structured; I am asking specifically why the synchronized blocks appear to be viewing these two references to the same object as different objects.

To add some context, this is for a project using Android's SurfaceView class. I'm following as close as I can to the example LunarLander project provided by Android, in which the SurfaceHolder object that they use for locking is passed into the thread constructor and stored therein by reference. That might be the reason for the strange structure.

Answer 1

Why are two synchronized blocks acting like I've provided different monitor objects, when both monitor fields reference the same object?

Without seeing the code I would assume they really are different object even if you think they should be the same.

I have written a class with an internal private class extending Thread.

You should never extend Thread. This leads to all sorts of edge case you don't want in your code.

I had been synchronizing using a "synchronized" block on a field of the outer class (an Android SurfaceHolder), passing this object into the inner class and storing a reference to it as a field in the inner class, then synchronizing on that inner class field in the thread's loop. This, however, led to behaviors in which the outer class methods would run when the inner class should have been locking!

That sounds like you have two instances to me. You appear to have an object in a field and the outer object itself.

I tried removing the internal field and having the internal class lock on the exact same field as the external class, and everything worked fine.

This removed on of the two I mentioned.

Perhaps I have a fundamental misunderstanding of the way synchronized blocks work?

You have to lock on the same object instance. You don't lock on fields or classes or methods. I suggest creating an instance for locking which is only used for locking and this is all you use. This way there is no need to pass it to the inner class if it is a field of the outer class.

class Outer {
    final Object lock = new Object();

    public void method() {
        synchronized(lock) {
            // do something
            lock.notifyAll();
        }
    }

    class Inner implements Runnable {
         public void run() {
              while(!Thread.currentThread().isInterrupted()) {
                 synchronized(lock) {
                     lock.wait();
                     // check if anything changed.
                 }
              }
         }
    }
}

EDIT:

I verified that the object pointed at by both the internal and external fields was the same object by checking the == operator

If I copy the wrong referenced object for locking and compare it with itself it will be true too. If I copy a reference and have another reference I knows should be the same, why am copying it in the first place?

EDIT2: This is how I would write it.

class Outer {
    final Object lock = new Object();
    private Foo foo;        

    public Outer() {
        // The thread is actually started in an Android callback method,
        // but I'll start it here as a demonstration
        Thread thread = new Thread(new InnerRunnable());
        thread.start();
    }

    private void modifyFoo() {
        synchronized(lock) {
            Log.d("outer", "locked outer");
            foo.bar();    // Has some effect on foo
            Log.d("outer", "released outer");
        }
    }

    class InnerRunnable implement Runnable {
        private volatile boolean running = true;

        void setRunning(boolean running) {
            this.running = running;
        }

        @Override
        public void run() {
            while(running) {
                synchronized(lock) {
                    Log.d("inner", "locked inner");
                    foo.blah();    // Has some effect on foo
                    Log.d("inner", "released inner");
                }
            }
        }
    }
}

Optionally you can drop the lock and just use foo.