Don’t rely on inherited code, keep your eyes open

There is a cost to making mistakes when coding, and the cost of not dealing with these mistakes will increase over time.

Do you remember the technical debt metaphor? We don’t really know how much debt we have taken on – you may have taken on a lot of unintentional technical debt – and you may still be taking it on without knowing it. Error-prone code – (the 20% of the code where 80% of bugs are found) is the second more expensive reason of this unintentional technical debt (the first one might be making a fundamental mistake in architecture or the platform technology). And as you know, not rewriting this code is one of the most expensive mistakes that developers make.

But the first step is to find this code, and it is always a hard duty. You can find this type of code in the most unlikely of places, even in a reputated high qualyty book.

Let’s start staring at the following example:

package eu.albertomorales.OCAOCP.threads.example2;

public class ThreadA {

  public static void main(String[] args) {
    ThreadB b = new ThreadB();
    b.start();
    System.out.println("A says: willing to get b lock");
    synchronized(b){
      System.out.println("A says: b lock gotten");
      try {
        System.out.println("A says: before wait()");
        b.wait();
        System.out.println("A says: after wait()");
      } catch (InterruptedException ie) {
        System.out.println("InterruptedException, Total is: " + 
              b.total);
      }
    }
    System.out.println("A says: b lock released");
  }
}
package eu.albertomorales.OCAOCP.threads.example2;

public class ThreadB extends Thread {

  int total;

  @Override
  public void run() {
    System.out.println("B says: willing to get 'me' lock");
    synchronized (this) {
      System.out.println("B says: 'me' lock gotten");
      for (int i = 0; i < 100; i++) {
        total += i;
      }
      System.out.println("B says: before notify()");
      notify();
      System.out.println("B says: after notify()");
    }
    System.out.println("B says: run has finished");
  }
}

It’s been taken from “OCA/OCP Java SE 7 Programmer I & II Study Guide” (chapter 13 Threads), you know, the reference book to prepare exams IZ0-803 & IZ0-804 from Katy Sierra and Bert Bates (Oracle Press).

This example is suposed to illustrate the correct use of wait() and notify(), the interaction between them and how to synchronize code properly.

If you execute ThreadA (the only class with a main() method), the show starts:

  • Main thread starts an instance of ThreadB (we’re calling it b) and “continues” executing itself.
  • Main thread gets the lock on b.
  • Main thread invokes b.wait(), and from that moment, it doesn’t execute any further instructions until the notify() method of b is called. Fortunately, main thread temporarily release the lock for other thread to use.
  • Thread b can then obtain its own lock.
  • During the execution of the loop inside the run() method of ThreadB, main thread is waiting to get the lock on b again, to continue execution.
  • Thread b notifies (notify()) main thread, releases the lock and finishes execution.

This is a capture of the standard output (remember: this time, on our machine):

A says: willing to get b lock
A says: b lock gotten
A says: before wait()
B says: willing to get 'me' lock
B says: 'me' lock gotten
B says: before notify()
B says: after notify()
B says: run has finished
A says: after wait()
A says: b lock released

It seems to work properly. But if you look closely enough, you will clearly see the (posibility of) fail. The author of the example is relying on the fact that

synchronized(b){

(in main thread) is reached before method run of b starts. But as we comment in a previous post, in JAVA, when it comes to threads, very little is guaranteed. Specially:

There is no guarantee that once a thread starts executing, it will keep executing until it’s done

Due to this fact (lack of guarantee), we thought that the code in the example behaves as expected by serendipity. If you run the code multiple times or on multiple machines, you may see different output, because the behavior you see is not guaranteed. Sometimes a subtle change in the code (or the conditions, or the VM, etc…) can change the behavior.

Let’s try to break the example.

It has been easy to influence the thread scheduler (remember, we don’t control it) to put main thread to sleep, and to give thread b a chance to run. Introducing a tiny (1ms) delay is enough to break the example:
package eu.albertomorales.OCAOCP.threads.example2;
public class ThreadA {

  public static void main(String[] args) {
    ThreadB b = new ThreadB();
    b.start();
        try {
          Thread.sleep(1);
        } catch (InterruptedException e) {
          e.printStackTrace();
        }
    System.out.println("A says: willing to get b lock");
    synchronized(b){
      System.out.println("A says: b lock gotten");
      try {
        System.out.println("A says: before wait()");
        b.wait();
        System.out.println("A says: after wait()");
      } catch (InterruptedException ie) {
        System.out.println("InterruptedException, Total is: " 
              + b.total);
      }
    }
    System.out.println("A says: b lock released");
  }
}

Running this code might produce the following:

B says: willing to get 'me' lock
B says: 'me' lock gotten
B says: before notify()
B says: after notify()
B says: run has finished
A says: willing to get b lock
A says: b lock gotten
A says: before wait()
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 The result now has changed substantially:
  • Main thread starts an instance of ThreadB (we’re calling it b) and goes to sleep for, at least, 1ms.
  • Thread b is chosen (by serendipity) by the thread scheduler to run.
  • Thread b can obtain its own lock
  • Loop is completed
  • Thread b notifies to nobody (nobody is waiting), releases the lock and finishes execution.
  • Main thread gets the lock on b.
  • Main thread invokes b.wait(), and from that moment, it doesn’t execute any further instructions until the notify() method of b is called. Unfortunately, thread b is in a dead state, so nobody is going to notify (notify()) to main thread. What does that mean? Main thread never finishes, it will be waiting and last forever.

We can find the mistake in the assumption

“… As soon as line 4 calls the start() method, ThreadA will continue with the next line of code in its own class…” (taken from the related book’s example explanation, pg. 757). This is the expected behavior, but this behavior is not guaranteed. The thread scheduler needs no reason for moving the current thread from running to runnable in order to give another thread a chance to run.

We trust in your JAVA threads knowledge, and you know different JVMs can run threads in different ways. So you musn’t make the mistake of designing your software to be dependent on a particular behaviour (on a particular implementation of the JVM).

But the key message is that you musn’t rely on inherited code. As we’ve shown in this post, even inside a reference book you can find some error-prone code. Instead, you must keep your eyes open.

Eyes-Wide-Open-300x234


Would you like to share another example? Have you got an interesting one, and would you like to share it with the comunity? If so, or you’re interested in receiving more information, please post them in the comments so we can write about it, and we can keep in touch!

And as always, if you’ve liked this post, you can it. Thank you.

Deja un comentario