Synchronization.

3 files changed, 125 insertions, 10 deletions

diff --git a/src/main/java/com/google/dexmaker/Code.java b/src/main/java/com/google/dexmaker/Code.java
index 0c2a1db..50345c3 100644
--- a/src/main/java/com/google/dexmaker/Code.java
+++ b/src/main/java/com/google/dexmaker/Code.java
@@ -125,11 +125,11 @@ import java.util.List;
  * <ul>
  *     <li>{@link #invokeStatic} is used for static methods.</li>
  *     <li>{@link #invokeDirect} is used for private instance methods and
- *         constructors</li>
+ *         for constructors to call their superclass's constructor.</li>
  *     <li>{@link #invokeInterface} is used to invoke a method whose declaring
  *         type is an interface.</li>
  *     <li>{@link #invokeVirtual} is used to invoke any other method. The target
- *         must not be static, private, a constructor method, or an interface
+ *         must not be static, private, a constructor, or an interface
  *         method.</li>
  *     <li>{@link #invokeSuper} is used to invoke the closest superclass's
  *         virtual method. The target must not be static, private, a constructor
@@ -155,6 +155,17 @@ import java.util.List;
  * Use {@link #cast} to perform either a <strong>numeric cast</strong> or a
  * <strong>type cast</strong>. Interrogate the type of a value in a local using
  * {@link #instanceOfType}.
+ *
+ * <h3>Synchronization</h3>
+ * Acquire a monitor using {@link #monitorEnter}; release it with {@link
+ * #monitorExit}. It is the caller's responsibility to guarantee that enter and
+ * exit calls are balanced, even in the presence of exceptions thrown.
+ *
+ * <strong>Warning:</strong> Even if a method has the {@code synchronized} flag,
+ * dex requires instructions to acquire and release monitors manually. A method
+ * declared with {@link java.lang.reflect.Modifier#SYNCHRONIZED SYNCHRONIZED}
+ * but without manual calls to {@code monitorEnter()} and {@code monitorExit()}
+ * will not be synchronized when executed.
  */
 public final class Code {
     private final MethodId<?, ?> method;
@@ -678,6 +689,18 @@ public final class Code {
         addInstruction(new PlainInsn(rop, sourcePosition, target.spec(), RegisterSpecList.EMPTY));
     }
 
+    // instructions; synchronized
+
+    public void monitorEnter(Local<?> monitor) {
+        addInstruction(new ThrowingInsn(Rops.MONITOR_ENTER, sourcePosition,
+                RegisterSpecList.make(monitor.spec()), catches));
+    }
+
+    public void monitorExit(Local<?> monitor) {
+        addInstruction(new ThrowingInsn(Rops.MONITOR_ENTER, sourcePosition,
+                RegisterSpecList.make(monitor.spec()), catches));
+    }
+
     // produce BasicBlocks for dex
 
     BasicBlockList toBasicBlocks() {
diff --git a/src/main/java/com/google/dexmaker/DexMaker.java b/src/main/java/com/google/dexmaker/DexMaker.java
index be33313..bfa83af 100644
--- a/src/main/java/com/google/dexmaker/DexMaker.java
+++ b/src/main/java/com/google/dexmaker/DexMaker.java
@@ -25,6 +25,7 @@ import com.android.dx.dex.file.ClassDefItem;
 import com.android.dx.dex.file.DexFile;
 import com.android.dx.dex.file.EncodedField;
 import com.android.dx.dex.file.EncodedMethod;
+import com.android.dx.rop.code.AccessFlags;
 import static com.android.dx.rop.code.AccessFlags.ACC_CONSTRUCTOR;
 import com.android.dx.rop.code.LocalVariableInfo;
 import com.android.dx.rop.code.RopMethod;
@@ -227,7 +228,12 @@ public final class DexMaker {
      *
      * @param flags a bitwise combination of {@link Modifier#PUBLIC}, {@link
      *     Modifier#PRIVATE}, {@link Modifier#PROTECTED}, {@link Modifier#STATIC},
-     *     {@link Modifier#FINAL}, and {@link Modifier#VARARGS}.
+     *     {@link Modifier#FINAL}, {@link Modifier#SYNCHRONIZED} and {@link
+     *     Modifier#VARARGS}.
+     *     <p><strong>Warning:</strong> the {@link Modifier#SYNCHRONIZED} flag
+     *     is insufficient to generate a synchronized method. You must also use
+     *     {@link Code#monitorEnter} and {@link Code#monitorExit} to acquire
+     *     a monitor.
      */
     public Code declareConstructor(MethodId<?, ?> method, int flags) {
         return declare(method, flags | ACC_CONSTRUCTOR);
@@ -238,13 +244,22 @@ public final class DexMaker {
      *
      * @param flags a bitwise combination of {@link Modifier#PUBLIC}, {@link
      *     Modifier#PRIVATE}, {@link Modifier#PROTECTED}, {@link Modifier#STATIC},
-     *     {@link Modifier#FINAL}, and {@link Modifier#VARARGS}.
+     *     {@link Modifier#FINAL}, {@link Modifier#SYNCHRONIZED} and {@link
+     *     Modifier#VARARGS}.
+     *     <p><strong>Warning:</strong> the {@link Modifier#SYNCHRONIZED} flag
+     *     is insufficient to generate a synchronized method. You must also use
+     *     {@link Code#monitorEnter} and {@link Code#monitorExit} to acquire
+     *     a monitor.
      */
     public Code declare(MethodId<?, ?> method, int flags) {
         TypeDeclaration typeDeclaration = getTypeDeclaration(method.declaringType);
         if (typeDeclaration.methods.containsKey(method)) {
             throw new IllegalStateException("already declared: " + method);
         }
+        // replace the SYNCHRONIZED flag with the DECLARED_SYNCHRONIZED flag
+        if ((flags & Modifier.SYNCHRONIZED) != 0) {
+            flags = (flags & ~Modifier.SYNCHRONIZED) | AccessFlags.ACC_DECLARED_SYNCHRONIZED;
+        }
         MethodDeclaration methodDeclaration = new MethodDeclaration(method, flags);
         typeDeclaration.methods.put(method, methodDeclaration);
         return methodDeclaration.code;
diff --git a/src/test/java/com/google/dexmaker/DexMakerTest.java b/src/test/java/com/google/dexmaker/DexMakerTest.java
index 4948edb..97aeb56 100644
--- a/src/test/java/com/google/dexmaker/DexMakerTest.java
+++ b/src/test/java/com/google/dexmaker/DexMakerTest.java
@@ -21,11 +21,13 @@ import java.io.IOException;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import static java.lang.reflect.Modifier.FINAL;
 import static java.lang.reflect.Modifier.PRIVATE;
 import static java.lang.reflect.Modifier.PROTECTED;
 import static java.lang.reflect.Modifier.PUBLIC;
 import static java.lang.reflect.Modifier.STATIC;
+import static java.lang.reflect.Modifier.SYNCHRONIZED;
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.Callable;
@@ -274,6 +276,29 @@ public final class DexMakerTest extends TestCase {
         assertEquals("abc", getMethod().invoke(null, callable));
     }
 
+    public void testInvokeVoidMethodIgnoresTargetLocal() throws Exception {
+        /*
+         * public static int call() {
+         *   int result = 5;
+         *   DexMakerTest.voidMethod();
+         *   return result;
+         * }
+         */
+        MethodId<?, Integer> methodId = GENERATED.getMethod(TypeId.INT, "call");
+        MethodId<?, Void> voidMethod = TEST_TYPE.getMethod(TypeId.VOID, "voidMethod");
+        Code code = dexMaker.declare(methodId, PUBLIC | STATIC);
+        Local<Integer> result = code.newLocal(TypeId.INT);
+        code.loadConstant(result, 5);
+        code.invokeStatic(voidMethod, null);
+        code.returnValue(result);
+
+        assertEquals(5, getMethod().invoke(null));
+    }
+
+    @SuppressWarnings("unused") // called by generated code
+    public static void voidMethod() {
+    }
+
     public void testParameterMismatch() throws Exception {
         TypeId<?>[] argTypes = {
                 TypeId.get(Integer.class), // should fail because the code specifies int
@@ -407,7 +432,7 @@ public final class DexMakerTest extends TestCase {
         assertEquals(0xabcd, a.get(instance));
     }
 
-    public void testReturnBoolean() throws Exception {
+    public void testReturnType() throws Exception {
         testReturnType(boolean.class, true);
         testReturnType(byte.class, (byte) 5);
         testReturnType(char.class, 'E');
@@ -1630,6 +1655,63 @@ public final class DexMakerTest extends TestCase {
         return getMethod();
     }
 
+    public void testSynchronizedFlagImpactsDeclarationOnly() throws Exception {
+        /*
+         * public synchronized void call() {
+         *   wait(100L);
+         * }
+         */
+        MethodId<?, Void> methodId = GENERATED.getMethod(TypeId.VOID, "call");
+        MethodId<Object, Void> wait = TypeId.OBJECT.getMethod(TypeId.VOID, "wait", TypeId.LONG);
+        Code code = dexMaker.declare(methodId, PUBLIC | SYNCHRONIZED);
+        Local<?> thisLocal = code.getThis(GENERATED);
+        Local<Long> timeout = code.newLocal(TypeId.LONG);
+        code.loadConstant(timeout, 100L);
+        code.invokeVirtual(wait, null, thisLocal, timeout);
+        code.returnVoid();
+
+        addDefaultConstructor();
+
+        Class<?> generatedClass = generateAndLoad();
+        Object instance = generatedClass.newInstance();
+        Method method = generatedClass.getMethod("call");
+        assertTrue(Modifier.isSynchronized(method.getModifiers()));
+        try {
+            method.invoke(instance);
+            fail();
+        } catch (InvocationTargetException expected) {
+            assertTrue(expected.getCause() instanceof IllegalMonitorStateException);
+        }
+    }
+
+    public void testMonitorEnterMonitorExit() throws Exception {
+        /*
+         * public synchronized void call() {
+         *   synchronized (this) {
+         *     wait(100L);
+         *   }
+         * }
+         */
+        MethodId<?, Void> methodId = GENERATED.getMethod(TypeId.VOID, "call");
+        MethodId<Object, Void> wait = TypeId.OBJECT.getMethod(TypeId.VOID, "wait", TypeId.LONG);
+        Code code = dexMaker.declare(methodId, PUBLIC);
+        Local<?> thisLocal = code.getThis(GENERATED);
+        Local<Long> timeout = code.newLocal(TypeId.LONG);
+        code.monitorEnter(thisLocal);
+        code.loadConstant(timeout, 100L);
+        code.invokeVirtual(wait, null, thisLocal, timeout);
+        code.monitorExit(thisLocal);
+        code.returnVoid();
+
+        addDefaultConstructor();
+
+        Class<?> generatedClass = generateAndLoad();
+        Object instance = generatedClass.newInstance();
+        Method method = generatedClass.getMethod("call");
+        assertFalse(Modifier.isSynchronized(method.getModifiers()));
+        method.invoke(instance); // will take 100ms
+    }
+
     // TODO: cast primitive to non-primitive
     // TODO: cast non-primitive to primitive
     // TODO: cast byte to integer
@@ -1648,14 +1730,9 @@ public final class DexMakerTest extends TestCase {
 
     // TODO: declare native method or abstract method
 
-    // TODO: synchronized or declared synchronized?
-
     // TODO: get a thrown exception 'e' into a local
 
     // TODO: move a primitive or reference
-    // TODO: return void - target local can be null?
-
-    // TODO: declaring constructor calling superconstructor
 
     private void addDefaultConstructor() {
         Code code = dexMaker.declareConstructor(GENERATED.getConstructor(), PUBLIC);