Java Solaris Communities Sun Store Join SDN My Profile Why Join?
 
Bug Database
Bug Detail
Quick Lists
Top 25 Bugs
Top 25 RFE's
Recently Closed Bugs
Printable Page Printable Page


Bug Database
Bug ID: 4414101
Votes 0
Synopsis synchronized statement generates catch around the monitorexit
Category java:compiler
Reported Against merlin-beta
Release Fixed
State 11-Closed, Not a Defect, bug
Priority: 4-Low
Related Bugs 6271353 , 6286708 , 4414755 , 4504165
Submit Date 10-FEB-2001
Description
Code generation for try{}finally{} generates a catch block around the 
implicitly generated catch-all clause that jsrs to the finally
clause itself. This catch phrase names itself as the recipient!
This is useless.

More interestingly, the synchronized statement places a catch record 
around the monitorexit instruction so it can retry the instruction
if there is an asynchronous exception. Infortunately, that catch
record catches all, so an IllegalMonitorStateException will cause the VM
to enter an infinite loop retrying the monitorexit instruction forever.

frog$ cat -n Fbug.java
     1	class Fbug {
     2	    int m;
     3	
     4	    void test(Fbug f, int m) {
     5		try {
     6		    this.m = f.m;
     7		} finally {
     8		    this.m = 0;
     9		}
    10	    }
    11	}
frog$ javap -c Fbug
Compiled from Fbug.java
synchronized class Fbug extends java.lang.Object 
    /* ACC_SUPER bit set */
{
    int m;
    Fbug();
    void test(Fbug, int);
}

Method Fbug()
   0 aload_0
   1 invokespecial #1 <Method java.lang.Object()>
   4 return

Method void test(Fbug, int)
   0 aload_0
   1 aload_1
   2 getfield #2 <Field int m>
   5 putfield #2 <Field int m>
   8 jsr 20
  11 goto 29
  14 astore_3
  15 jsr 20
  18 aload_3
  19 athrow
  20 astore 4
  22 aload_0
  23 iconst_0
  24 putfield #2 <Field int m>
  27 ret 4
  29 return
Exception table:
   from   to  target type
     0    11    14   any
    14    18    14   any
frog$ 
Work Around
N/A
Evaluation
While the code looks a bit
silly for the try-finally, it is at least correct. The related code
that we now generate for synchronized(){} statements illustrates the
need for these strange catch blocks. It is necessary to handle
asynchronous exceptions that can arise before the monitor is unlocked.

Unfortunately, this catch block is too liberal in what it catches
in the synchronized statement case. A IlegalMonitorStateException 
thrown from the monitorexit instruction will cause the VM to enter 
an infinite loop. The catch phrase should catch only ThreadDeath.

As for the try-finally, we should avoid generating that extra 
exception record.

  xxxxx@xxxxx   2001-02-13

The language spec requires the compiler to generate code that releases the
monitor precisely once when exiting the synchronized block *for any reason*.
See 14.18 sixth paragraph and section 17.13.  Failure of the VM to release
the lock is no excuse to ignore this part of the language spec: the compiler
must not allow the synchronized block to be exited until the monitor is 
successfully unlocked.  It is therefore correct for the compiler to generate 
code that fails to exit the monitored block when the monitor cannot be released.

The inability of the VM to release the lock is a kind of deadlock that
the Java programming language does not prevent or require detection of
(17.13 last paragraph).  Moreover, the possibility of an asynchronous
throw of llegalMonitorStateException is yet another reason that such
an exception should receive no special treatment.

In short, javac's treatment of this in 1.4 is precisely correct.

  xxxxx@xxxxx   2002-02-20
Comments
  
  Include a link with my name & email   

Submitted On 19-FEB-2002
eblake
If I understand the evaluation correctly (and based on some offline
discussion with Neal Gafter), this is the best solution for a
synchronized statement:

class T {
  static {
    Object o = null;
    int x = 3;
    synchronized(o) {
      x = 4;
    }
  }
}

Method static {}
 0 aconst_null
 1 astore_0
 2 iconst_3
 3 istore_1
 4 aload_0
 5 dup
 6 astore_2
 7 monitorenter
 8 iconst_4
 9 istore_1
10 aload_2
11 monitorexit
12 goto 18
15 aload_2
16 monitorexit
17 athrow
18 return
Exception table:
 from to target type
  11 12 17  java.lang.IllegalMonitorStateException
  8 12 15 any
  16 17  17  java.lang.IllegalMonitorStateException
  15 17 15 any


Submitted On 18-JUN-2002
davidholmes
eblake, I have to disagree. There are two cases to consider:
a) the current thread owns the monitor and 
b) the current thread does not own the monitor

If we have a) and an exception occurs while trying to 
release the monitor then the language semantics put us in 
an unwinnable position: we *must* release the monitor 
before propogating the exception but we can't release the 
monitor because it is excepting. An infinite loop is the 
only legal recourse without halting the VM. Personally I 
think I'd halt the VM.

Case b) is a different matter. Unless it is illegal to 
accept bytecode that tries to release a monitor that is not 
locked by the current thread, or which uses a null 
reference, then the IllegalMonitorStateException or 
NullPointerException should be thrown and should propogate. 
The JVMS permits both exceptions to occur and so we must 
deal with them.

The null reference can probably be dealt with by an 
aposteri check of what is pushed (and a good JIT will 
remove the need for the check). Note that it is not 
sufficient to simply use the type of exception as a guide.

The IllegalMonitorStateException is more difficult because 
it requires knowledge that is not available to the catch 
block. We'd need to check that the current thread is not 
the owner - but it could be that test which is erroneously 
causing an exception. Again the type of the exception is 
not itself sufficient.

The whole discussion of asynchronous exceptions has me 
intrigued because Java doesn't have them. Thread.stop is 
deprecated and never guaranteed immediacy. The RTSJ has 
them but only specific code and *not* within synchronized 
blocks. So I'm at a loss to understand what asynchronous 
exceptions they were trying to deal with.

Bottom line is that this is a difficult problem to solve. 
But I have to disagree that the current javac treatment 
is "precisely correct". But it might be the best we can 
reasonably do.


Submitted On 11-SEP-2002
eblake
Further discussion with Neal Gafter leads to the current behavior in the jikes compiler, which Neal praised as 
asynchronous-safe (see 
http://www-124.ibm.com/developerworks/bugs/?func=detailbug&bug_id=2647&group_id=10).
$ cat -n T.java
     1  class T {
     2  static {
     3      Object o = null;
     4      int x = 3;
     5      synchronized(o) {
     6          x = 4;
     7      }
     8  }
     9  }

compiles to:

Method static {} 
0 aconst_null 
1 astore_0 
2 iconst_3 
3 istore_1 
4 goto 10 
7 aload_2 
8 monitorexit 
9 athrow 
10 aload_0 
11 dup 
12 astore_2 
13 monitorenter 
14 iconst_4 
15 istore_1 
16 aload_2 
17 monitorexit 
18 return 
Exception table: 
from to target type 
7 9 7 any 
14 18 7 any




PLEASE NOTE: JDK6 is formerly known as Project Mustang