2

I've never worked with Concurrency library before.

public class QueueExecutor  {
 static final int defaultCorePoolSize = 5;
 static final int defaultMaximumPoolSize = 10;
 static final long defaultKeepAliveTime = 10;
 static final TimeUnit defaultTimeUnit = TimeUnit.MINUTES;
 static final BlockingQueue<Runnable> workQueue = new LinkedBlockingQueue<Runnable>();
 private static ThreadPoolExecutor instance;

 private QueueExecutor() {
  instance = new ThreadPoolExecutor(defaultCorePoolSize, defaultMaximumPoolSize, defaultKeepAliveTime, defaultTimeUnit, workQueue);
 }  

public static ThreadPoolExecutor getInstance() {
     if (instance == null) {
         QueueExecutor();
     }
     return instance;
 }

public static add(Runnable runnable){

} instance.execute(runnable);

}

My question is the following, If this object is running inside JBoss application container, should I synchronize add and getInstance functions and why? I think that these ThreadPoolExecutor is already syncronized.

danny.lesnik
  • 18,479
  • 29
  • 135
  • 200

2 Answers2

4

There are two things I can say.

  1. Declare instance final (and initialize it accordingly). Since it is static it will only be created on class initialization, and the only time you will invoke the class is to getInstance(). If you do that you don't need to worry about synchronization.

  2. The reason add does not need to be synchronized is because the execute method handles all the synchronization for you.

Those two points being said, it is suggested to avoid creating your own thread in a J2EE environment. You can read more here

Community
  • 1
  • 1
John Vint
  • 39,695
  • 7
  • 78
  • 108
  • Thanks. Why do I not have to synchronize if I declare instance as final? – danny.lesnik Nov 22 '11 at 17:32
  • The JLS specifies it in 17.5 specifically `Final fields also allow programmers to implement thread-safe immutable objects without synchronization.` You can read more on that here http://java.sun.com/docs/books/jls/third_edition/html/memory.html. – John Vint Nov 22 '11 at 17:34
  • In short, if the field is final, then you will garuntee that any thread reading the `instance` field will have the most up to date value. The absence of the final declaration may lead to a race/memory issue where one thread could see a different/cached value of `instance` – John Vint Nov 22 '11 at 17:36
  • 1
    The last, less obvious reason, is that class initialization is thread-safe. Only one thread can initialize a class at any time, and that initialization happens only once (per class instance). So only one thread will ever initialize `instance` – John Vint Nov 22 '11 at 17:38
  • Yep, final makes variable immutable. sorry for asking stupid question :) – danny.lesnik Nov 22 '11 at 17:40
2

The ThreadPoolExecutor is already thread-safe, but your class is not. Synchronize getInstance() or at least the block you're checking and initializing instance. add() doesn't have to be synchronized because it just delegates the operation, but I would prefer using getInstance().execute(runnable), so you won't get a NullPointerException.

Community
  • 1
  • 1
electrodraco
  • 323
  • 2
  • 6
  • what do you mean by "class is not synchronized"? Could you please explain, what is the problem if my class is not Synchronized? – danny.lesnik Nov 22 '11 at 17:46
  • If the first two Threads enter the `getInstance()` method at the same time, there is a very low chance that they get both `true` from `instance == null` and instantiate two different `ThreadPoolExecutor`. – electrodraco Nov 22 '11 at 23:00
  • thanks, but why could it happen? ThreadPoolExecutor is already syncronized object. am I wrong? – danny.lesnik Nov 23 '11 at 09:33
  • If you want to synchronize two Threads, you'll have to lock over a monitor, which is a particular instance. The first time a thread enters your methods, there is no instance at all, so the check instance == null cannot be synchronized. Even if there is an instance set, dealing with the reference doesn't mean you get the lock over the instance, so you'll have to lock over another monitor, which is always the same and exists from the very beginning: The Class you've written. – electrodraco Nov 25 '11 at 11:37