| 1845039 |
(M6) Jacob DLL names are now qualified by platform and
release. The JacobLibraryLoader now determines the correct 32bit or 64bit
diff --git a/jacob/src/com/jacob/com/JacobObject.java b/jacob/src/com/jacob/com/JacobObject.java
index 4715da9..3b96363 100644
--- a/jacob/src/com/jacob/com/JacobObject.java
+++ b/jacob/src/com/jacob/com/JacobObject.java
@@ -96,8 +96,8 @@ public static String getBuildVersion() {
*/
protected static void debug(String istrMessage) {
if (isDebugEnabled()) {
- System.out.println(istrMessage + " in thread "
- + Thread.currentThread().getName());
+ System.out.println(Thread.currentThread().getName() + ": "
+ + istrMessage);
}
}
diff --git a/jacob/src/com/jacob/com/ROT.java b/jacob/src/com/jacob/com/ROT.java
index f71ba4e..9d81580 100644
--- a/jacob/src/com/jacob/com/ROT.java
+++ b/jacob/src/com/jacob/com/ROT.java
@@ -95,7 +95,7 @@ protected synchronized static Map addThread() {
}
/**
- * returns the pool for this thread if it exists. can create a new one if
+ * Returns the pool for this thread if it exists. can create a new one if
* you wish by passing in TRUE
*
* @param createIfDoesNotExist
@@ -115,19 +115,20 @@ protected synchronized static Map getThreadObjects(
* Iterates across all of the entries in the Hashmap in the rot that
* corresponds to this thread. This calls safeRelease() on each entry and
* then clears the map when done and removes it from the rot. All traces of
- * this thread's objects will disapear. This is called by COMThread in the
+ * this thread's objects will disappear. This is called by COMThread in the
* tear down and provides a synchronous way of releasing memory
*/
- protected synchronized static void clearObjects() {
- Map tab = getThreadObjects(false);
+ protected static void clearObjects() {
if (JacobObject.isDebugEnabled()) {
JacobObject.debug("ROT: " + rot.keySet().size()
+ " thread tables exist");
}
+
+ Map tab = getThreadObjects(false);
if (tab != null) {
if (JacobObject.isDebugEnabled()) {
JacobObject.debug("ROT: " + tab.keySet().size()
- + " objects to clear in this thread ");
+ + " objects to clear in this thread's ROT ");
}
// walk the values
Iterator it = tab.keySet().iterator();
@@ -155,13 +156,11 @@ protected synchronized static void clearObjects() {
}
o.safeRelease();
}
- // used to be an iterator.remove() but why bother when we're
- // nuking them all anyway?
}
// empty the collection
tab.clear();
// remove the collection from rot
- rot.remove(Thread.currentThread().getName());
+ ROT.removeThread();
if (JacobObject.isDebugEnabled()) {
JacobObject.debug("ROT: thread table cleared and removed");
}
@@ -172,19 +171,28 @@ protected synchronized static void clearObjects() {
}
}
+ /**
+ * Removes the map from the rot that is associated with the current thread.
+ */
+ private synchronized static void removeThread() {
+ // should this see if it exists first?
+ rot.remove(Thread.currentThread().getName());
+ }
+
/**
* @deprecated the java model leave the responsibility of clearing up
* objects to the Garbage Collector. Our programming model
* should not require that the user specifically remove object
- * from the thread.
- *
- * This will remove an object from the ROT
+ * from the thread.
+ * This will remove an object from the ROT
+ * This does not need to be synchronized because only the rot
+ * modification related methods need to synchronized. Each
+ * individual map is only modified in a single thread.
* @param o
*/
@Deprecated
- protected synchronized static void removeObject(JacobObject o) {
- String t_name = Thread.currentThread().getName();
- Map tab = rot.get(t_name);
+ protected static void removeObject(JacobObject o) {
+ Map tab = ROT.getThreadObjects(false);
if (tab != null) {
tab.remove(o);
}
@@ -192,11 +200,23 @@ protected synchronized static void removeObject(JacobObject o) {
}
/**
- * adds an object to the HashMap for the current thread
+ * Adds an object to the HashMap for the current thread.
+ *
+ * This method does not need to be threaded because the only concurrent
+ * modification risk is on the hash map that contains all of the thread
+ * related hash maps. The individual thread related maps are only used on a
+ * per thread basis so there isn't a locking issue.
+ *
+ * In addition, this method cannot be threaded because it calls
+ * ComThread.InitMTA. The ComThread object has some methods that call ROT so
+ * we could end up deadlocked. This method should be safe without the
+ * synchronization because the ROT works on per thread basis and the methods
+ * that add threads and remove thread related entries are all synchronized
+ *
*
* @param o
*/
- protected synchronized static void addObject(JacobObject o) {
+ protected static void addObject(JacobObject o) {
// check the system property to see if this class is put in the ROT
// the default value is "true" which simulates the old behavior
String shouldIncludeClassInROT = System.getProperty(o.getClass()
@@ -208,11 +228,14 @@ protected synchronized static void addObject(JacobObject o) {
+ o.getClass().getName() + " not added to ROT");
}
} else {
+ // first see if we have a table for this thread
Map tab = getThreadObjects(false);
if (tab == null) {
// this thread has not been initialized as a COM thread
// so make it part of MTA for backwards compatibility
ComThread.InitMTA(false);
+ // don't really need the "true" because the InitMTA will have
+ // called back to the ROT to create a table for this thread
tab = getThreadObjects(true);
}
if (JacobObject.isDebugEnabled()) {
@@ -220,6 +243,7 @@ protected synchronized static void addObject(JacobObject o) {
+ o.getClass().getName()
+ " table size prior to addition:" + tab.size());
}
+ // add the object to the table that is specific to this thread
if (tab != null) {
tab.put(o, null);
}
diff --git a/jacob/unittest/com/jacob/com/JacobDeadlockTest.java b/jacob/unittest/com/jacob/com/JacobDeadlockTest.java
new file mode 100644
index 0000000..1639fed
--- /dev/null
+++ b/jacob/unittest/com/jacob/com/JacobDeadlockTest.java
@@ -0,0 +1,120 @@
+package com.jacob.com;
+
+import com.jacob.test.BaseTestCase;
+
+/**
+ * Sourceforge defect report 1986987 July 2008. This test case demonstrated a
+ * deadlock issue.
+ *
+ * - One thread attempts to create an object in a thread where InitMTA has
+ * not been called. This results in ROT.addObject being called which then calls
+ * ComThread.InitMTA
+ *
- One thread attempts to call ComThread.release() which then calls ROT
+ * .clear objects.
+ *
+ * The result is one thread with a call sequence ComThread--ROT and the other
+ * with a sequence ROT--ComThread resulting in deadlock.
+ *
+ * This test will fail with debug logging turned on because of the amount of
+ * time it takes to write the debug output.
+ *
+ * @author jsamarziya
+ *
+ */
+public class JacobDeadlockTest extends BaseTestCase {
+ private static final long TIMEOUT = 5000l;
+
+ /** Thread component */
+ public static class TestThread extends Thread {
+ private final int id;
+ private final boolean initCOM;
+ private final boolean writeOutput;
+
+ /**
+ * constructor for ThestThread
+ *
+ * @param id
+ * @param initCOM
+ * @param writeOutput
+ *
+ */
+ public TestThread(int id, boolean initCOM, boolean writeOutput) {
+ this.id = id;
+ this.initCOM = initCOM;
+ this.writeOutput = writeOutput;
+ }
+
+ @Override
+ public void run() {
+ for (int i = 0; i < 1000; i++) {
+ log("iteration " + i);
+ if (initCOM) {
+ log("Initializing COM thread");
+ ComThread.InitMTA(false);
+ }
+ log("Creating JacobObject");
+ new JacobObject();
+ log("Releasing COM thread");
+ ComThread.Release();
+ }
+ log("Exiting Java Thread");
+ }
+
+ private void log(String message) {
+ if (writeOutput) {
+ System.out.println(Thread.currentThread().getName()
+ + ": TestThread[" + id + "] " + " " + " - " + message);
+ }
+ }
+ }
+
+ /**
+ * This test shows that if ComThread.Init() is called explicitly, no problem
+ * occurs.
+ *
+ * @throws InterruptedException
+ */
+ public void testShowNoProblemIfCOMIsInitialized()
+ throws InterruptedException {
+ runTest(2, true, false);
+ runTest(100, true, false);
+ }
+
+ /**
+ * This test shows that if only one thread is creating COM objects, no
+ * problem occurs.
+ *
+ * @throws InterruptedException
+ */
+ public void testShowNoProblemIfSingleThreaded() throws InterruptedException {
+ runTest(1, false, false);
+ runTest(1, true, false);
+ }
+
+ /**
+ * Runs the test with two threads, which don't initialize the COM thread.
+ *
+ * This test will always fail.
+ *
+ * @throws InterruptedException
+ */
+ public void testShowDeadlockProblem() throws InterruptedException {
+ runTest(2, false, true);
+ }
+
+ private void runTest(int numberOfThreads, boolean initCOM,
+ boolean writeOutput) throws InterruptedException {
+ Thread[] threads = new Thread[numberOfThreads];
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new TestThread(i, initCOM, writeOutput);
+ threads[i].start();
+ }
+ for (int i = 0; i < threads.length; i++) {
+ threads[i].join(TIMEOUT);
+ if (threads[i].isAlive()) {
+ fail("thread " + i + " failed to finish in " + TIMEOUT
+ + " milliseconds");
+ }
+ }
+ }
+}
|