From 5d0533b8903d5604883fbbfc95ccfda7ca4c1467 Mon Sep 17 00:00:00 2001
From: Alexander Kriegisch <Alexander@Kriegisch.name>
Date: Thu, 29 Dec 2022 15:17:53 +0100
Subject: [PATCH] Add test reproducing problem from #198

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
---
 .../declareMixin/CaseEConcurrent.java         | 84 +++++++++++++++++++
 .../systemtest/ajc164/DeclareMixinTests.java  |  6 ++
 .../systemtest/ajc164/declareMixin.xml        | 68 +++++++++------
 3 files changed, 133 insertions(+), 25 deletions(-)
 create mode 100644 tests/features164/declareMixin/CaseEConcurrent.java

diff --git a/tests/features164/declareMixin/CaseEConcurrent.java b/tests/features164/declareMixin/CaseEConcurrent.java
new file mode 100644
index 000000000..df1e13774
--- /dev/null
+++ b/tests/features164/declareMixin/CaseEConcurrent.java
@@ -0,0 +1,84 @@
+// TESTING: multiple instances causing factory invocation multiple times (but is cached correctly)
+// Concurrency fix regression test for https://github.com/eclipse/org.aspectj/issues/198
+import org.aspectj.lang.annotation.*;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class CaseEConcurrent {
+  private String id;
+
+  public static void main(String[]argv) throws InterruptedException {
+    final CaseEConcurrent cea = new CaseEConcurrent("a");
+    final CaseEConcurrent ceb = new CaseEConcurrent("b");
+
+    Thread t1 = new Thread(new Runnable() { public void run() { ((I)cea).methodOne(); } });
+    Thread t2 = new Thread(new Runnable() { public void run() { ((I)cea).methodTwo(); } });
+    Thread t3 = new Thread(new Runnable() { public void run() { ((I)ceb).methodOne(); } });
+    Thread t4 = new Thread(new Runnable() { public void run() { ((I)ceb).methodTwo(); } });
+
+    t1.start();
+    t2.start();
+    t3.start();
+    t4.start();
+
+    t1.join();
+    t2.join();
+    t3.join();
+    t4.join();
+  }
+
+  public CaseEConcurrent(String id) {
+    this.id=id;
+  }
+
+  public String toString() {
+    return "CaseEConcurrent instance: "+id;
+  }
+
+  // Helper methods 'doSomething' and 'callMe' help to produce byte code similar to what we need in order to fix
+  // https://github.com/eclipse/org.aspectj/issues/198. If necessary, just temporarily uncomment, compile and analyse
+  // the byte code, e.g. with JDK tool 'javap -v'.
+  /*
+  public void doSomething() {
+    synchronized (this) {
+      if (id == null)
+        id = "doing something";
+    }
+    callMe(id);
+  }
+
+  public void callMe(String param) {
+    System.out.println("I was called with param " + param);
+  }
+  */
+
+}
+
+aspect X {
+  @DeclareMixin("CaseEConcurrent")
+  public I createImplementation(Object o) {
+    System.out.println("Delegate factory invoked for " + o);
+    try { Thread.sleep(250); } catch (InterruptedException e) { throw new RuntimeException(e); }
+    Implementation impl = new Implementation(o);
+    return impl;
+  }
+}
+
+interface I {
+  void methodOne();
+  void methodTwo();
+}
+
+class Implementation implements I {
+  Object o;
+
+  public Implementation(Object o) {
+    this.o = o;
+  }
+
+  public void methodOne() {
+    System.out.println("methodOne running on "+o);
+  }
+  public void methodTwo() {
+    System.out.println("methodTwo running on "+o);
+  }
+}
diff --git a/tests/src/test/java/org/aspectj/systemtest/ajc164/DeclareMixinTests.java b/tests/src/test/java/org/aspectj/systemtest/ajc164/DeclareMixinTests.java
index 1f5c135e3..1c312d18b 100644
--- a/tests/src/test/java/org/aspectj/systemtest/ajc164/DeclareMixinTests.java
+++ b/tests/src/test/java/org/aspectj/systemtest/ajc164/DeclareMixinTests.java
@@ -67,6 +67,12 @@ public class DeclareMixinTests extends org.aspectj.testing.XMLBasedAjcTestCase {
 		runTest("casee");
 	}
 
