Browse Source

BcelTypeMunger.mungeMethodDelegate: only use 'synchronized' when necessary

Relates to #198. Now, we create a delegate method body which basically
looks as follows:

public void methodOne() {
  if (this.ajc$instance$MyAspect$MyMixin == null) {
    synchronized(this) {
      if (this.ajc$instance$MyAspect$MyMixin == null) {
        this.ajc$instance$MyAspect$MyMixin = MyAspect.aspectOf().createImplementation(this);
      }
    }
  }
  this.ajc$instance$MyAspect$MyMixin.methodOne();
}

The idea for the outer null check is from @aclement, see
https://github.com/eclipse/org.aspectj/pull/205#issuecomment-1371556080.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
tags/V1_9_20
Alexander Kriegisch 1 year ago
parent
commit
c8b9568a85

+ 5
- 3
tests/features164/declareMixin/CaseEConcurrent.java View File

// the byte code, e.g. with JDK tool 'javap -v'. // the byte code, e.g. with JDK tool 'javap -v'.
/* /*
public void doSomething() { public void doSomething() {
synchronized (this) {
if (id == null)
id = "doing something";
if (id == null) {
synchronized (this) {
if (id == null)
id = "doing something";
}
} }
callMe(id); callMe(id);
} }

+ 25
- 3
weaver/src/main/java/org/aspectj/weaver/bcel/BcelTypeMunger.java View File

InstructionList body = new InstructionList(); InstructionList body = new InstructionList();
InstructionFactory fact = gen.getFactory(); InstructionFactory fact = gen.getFactory();


// Create a delegate method body which basically looks as follows:
//
// public void methodOne() {
// if (this.ajc$instance$MyAspect$MyMixin == null) {
// synchronized(this) {
// if (this.ajc$instance$MyAspect$MyMixin == null) {
// this.ajc$instance$MyAspect$MyMixin = MyAspect.aspectOf().createImplementation(this);
// }
// }
// }
// this.ajc$instance$MyAspect$MyMixin.methodOne();
// }

// Before synchronising on 'this', perform a preliminary null check on the delegate field. Only if it is null,
// it makes sense to synchronise, then check for null again and initialise the delegate, if null.
body.append(InstructionConstants.ALOAD_0);
body.append(Utility.createGet(fact, munger.getDelegate(weaver.getLazyClassGen().getType())));
InstructionBranch outerIfNonNull = InstructionFactory.createBranchInstruction(Constants.IFNONNULL, null);
body.append(outerIfNonNull);

// Wrap delegate field initialisation in 'synchronized(this)' block - MONITORENTER (beginning of 'try') // Wrap delegate field initialisation in 'synchronized(this)' block - MONITORENTER (beginning of 'try')
body.append(InstructionConstants.ALOAD_0); body.append(InstructionConstants.ALOAD_0);
body.append(InstructionConstants.MONITORENTER); body.append(InstructionConstants.MONITORENTER);
// synchronized block, see https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-3.html#jvms-3.14. // synchronized block, see https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-3.html#jvms-3.14.
InstructionHandle tryStart = body.append(InstructionConstants.ALOAD_0); InstructionHandle tryStart = body.append(InstructionConstants.ALOAD_0);
body.append(Utility.createGet(fact, munger.getDelegate(weaver.getLazyClassGen().getType()))); body.append(Utility.createGet(fact, munger.getDelegate(weaver.getLazyClassGen().getType())));
InstructionBranch ifNonNull = InstructionFactory.createBranchInstruction(Constants.IFNONNULL, null);
body.append(ifNonNull);
InstructionBranch innerIfNonNull = InstructionFactory.createBranchInstruction(Constants.IFNONNULL, null);
body.append(innerIfNonNull);


// Create and store a new instance // Create and store a new instance
body.append(InstructionConstants.ALOAD_0); // 'this' is where we'll store the field value body.append(InstructionConstants.ALOAD_0); // 'this' is where we'll store the field value


// Wrap delegate field initialisation in 'synchronized(this)' block - MONITOREXIT (end of 'try') // Wrap delegate field initialisation in 'synchronized(this)' block - MONITOREXIT (end of 'try')
InstructionHandle ifNonNullElse = body.append(InstructionConstants.ALOAD_0); InstructionHandle ifNonNullElse = body.append(InstructionConstants.ALOAD_0);
ifNonNull.setTarget(ifNonNullElse);
innerIfNonNull.setTarget(ifNonNullElse);
body.append(InstructionConstants.MONITOREXIT); body.append(InstructionConstants.MONITOREXIT);


// There was no error in the 'synchronized(this)' block -> jump to first instruction after exception handler // There was no error in the 'synchronized(this)' block -> jump to first instruction after exception handler
InstructionHandle afterTryCatch = body.append(InstructionConstants.ALOAD_0); InstructionHandle afterTryCatch = body.append(InstructionConstants.ALOAD_0);
// Tell 'gotoAfterTryCatch' where to find the first statement after the exception handler // Tell 'gotoAfterTryCatch' where to find the first statement after the exception handler
gotoAfterTryCatch.setTarget(afterTryCatch); gotoAfterTryCatch.setTarget(afterTryCatch);
// The outer delegate field null check should also jump to the same place, if the delegate field is not null
outerIfNonNull.setTarget(afterTryCatch);
body.append(Utility.createGet(fact, munger.getDelegate(weaver.getLazyClassGen().getType()))); body.append(Utility.createGet(fact, munger.getDelegate(weaver.getLazyClassGen().getType())));


// args // args

Loading…
Cancel
Save