Java basic thread pool implementation with locks.ReentrantLock

Question

I'm new at Java. I was just experimenting with threads, and I wanted to create something like a Thread Pool (if this is actually what I am doing..).

Basically I have a while loop which fires Threads until there are still tasks to be executed && while max concurrent threads is not greater than n. Each thread use java.util.concurrent.locks.ReentrantLock to provide a lock around the the task count variable which is decreased in each thread and the thread count variable which is increased as soon the thread starts and decreased just before the thread ends(code smell?):

import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

public class Test { 
   public static void main(String[] args) {

      //overcome limitation of closure not being able to modify outer variable with arr.
      final int[] runningThreads = {0};
      final int[] taskcount = {10};

      final Lock _mutex = new ReentrantLock(true);

      int maxThreadQty = 3;


      while ((taskcount[0] > 0) && (runningThreads[0] < maxThreadQty))  {

         new Thread("T") {
             public void run() {
                  System.out.println("New Thread Started");
                  _mutex.lock();
                  runningThreads[0]++;
                  System.out.println("Running Threads: " + runningThreads[0]);
                  System.out.println("Times to go: " + taskcount[0]);
                  _mutex.unlock();
                  // actually do something;
                  _mutex.lock();
                  taskcount[0]--;
                  runningThreads[0]--;
                  _mutex.unlock();
             }

          }.start();
      }
   }
}

When I run the code, new Threads keep being fired forever and task count is only decreased like two or three times......

Last lines of output (read times to go as tasks to go):

Running Threads: 565
Times to go: 8
Running Threads: 566
Times to go: 8
Running Threads: 567
Times to go: 8
Running Threads: 568
Times to go: 8
Running Threads: 569
Times to go: 8
Running Threads: 570
Times to go: 8
Running Threads: 571
Times to go: 8
Running Threads: 572
Times to go: 8
Running Threads: 573
Times to go: 8
Running Threads: 574
Times to go: 8
Running Threads: 575
Times to go: 8

CTRL-C

I am sure there must be something totally wrong with the way I am using Threads or locks.. but as a java newbie there's so many things that I could be missing out (probably even most basic ones) that some help and some putting me back on the right path would be very appreciated...! Thank you.

I used this as a reference for threads: http://tutorials.jenkov.com/java-concurrency/creating-and-starting-threads.html

And then this stackoverflow answer to see how to use ReentrantLock: https://stackoverflow.com/a/12510490/988591

This for the closure not being able to modify outer variables workaround (using arrays values): http://c2.com/cgi/wiki?ClosuresThatWorkAroundFinalLimitation

Answer 1

Couldn't you use the built in thread pool functionality?

If not, the problem is that runningThreads doesn't get increased until each thread starts and has acquired the lock. In practice the main thread may run for quite some time, all the while spinning up new threads without limit.

One solution might be to increase the runningThreads variable on the main thread, just before starting the new thread, but leave decreasing the variable inside each of the worker threads.

I don't want to suggest that everything else about your code is "good" (creating a robust thread-pool implementation can be quite a difficult and involved task) but a minimal change that may avoid the problem could be

import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

public class Test { 
   public static void main(String[] args) {

      //overcome limitation of closure not being able to modify outer variable with arr.
      final int[] runningThreads = {0};
      final int[] taskcount = {10};

      final Lock _mutex = new ReentrantLock(true);

      int maxThreadQty = 3;


      while ((taskcount[0] > 0) && (runningThreads[0] < maxThreadQty))  {
         System.out.println("New Thread Started");
         _mutex.lock();
         runningThreads[0]++;
         System.out.println("Running Threads: " + runningThreads[0]);
         System.out.println("Times to go: " + taskcount[0]);
         _mutex.unlock();
         new Thread("T") {
             public void run() {
                  // actually do something;
                  _mutex.lock();
                  taskcount[0]--;
                  runningThreads[0]--;
                  _mutex.unlock();
             }

          }.start();
      }
   }
}