From fbc820c6f2bcd9920a729e16abc33af869a60216 Mon Sep 17 00:00:00 2001
From: adam <adam@megacz.com>
Date: Sun, 10 Oct 2010 19:07:56 +0000
Subject: [PATCH] fix ThreadPool bug

Ignore-this: 520059a0cd173fd45b33989c46764c5a

darcs-hash:20101010190756-5007d-272acdb4c122f2895921154cc32ff6b2173ae036.gz
---
 src/org/ibex/util/Log.java        |    2 +-
 src/org/ibex/util/ThreadPool.java |   58 ++++++++++++++++++++-----------------
 2 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/src/org/ibex/util/Log.java b/src/org/ibex/util/Log.java
index 175a8df..dc914c2 100644
--- a/src/org/ibex/util/Log.java
+++ b/src/org/ibex/util/Log.java
@@ -125,7 +125,7 @@ public class Log {
 
     public static void printStackTrace(Object o, int level) {
         try {
-            throw new Exception();
+            throw new Exception("just printing a stack trace; no real problem");
         } catch (Exception e) {
             log(o, e, level);
         }
diff --git a/src/org/ibex/util/ThreadPool.java b/src/org/ibex/util/ThreadPool.java
index 0fb6d07..1c8d6bd 100644
--- a/src/org/ibex/util/ThreadPool.java
+++ b/src/org/ibex/util/ThreadPool.java
@@ -1,4 +1,4 @@
-// Copyright 2000-2005 the Contributors, as shown in the revision logs.
+// Copyright 2000-2009 the Contributors, as shown in the revision logs.
 // Licensed under the Apache Public Source License 2.0 ("the License").
 // You may not use this file except in compliance with the License.
 
@@ -18,6 +18,8 @@ import java.io.*;
  *     threads exist overall, any threads which complete their task will
  *     be allowed to die.
  *
+ *   Locking order: never attempt to acquire a lock on a PooledThread
+ *   while holding the ThreadPool lock.
  */
 public final class ThreadPool {
 
@@ -31,48 +33,49 @@ public final class ThreadPool {
 
     public ThreadPool(int minThreads) {
         this.minThreads     = minThreads;
-        this.numThreads     = 0;
+        this.numThreads     = minThreads;
         this.numIdleThreads = 0;
         this.idleThreads    = new PooledThread[minThreads];
         for(int i=0; i<this.idleThreads.length; i++)
             new PooledThread();
     }
 
-    public synchronized void start(Runnable r) {
-        /* if no idle threads, wait for MINIMUM_DISPATCH_DELAY or until notified */
-        if (numIdleThreads == 0) try {
-            this.wait(MINIMUM_DISPATCH_DELAY);
-        } catch (Exception e) { Log.error(this, e); }
-
-        /* if there are idle threads, use one */
-        if (numIdleThreads > 0) {
-            numIdleThreads--;
-            idleThreads[numIdleThreads].start(r);
-            idleThreads[numIdleThreads] = null;
-            return;
+    public void start(Runnable r) {
+        synchronized(ThreadPool.this) {
+            /* if no idle threads, wait for MINIMUM_DISPATCH_DELAY or until notified */
+            if (numIdleThreads == 0) try {
+                this.wait(MINIMUM_DISPATCH_DELAY);
+            } catch (Exception e) { Log.error(this, e); }
+            
+            /* if there are idle threads, use one */
+            if (numIdleThreads > 0) {
+                numIdleThreads--;
+                idleThreads[numIdleThreads].start(r);
+                idleThreads[numIdleThreads] = null;
+                return;
+            }
+            /* otherwise, create a new thread */
+            numThreads++;
         }
-
-        /* otherwise, create a new thread */
         new PooledThread().start(r);
     }
 
     private class PooledThread extends Thread {
         private Runnable runnable = null;
-        public PooledThread() {
-            synchronized(ThreadPool.this) { numThreads++; }
-            start();
-        }
-        public synchronized void start(Runnable runnable) {
-            if (this.runnable != null)
-                throw new Error("start() on a thread that is alread busy!");
-            this.runnable = runnable;
-            notify();
+        public PooledThread() { start(); }
+        void start(Runnable runnable) {
+            synchronized(this) {
+                if (this.runnable != null)
+                    throw new Error("start() on a thread that is alread busy!");
+                this.runnable = runnable;
+                notify();
+            }
         }
         public void run() {
             try {
                 while(true) {
                     synchronized(this) {
-                        if (runnable==null)
+                        if (runnable==null) {
                             synchronized(ThreadPool.this) {
                                 /* if the idle array is full, just let ourselves die */
                                 if (numIdleThreads > minIdleThreads && numThreads > minThreads) return;
@@ -80,12 +83,13 @@ public final class ThreadPool {
                                 idleThreads[numIdleThreads++] = this;
                                 ThreadPool.this.notifyAll();
                             }
+                        }
                     }
                     try {
                         synchronized(this) { while (runnable==null) wait(); }
                         runnable.run();
+                        synchronized(this) { runnable = null; }
                     } catch (Exception e) { Log.error(this, e); }
-                    runnable = null;
                 }
             } finally {
                 synchronized(ThreadPool.this) {
-- 
1.7.10.4