Hi Alexander,

Thanks for your reply.

ThreadLocal is used to avoid multiple threads creating objects of the same
class and need to increment the counter at the same time.

Thanks for your time reviewing it! I’d really appreciate it if you want to
help to refactor it.

regards,
Yongle

On May 21, 2018 at 9:58:16 AM, Alexander Kriegisch (alexan...@kriegisch.name)
wrote:

Hello Yongle.

I am happy you found a workaround for your problem, although it seems a bit
over-engineered. There are several parts which could use refactoring, but
most of all I am irritated by the ThreadLocal. Why would you need that? You
are keeping only one ID (truly just a simple counter) per class. What are
you gaining by using the ThreadLocal there?
Regards
-- 
Alexander Kriegisch
https://scrum-master.de


Yongle Zhang schrieb am 21.05.2018 02:17:

Hi Alexander,

Thank you for your quick reply! Really appreciate your effort!

In terms of my original goal, I had to come up with some workaround.

The general idea is to just maintain a Map<java.lang.class, counter> by
myself. There’s some synchronization and optimization to do.

I’ll post here in case it’s helpful for others.

To make it clear, this method works for “inserting an ID for objects of
classes in any jar” except for rt.jar.

import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.Callable;
import java.lang.Runnable;

privileged aspect MyRunnablesCallables {

  public static Map<java.lang.Class, ThreadLocal<Long>> My_counters = new
      HashMap<java.lang.Class, ThreadLocal<Long>>();

  public static long getMyCounter(java.lang.Class clazz) {
    ThreadLocal<Long> local = My_counters.get(clazz);
    if (local == null) {
         // need to initialize the ThreadLocal
         // This is the only place that needs synchronization.
      synchronized (clazz) { // synchronize on the given clazz is enough
        // Double check to avoid multi threads entered the previous if statement
        if (local == null) {
          local = new ThreadLocal<Long>() {
            @Override
            protected Long initialValue() {
              return (long) 0;
            }
          };
          My_counters.put(clazz, local);
        }
      }
    }
    return (long) local.get();
  }

  private static void setMyCounter(java.lang.Class clazz, long counter) {
    ThreadLocal<Long> local = My_counters.get(clazz);
    // This function is only called in incrementMyCounter() so it's not null.
    assert local != null;
    local.remove();
    local.set(counter);
  }

  public static void incrementMyCounter(java.lang.Class clazz) {
    setMyCounter(clazz, getMyCounter(clazz)+1);
  }

  public interface InstrumentedRunnableCallable {}

  private boolean InstrumentedRunnableCallable.My_id_assigned = false;

  // here we use (thread id + a thread local id) as this object's ID.
  // For now we don't consider the case when Thread ID could be reused.
  private long InstrumentedRunnableCallable.My_id = -1;
  private long InstrumentedRunnableCallable.My_creator_tid = -1;

  public long InstrumentedRunnableCallable.getMyId() {
    return My_id;
  }
  public void InstrumentedRunnableCallable.setMyId(long id) {
    My_id = id;
  }

  public long InstrumentedRunnableCallable.getCreatorTid() {
    return My_creator_tid;
  }
  public void InstrumentedRunnableCallable.setCreatorTid(long id) {
    My_creator_tid = id;
  }

  public boolean InstrumentedRunnableCallable.getMyIdAssigned() {
    return My_id_assigned;
  }
  public void InstrumentedRunnableCallable.setMyIdAssigned(boolean assigned) {
    My_id_assigned = assigned;
  }

  declare parents: (Runnable)+ implements InstrumentedRunnableCallable;
  declare parents: (Callable)+ implements InstrumentedRunnableCallable;

  after() returning(InstrumentedRunnableCallable r):
      (call(InstrumentedRunnableCallable+.new(..))) {
    if (r.getMyIdAssigned() == false) { // in case there're nested constructors
      r.setMyIdAssigned(true);

      r.setCreatorTid(Thread.currentThread().getId());

      r.setMyId(MyRunnablesCallables.getMyCounter(r.getClass()));

      MyRunnablesCallables.incrementMyCounter(r.getClass());

    }
    System.out.println( "[Create Runnable/Callable] " +
        "TID: " + Thread.currentThread().getId()
        + " creator tid " + r.getCreatorTid()
        + " id " + r.getMyId()
        + " class " + r.getClass().getName()
    );
  }

  before(InstrumentedRunnableCallable r):
      this(r) &&
      (execution(void Runnable+.run(..)) || execution(* Callable+.call(..))) {
    System.out.println( "[Before Runnable run() / Callable call() ] " +
        "TID: " + Thread.currentThread().getId()
        + " creator tid " + r.getCreatorTid()
        + " id " + r.getMyId()
        + " class " + r.getClass().getName()
    );
  }

  after(InstrumentedRunnableCallable r):
      this(r) &&
      (execution(void Runnable+.run(..)) || execution(* Callable+.call(..))) {
    System.out.println( "[After Runnable run() / Callable call() ] " +
        "TID: " + Thread.currentThread().getId()
        + " creator tid " + r.getCreatorTid()
        + " id " + r.getMyId()
        + " class " + r.getClass().getName()
    );
  }
}