+	// multiple instances causing factory invocation multiple times (but is cached), concurrent case
+	// see https://github.com/eclipse/org.aspectj/issues/198
+	public void testCaseEConcurrent() {
+		runTest("casee_concurrent");
+	}
+
 	// Factory method directly takes the type specified in the Mixin target (strongly typed)
 	public void testCaseF() {
 		runTest("casef");
diff --git a/tests/src/test/resources/org/aspectj/systemtest/ajc164/declareMixin.xml b/tests/src/test/resources/org/aspectj/systemtest/ajc164/declareMixin.xml
index 02853f93a..c397fae87 100644
--- a/tests/src/test/resources/org/aspectj/systemtest/ajc164/declareMixin.xml
+++ b/tests/src/test/resources/org/aspectj/systemtest/ajc164/declareMixin.xml
@@ -1,7 +1,7 @@
 <!DOCTYPE suite SYSTEM "../tests/ajcTestSuite.dtd"[]>
 
 <suite>
-   
+
    <ajc-test dir="features164/declareMixin" title="casea">
      <compile files="CaseA.java" options="-1.5"/>
      <run class="CaseA">
@@ -11,7 +11,7 @@
        </stdout>
      </run>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="caseb">
      <compile files="CaseB.java" options="-1.5"/>
      <run class="CaseB">
@@ -21,7 +21,7 @@
        </stdout>
      </run>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="casec">
      <compile files="CaseC.java" options="-1.5"/>
      <run class="CaseC">
@@ -31,7 +31,7 @@
        </stdout>
      </run>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="cased">
      <compile files="CaseD.java" options="-1.5"/>
      <run class="CaseD">
@@ -41,7 +41,7 @@
        </stdout>
      </run>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="casee">
      <compile files="CaseE.java" options="-1.5"/>
      <run class="CaseE">
@@ -55,7 +55,25 @@
        </stdout>
      </run>
    </ajc-test>
-   
+
+   <ajc-test dir="features164/declareMixin" title="casee_concurrent">
+     <compile files="CaseEConcurrent.java" options="-1.5"/>
+     <run class="CaseEConcurrent">
+       <stdout ordered="no">
+         <!--
+           Without the concurrency fix from https://github.com/eclipse/org.aspectj/issues/198, each delegate factory
+           would be invoked twice
+         -->
+         <line text="Delegate factory invoked for CaseEConcurrent instance: a"/>
+         <line text="Delegate factory invoked for CaseEConcurrent instance: b"/>
+         <line text="methodOne running on CaseEConcurrent instance: a"/>
+         <line text="methodTwo running on CaseEConcurrent instance: a"/>
+         <line text="methodOne running on CaseEConcurrent instance: b"/>
+         <line text="methodTwo running on CaseEConcurrent instance: b"/>
+       </stdout>
+     </run>
+   </ajc-test>
+
    <ajc-test dir="features164/declareMixin" title="casef">
      <compile files="CaseF.java" options="-1.5"/>
      <run class="CaseF">
@@ -65,7 +83,7 @@
        </stdout>
      </run>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="caseg">
      <compile files="CaseG.java" options="-1.5"/>
      <run class="CaseG">
@@ -77,44 +95,44 @@
        </stdout>
      </run>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="caseh">
      <compile files="CaseH.java" options="-1.5">
          <message kind="error" text="The value for annotation attribute DeclareMixin"/>
      </compile>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="casei">
      <compile files="CaseI.java" options="-1.5">
          <message kind="error" text="Types listed in the 'interfaces'"/>
      </compile>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="casej">
      <compile files="CaseJ.java" options="-1.5">
          <message kind="error" text="createImplementation1"/>
          <message kind="error" text="Method 'int X.createImplementation2(java.lang.Object)':  factory methods "/>
      </compile>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="casek">
      <compile files="CaseK.java" options="-1.5">
          <message kind="error" text="factory methods for a mixin can take a maximum of one parameter"/>
      </compile>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="casel">
      <compile files="CaseL.java" options="-1.5">
        <message kind="error" text="Cannot cast from CaseL to C"/>
      </compile>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="casem">
      <compile files="CaseM.java" options="-1.5">
        <message kind="error" text=": factory methods for a mixin must either return an interface type or specify interfaces in the annotation and return a class"/>
      </compile>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="casen">
      <compile files="CaseN.java" options="-1.5"/>
      <run class="CaseN">
@@ -123,13 +141,13 @@
        </stdout>
      </run>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="caseo">
      <compile files="CaseO.java" options="-1.5">
        <message kind="error" text="factory method does not return something that implements 'I'"/>
      </compile>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="casep">
      <compile files="CaseP.java" options="-1.5"/>
      <run class="CaseP">
@@ -138,7 +156,7 @@
        </stdout>
      </run>
    </ajc-test>
-   
+
    <ajc-test dir="features164/declareMixin" title="caseq">
      <compile files="CaseQ.java" options="-1.5"/>
      <run class="CaseQ">
@@ -147,8 +165,8 @@
          <line text="goo() running"/>
        </stdout>
      </run>
-   </ajc-test>   
-     
+   </ajc-test>
+
    <ajc-test dir="features164/declareMixin" title="caser">
      <compile files="CaseR.java" options="-1.5"/>
      <run class="CaseR">
@@ -157,17 +175,17 @@
          <line text="false"/>
        </stdout>
      </run>
-   </ajc-test>    
-    
+   </ajc-test>
+
    <ajc-test dir="features164/declareMixin" title="cases">
      <compile files="CaseS.java" options="-1.5">
        <message kind="error" text="not compatible"/>
      </compile>
-   </ajc-test>     
-   
+   </ajc-test>
+
    <ajc-test dir="features164/declareMixin" title="caset">
      <compile files="CaseT.java" options="-1.5 -showWeaveInfo">
        <message kind="weave" text="Mixing interface 'I' (CaseT.java) into type 'CaseT' (CaseT.java)"/>
      </compile>
-   </ajc-test>    
-</suite>
\ No newline at end of file
+   </ajc-test>
+</suite>
-- 
2.39.5