From e8c320c6c31237d0c930ea1c6d77d4bd5f691e4f Mon Sep 17 00:00:00 2001
From: Trustin Lee <trustin@gmail.com>
Date: Fri, 19 Feb 2010 08:23:48 +0000
Subject: [PATCH] * Removed 'volatile' from the member variables that are
 protected by synchronized (this) block * Updated comments regarding thread
 safety

---
 .../jboss/netty/channel/DefaultChannelFuture.java    | 12 +++++++-----
 .../channel/group/DefaultChannelGroupFuture.java     | 12 +++++++-----
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/main/java/org/jboss/netty/channel/DefaultChannelFuture.java b/src/main/java/org/jboss/netty/channel/DefaultChannelFuture.java
index ce8ce91b6a..87c743254b 100644
--- a/src/main/java/org/jboss/netty/channel/DefaultChannelFuture.java
+++ b/src/main/java/org/jboss/netty/channel/DefaultChannelFuture.java
@@ -71,8 +71,8 @@ public class DefaultChannelFuture implements ChannelFuture {
     private final Channel channel;
     private final boolean cancellable;
 
-    private volatile ChannelFutureListener firstListener;
-    private volatile List<ChannelFutureListener> otherListeners;
+    private ChannelFutureListener firstListener;
+    private List<ChannelFutureListener> otherListeners;
     private boolean done;
     private Throwable cause;
     private int waiters;
@@ -345,9 +345,11 @@ public class DefaultChannelFuture implements ChannelFuture {
     }
 
     private void notifyListeners() {
-        // There won't be any visibility problem or concurrent modification
-        // because 'ready' flag will be checked against both addListener and
-        // removeListener calls.
+        // This method doesn't need synchronization because:
+        // 1) This method is always called after synchronized (this) block.
+        //    Hence any listener list modification happens-before this method.
+        // 2) This method is only when 'done' is true.  If 'done' is true,
+        //    the listener list is never modified - see add/removeListener().
         if (firstListener != null) {
             notifyListener(firstListener);
             firstListener = null;
diff --git a/src/main/java/org/jboss/netty/channel/group/DefaultChannelGroupFuture.java b/src/main/java/org/jboss/netty/channel/group/DefaultChannelGroupFuture.java
index 08a5d4f538..4c333ab472 100644
--- a/src/main/java/org/jboss/netty/channel/group/DefaultChannelGroupFuture.java
+++ b/src/main/java/org/jboss/netty/channel/group/DefaultChannelGroupFuture.java
@@ -48,8 +48,8 @@ public class DefaultChannelGroupFuture implements ChannelGroupFuture {
 
     private final ChannelGroup group;
     final Map<Integer, ChannelFuture> futures;
-    private volatile ChannelGroupFutureListener firstListener;
-    private volatile List<ChannelGroupFutureListener> otherListeners;
+    private ChannelGroupFutureListener firstListener;
+    private List<ChannelGroupFutureListener> otherListeners;
     private boolean done;
     int successCount;
     int failureCount;
@@ -346,9 +346,11 @@ public class DefaultChannelGroupFuture implements ChannelGroupFuture {
     }
 
     private void notifyListeners() {
-        // There won't be any visibility problem or concurrent modification
-        // because 'ready' flag will be checked against both addListener and
-        // removeListener calls.
+        // This method doesn't need synchronization because:
+        // 1) This method is always called after synchronized (this) block.
+        //    Hence any listener list modification happens-before this method.
+        // 2) This method is only when 'done' is true.  If 'done' is true,
+        //    the listener list is never modified - see add/removeListener().
         if (firstListener != null) {
             notifyListener(firstListener);
             firstListener = null;