commits@javamail.java.net

[mercurial:75] Fix notification deadlock introduced by synchronization "fix" - bug 6738

From: <shannon_at_kenai.com>
Date: Tue, 11 Nov 2008 01:05:06 +0000 (GMT)

Repository: mercurial
Revision: 75
Author: Bill Shannon <bill.shannon_at_sun.com>
Date: 2008-08-18 23:13:16 UTC

Log Message:
-----------
Fix notification deadlock introduced by synchronization "fix" - bug
6738454

Modified Paths:
--------------
    doc/release/CHANGES.txt
    mail/src/main/java/javax/mail/Service.java

Diffs:
-----
diff -r 47fa61b3ac79 -r 39a2c0d71d80 doc/release/CHANGES.txt
--- a/doc/release/CHANGES.txt Mon Aug 18 16:02:06 2008 -0700
+++ b/doc/release/CHANGES.txt Mon Aug 18 16:13:16 2008 -0700
@@ -25,6 +25,7 @@
 6720896 add mail.mime.uudecode.ignoreerrors system property
 6720896 add mail.mime.uudecode.ignoremissingbeginend system
property
 6730637 deadlock when mail.imap.connectionpoolsize greater than
1
+6738454 deadlock when connection is broken
 GF 3929 Inconsistent synchronization in
com.sun.mail.iap.Protocol
 GF 4997 BASE64DecoderStream.skip (etc) skips the wrong number
of octets
 GF 5189 Can't specify SSLSocketFactory for STARTTLS in Javamail
1.4
diff -r 47fa61b3ac79 -r 39a2c0d71d80
mail/src/main/java/javax/mail/Service.java
--- a/mail/src/main/java/javax/mail/Service.java Mon Aug 18
16:02:06 2008 -0700
+++ b/mail/src/main/java/javax/mail/Service.java Mon Aug 18
16:13:16 2008 -0700
@@ -73,7 +73,15 @@
     protected boolean debug = false;
 
     private boolean connected = false;
- private Vector connectionListeners = null;
+
+ /*
+ * connectionListeners is a Vector, initialized here,
+ * because we depend on it always existing and depend
+ * on the synchronization that Vector provides.
+ * (Sychronizing on the Service object itself can cause
+ * deadlocks when notifying listeners.)
+ */
+ private Vector connectionListeners = new Vector();
 
     /**
      * Constructor.
@@ -474,9 +482,7 @@
      * @param l the Listener for Connection events
      * @see javax.mail.event.ConnectionEvent
      */
- public synchronized void addConnectionListener(ConnectionListener
l) {
- if (connectionListeners == null)
- connectionListeners = new Vector();
+ public void addConnectionListener(ConnectionListener l) {
        connectionListeners.addElement(l);
     }
 
@@ -489,9 +495,8 @@
      * @param l the listener
      * @see #addConnectionListener
      */
- public synchronized void
removeConnectionListener(ConnectionListener l) {
- if (connectionListeners != null)
- connectionListeners.removeElement(l);
+ public void removeConnectionListener(ConnectionListener l) {
+ connectionListeners.removeElement(l);
     }
 
     /**
@@ -504,8 +509,13 @@
      * ConnectionListeners. Note that the event dispatching occurs
      * in a separate thread, thus avoiding potential deadlock
problems.
      */
- protected synchronized void notifyConnectionListeners(int type) {
- if (connectionListeners != null) {
+ protected void notifyConnectionListeners(int type) {
+ /*
+ * Don't bother queuing an event if there's no listeners.
+ * Yes, listeners could be removed after checking, which
+ * just makes this an expensive no-op.
+ */
+ if (connectionListeners.size() > 0) {
            ConnectionEvent e = new ConnectionEvent(this, type);
            queueEvent(e, connectionListeners);
        }