On May 19, 2018 at 7:01:56 AM, Alexander Kriegisch (alexan...@kriegisch.name)
wrote:

Okay, I think I figured it out. It is not per se a problem with the 3rd
party code, I can reproduce it with or without Thrift. The core problem is
that the inner class you want to instrument in the library is non-public.
To be exact, it is a private, non-static inner class. But what really
matters is that it is anything but public/protected, static or not is not
so important.

To Andy Clement: Maybe this is a shortcoming in AspectJ and we need a
Bugzilla ticket for it, but first I am going to post some sample code here:
------------------------------

package de.scrum_master.app;

public class Application {
  private static class InnerStaticRunnable implements Runnable {
    @Override
    public void run() {
      System.out.println("Running inner static runnable");
    }
  }

  private class InnerRunnable implements Runnable {
    @Override
    public void run() {
      System.out.println("Running inner runnable");
    }
  }

  public void doSomething() {
    new InnerRunnable().run();
  }

  public static void main(String[] args) {
    new SomeRunnable().run();
    new Application.InnerStaticRunnable().run();
    new Application().doSomething();
  }
}

------------------------------

package de.scrum_master.aspect;

privileged aspect MyRunnables {
  public interface InstrumentedRunnable {}
  private long InstrumentedRunnable.myid = -1;
  public long InstrumentedRunnable.getMyid() { return myid; }
  public void InstrumentedRunnable.setMyid(long id) { myid = id; }

  declare parents : Runnable+ implements InstrumentedRunnable;

  after() returning(InstrumentedRunnable r) :
call(java.lang.Runnable+.new(..)) {
    System.out.println(thisJoinPoint);
    System.out.println("  Runnable: " + r);
    System.out.println("  Has aspect: " + PerRunnable.hasAspect(r.getClass()));
    long id = PerRunnable.aspectOf(r.getClass()).getCounter();
    r.setMyid(id);
    PerRunnable.aspectOf(r.getClass()).incrementCounter();
  }
}

------------------------------

package de.scrum_master.aspect;

privileged aspect PerRunnable pertypewithin(java.lang.Runnable+) {
  public long counter = 0;
  public long getCounter() { return counter; }
  public void incrementCounter() { counter++; }

  after() : staticinitialization(*) {
    System.out.println("getWithinTypeName() = " + getWithinTypeName());
  }
}

------------------------------

Now let's run the code after Ajc compilation and check the console log:

getWithinTypeName() = de.scrum_master.app.SomeRunnable
call(de.scrum_master.app.SomeRunnable())
  Runnable: de.scrum_master.app.SomeRunnable@5674cd4d
  Has aspect: true
Running some runnable
getWithinTypeName() = de.scrum_master.app.Application$InnerStaticRunnable
call(de.scrum_master.app.Application.InnerStaticRunnable(Application.InnerStaticRunnable))
  Runnable: de.scrum_master.app.Application$InnerStaticRunnable@65b54208
  Has aspect: false
Exception in thread "main" org.aspectj.lang.NoAspectBoundException
    at de.scrum_master.aspect.PerRunnable.aspectOf(PerRunnable.aj:1)
    at 
de.scrum_master.aspect.MyRunnables.ajc$afterReturning$de_scrum_master_aspect_MyRunnables$1$8a935d86(MyRunnables.aj:15)
    at de.scrum_master.app.Application.main(Application.java:24)

------------------------------

Please note "Has aspect: false" right before the exception. Now change the
inner classes to public or protected and the code works:

getWithinTypeName() = de.scrum_master.app.SomeRunnable
call(de.scrum_master.app.SomeRunnable())
  Runnable: de.scrum_master.app.SomeRunnable@5674cd4d
  Has aspect: true
Running some runnable
getWithinTypeName() = de.scrum_master.app.Application$InnerStaticRunnable
call(de.scrum_master.app.Application.InnerStaticRunnable())
  Runnable: de.scrum_master.app.Application$InnerStaticRunnable@65b54208
  Has aspect: true
Running inner static runnable
getWithinTypeName() = de.scrum_master.app.Application$InnerRunnable
call(de.scrum_master.app.Application.InnerRunnable(Application))
  Runnable: de.scrum_master.app.Application$InnerRunnable@6b884d57
  Has aspect: true
Running inner runnable

------------------------------

Any comments, Andy?

-- 

Alexander Kriegisch
https://scrum-master.de



Yongle Zhang schrieb am 18.05.2018 04:03:

More information

   - The part of the code that’s causing the exception - this is decompiled
   from the woven .class file:

public class TThreadPoolServer {

    public void serve() {

  . . .

  TThreadPoolServer.WorkerProcess var13;

  TThreadPoolServer.WorkerProcess var10000 = var13 = new

  TThreadPoolServer.WorkerProcess(client, (<undefinedtype>)var10);

  // The exception says here afterReturning throws the exception


  
EpredRunnablesCallables.aspectOf().ajc$afterReturning$EpredRunnablesCallables$1$8a935d86(var13);

  . . .

  }

