title: Multithreaded correctness: Using synchronized block
body: <p>I am using the CMU Sphinx Speech Recognizer library (<a href="http://cmusphinx.sourceforge.net/2011/05/building-pocketsphinx-on-android/" rel="noreferrer">Link to source</a>) that makes some use of <code>synchronized</code> blocks.</p>
  
  <p>One example block from the RecognizerTask:</p>
  
  <pre><code>Event mailbox;
  
  [...]
  
  public void start() {
      synchronized (this.mailbox) {
          this.mailbox.notifyAll();
          this.mailbox = Event.START;
      }
  }
  </code></pre>
  
  <p>The code works without any problems, however BugFinder gives this warning:</p>
  
  <blockquote>
    <p>Bug: Synchronization on RecognizerTask.mailbox in futile attempt to
    guard it</p>
    
    <p>This method synchronizes on a field in what appears to be an
    attempt to guard against simultaneous updates to that field. But
    guarding a field gets a lock on the referenced object, not on the
    field. This may not provide the mutual exclusion you need, and other
    threads might be obtaining locks on the referenced objects (for other
    purposes). An example of this pattern would be:</p>
  
  <pre><code>private Long myNtfSeqNbrCounter = new Long(0);
  private Long getNotificationSequenceNumber() {
       Long result = null;
       synchronized(myNtfSeqNbrCounter) {
           result = new Long(myNtfSeqNbrCounter.longValue() + 1);
           myNtfSeqNbrCounter = new Long(result.longValue());
       }
       return result;
   }
  </code></pre>
  </blockquote>
  
  <p>To be honest, I don't quite understand the bug description and what is supposed to be wrong in this case.
  Is a global variable not a field? And if not, how can I improve the code?</p>
  
  <p><strong>/edit:</strong> This is the only part where <code>Event.wait()</code> is called:</p>
  
  <pre><code>Event todo = Event.NONE;
          synchronized (this.mailbox) {
              todo = this.mailbox;
              /* If we're idle then wait for something to happen. */
              if (state == State.IDLE &amp;&amp; todo == Event.NONE) {
                  try {
                      //Log.d(getClass().getName(), "waiting");
                      this.mailbox.wait();
                      todo = this.mailbox;
                      //Log.d(getClass().getName(), "got" + todo);
                  } catch (InterruptedException e) {
                      /* Quit main loop. */
                      //Log.e(getClass().getName(), "Interrupted waiting for mailbox, shutting down");
                      todo = Event.SHUTDOWN;
                  }
              }
              /* Reset the mailbox before releasing, to avoid race condition. */
              this.mailbox = Event.NONE;
          }
  </code></pre>
  
  <p>This code is actually using a <code>synchronized</code> statement as well. Does it make sense at all to use it?</p>