You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by wa...@apache.org on 2007/09/13 20:36:07 UTC
svn commit: r575397 -
/felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java
Author: walkerr
Date: Thu Sep 13 11:36:07 2007
New Revision: 575397
URL: http://svn.apache.org/viewvc?rev=575397&view=rev
Log:
Put back in "wait" step on event shutdown, but used dedicated shutdown lock outside of other synchronosed blocks to help avoid deadlocks (FELIX-363).
Modified:
felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java
Modified: felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java
URL: http://svn.apache.org/viewvc/felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java?rev=575397&r1=575396&r2=575397&view=diff
==============================================================================
--- felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java (original)
+++ felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java Thu Sep 13 11:36:07 2007
@@ -62,9 +62,10 @@
// A single thread is used to deliver events for all dispatchers.
private static Thread m_thread = null;
private static String m_threadLock = "thread lock";
+ private static String m_shutdownLock = "thread shutdown lock";
private static int m_references = 0;
private static boolean m_stopping = false;
- private static boolean m_stopped = false;
+
// List of requests.
private static final ArrayList m_requestList = new ArrayList();
// Pooled requests to avoid memory allocation.
@@ -84,24 +85,32 @@
// Start event dispatching thread if necessary.
if (m_thread == null || !m_thread.isAlive())
{
+ m_stopping = false;
+
m_thread = new Thread(new Runnable() {
public void run()
{
EventDispatcher.run();
// Ensure we update state even if stopped by external cause
// e.g. an Applet VM forceably killing threads
- m_thread = null;
- m_stopped = true;
- m_stopping = false;
+ synchronized (m_threadLock)
+ {
+ m_thread = null;
+ m_stopping = false;
+ m_references = 0;
+ }
+
+ synchronized (m_shutdownLock)
+ {
+ m_shutdownLock.notifyAll();
+ }
}
}, "FelixDispatchQueue");
m_thread.start();
}
-
+
// reference counting and flags
m_references++;
- m_stopping = false;
- m_stopped = false;
}
return eventDispatcher;
@@ -111,8 +120,8 @@
{
synchronized (m_threadLock)
{
- // Return if already stopped.
- if (m_stopped)
+ // Return if already dead or stopping.
+ if (m_thread == null || m_stopping)
{
return;
}
@@ -123,12 +132,28 @@
{
return;
}
-
- // Signal dispatch thread.
+
m_stopping = true;
- synchronized (m_requestList)
+ }
+
+ // Signal dispatch thread.
+ synchronized (m_requestList)
+ {
+ m_requestList.notify();
+ }
+
+ // Use separate lock for shutdown to prevent any chance of nested lock deadlock
+ synchronized (m_shutdownLock)
+ {
+ while (m_thread != null)
{
- m_requestList.notify();
+ try
+ {
+ m_shutdownLock.wait();
+ }
+ catch (InterruptedException ex)
+ {
+ }
}
}
}
@@ -531,8 +556,9 @@
private void fireEventAsynchronously(
Logger logger, int type, Object[] listeners, EventObject event)
{
+ //TODO: should possibly check this within thread lock, seems to be ok though without
// If dispatch thread is stopped, then ignore dispatch request.
- if (m_stopped || m_stopping)
+ if (m_stopping || m_thread == null)
{
return;
}
Re: svn commit: r575397 - /felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java
Posted by Karl Pauls <ka...@gmail.com>.
Rob,
I tried to simplify the code somewhat and do the clean-up in a finally
block. It now uses the m_threadLock instead of the m_shutdownLock. I
do think that should work correctly.
Could you retry this with your set-up? In case something is broken
again I rollback to your version. Thanks!
regards,
Karl
On 9/13/07, walkerr@apache.org <wa...@apache.org> wrote:
> Author: walkerr
> Date: Thu Sep 13 11:36:07 2007
> New Revision: 575397
>
> URL: http://svn.apache.org/viewvc?rev=575397&view=rev
> Log:
> Put back in "wait" step on event shutdown, but used dedicated shutdown lock outside of other synchronosed blocks to help avoid deadlocks (FELIX-363).
>
> Modified:
> felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java
>
> Modified: felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java
> URL: http://svn.apache.org/viewvc/felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java?rev=575397&r1=575396&r2=575397&view=diff
> ==============================================================================
> --- felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java (original)
> +++ felix/trunk/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java Thu Sep 13 11:36:07 2007
> @@ -62,9 +62,10 @@
> // A single thread is used to deliver events for all dispatchers.
> private static Thread m_thread = null;
> private static String m_threadLock = "thread lock";
> + private static String m_shutdownLock = "thread shutdown lock";
> private static int m_references = 0;
> private static boolean m_stopping = false;
> - private static boolean m_stopped = false;
> +
> // List of requests.
> private static final ArrayList m_requestList = new ArrayList();
> // Pooled requests to avoid memory allocation.
> @@ -84,24 +85,32 @@
> // Start event dispatching thread if necessary.
> if (m_thread == null || !m_thread.isAlive())
> {
> + m_stopping = false;
> +
> m_thread = new Thread(new Runnable() {
> public void run()
> {
> EventDispatcher.run();
> // Ensure we update state even if stopped by external cause
> // e.g. an Applet VM forceably killing threads
> - m_thread = null;
> - m_stopped = true;
> - m_stopping = false;
> + synchronized (m_threadLock)
> + {
> + m_thread = null;
> + m_stopping = false;
> + m_references = 0;
> + }
> +
> + synchronized (m_shutdownLock)
> + {
> + m_shutdownLock.notifyAll();
> + }
> }
> }, "FelixDispatchQueue");
> m_thread.start();
> }
> -
> +
> // reference counting and flags
> m_references++;
> - m_stopping = false;
> - m_stopped = false;
> }
>
> return eventDispatcher;
> @@ -111,8 +120,8 @@
> {
> synchronized (m_threadLock)
> {
> - // Return if already stopped.
> - if (m_stopped)
> + // Return if already dead or stopping.
> + if (m_thread == null || m_stopping)
> {
> return;
> }
> @@ -123,12 +132,28 @@
> {
> return;
> }
> -
> - // Signal dispatch thread.
> +
> m_stopping = true;
> - synchronized (m_requestList)
> + }
> +
> + // Signal dispatch thread.
> + synchronized (m_requestList)
> + {
> + m_requestList.notify();
> + }
> +
> + // Use separate lock for shutdown to prevent any chance of nested lock deadlock
> + synchronized (m_shutdownLock)
> + {
> + while (m_thread != null)
> {
> - m_requestList.notify();
> + try
> + {
> + m_shutdownLock.wait();
> + }
> + catch (InterruptedException ex)
> + {
> + }
> }
> }
> }
> @@ -531,8 +556,9 @@
> private void fireEventAsynchronously(
> Logger logger, int type, Object[] listeners, EventObject event)
> {
> + //TODO: should possibly check this within thread lock, seems to be ok though without
> // If dispatch thread is stopped, then ignore dispatch request.
> - if (m_stopped || m_stopping)
> + if (m_stopping || m_thread == null)
> {
> return;
> }
>
>
>
--
Karl Pauls
karlpauls@gmail.com