 // inner class
  private class WorkerProcess implements Runnable,
InstrumentedRunnableCallable {
        public long myid; // inserted by aspectj

        . . .

    }

}


   - Question: how does pertypewithin() work? what’s its scope?

For example, pertypewithin(Runnable+) - does it work every class in the
classpath, even including those in rt.jar? When does it create instance for
every class that implements Runnable (after class loading, on demand, …)?

Thank you!




On May 17, 2018 at 4:36:23 PM, Yongle Zhang (yng...@gmail.com) wrote:

Hi,
Problem

I have some aspects trying to insert an ID for every class that implements
Runnable.

My aspects (provided below) works fine for a simple test in which 1) I
wrote my own MyRunnable class implementing Runnable, 2) I have a simple
main function that creates and runs a thread using MyRunnable.

However, when I use it to instrument apache thrift library, it gives me
org.aspectj.lang.NoAspectBoundException exception.

I use compile-time weaving. The compile-time weaving finishes successfully,
and the instrumented .class code shows the aspects was woven. However,
running the instrumented apache thrift lib gives this excetpion:

(MyServer is my simple server implementation using thrift.
TThreadPoolServer is the server class in apache thrift lib.)

org.aspectj.lang.NoAspectBoundException
    at EpredPerRunnable.aspectOf(EpredPerRunnable.aj:1)
    at 
EpredRunnablesCallables.ajc$afterReturning$EpredRunnablesCallables$1$8a935d86(EpredRunnablesCallables.aj:55)
    at 
org.apache.thrift.server.TThreadPoolServer.serve(TThreadPoolServer.java:168)
    at MyServer.StartsimpleServer(MyServer.java:21)
    at MyServer.main(MyServer.java:28)

My Aspects

Here are the aspects I wrote:

1) I have a counter for each class implements Runnable using pertypewithin.

privileged aspect PerRunnable
    pertypewithin(java.lang.Runnable+)
{
  public long counter = 0;

  public long getCounter() {
    return counter;
  }

  public void incrementCounter() {
    counter++;
  }
}

2) I insert an id into each class that implements Runnable using interface.

privileged aspect MyRunnables {

  public interface InstrumentedRunnable {}

  private long InstrumentedRunnable.myid = -1;

  public long InstrumentedRunnable.getMyid() {
    return myid;
  }

  public void InstrumentedRunnable.setMyid(long id) {
    myid = id;
  }


  declare parents: (Runnable)+ implements InstrumentedRunnable;

  after() returning(InstrumentedRunnable r):
      call(java.lang.Runnable+.new(..)) {

      long id = PerRunnable.aspectOf(r.getClass()).getCounter();
      r.setMyid(id);

      PerRunnable.aspectOf(r.getClass()).incrementCounter();

  }

}

3) Part of my scripts that only instruments thrift:

CLASSPATH=$CLASSPATH:~/aspectj1.9/lib/aspectjtools.jar
CLASSPATH=$CLASSPATH:~/aspectj1.9/lib/aspectjrt.jar
AJC=~/aspectj1.9/bin/ajc

echo "Compiling Aspects ..."
$AJC -classpath $CLASSPATH:./lib/libthrift-0.11.0.jar -source 1.8 asp/*.aj

echo "Weaving aspect into thrift lib..."
$AJC -classpath
$CLASSPATH:./lib/servlet-api-2.5.jar:./lib/httpcore-4.4.1.jar:./lib/slf4j-api-1.7.12.jar:./lib/httpclient-4.4.1.jar
-source 1.8 -inpath ./lib/libthrift-0.11.0.jar -aspectpath ./asp/
-outjar ./my-libthrift-0.11.0.jar


4) Part of my scripts that starts the thrift server:

CLASSPATH=$CLASSPATH:~/aspectj1.9/lib/aspectjtools.jar
CLASSPATH=$CLASSPATH:~/aspectj1.9/lib/aspectjrt.jar

java -cp 
$CLASSPATH:./asp:./my-add-server.jar:./my-libthrift-0.11.0.jar:./lib/slf4j-api-1.7.12.jar
MyServer

Need Help

   1. Has anyone met such problem before? Any guess? Note that these
   aspects works for my own Runnable but not for thrift lib. (I can send more
   code needed including my test classes and my scripts, but they don’t fit
   within an email…)
   2. Is there a way to get all aspect instances and what they are matched
   to at runtime?
   3. Does aspectj has this feature: given 1) a pointcut, 2) the signature
   of a target (class/method) I want the pointcut to match, tell me whether
   they matched, and if not why.

Thank you for your time and help!







_______________________________________________
aspectj-users mailing list
aspectj-users@eclipse.org
To change your delivery options, retrieve your password, or unsubscribe
from this list, visit
https://dev.eclipse.org/mailman/listinfo/aspectj-users
_______________________________________________
aspectj-users mailing list
aspectj-users@eclipse.org
To change your delivery options, retrieve your password, or unsubscribe from 
this list, visit
https://dev.eclipse.org/mailman/listinfo/aspectj-users

Reply via email to