From aaf3f3e3322ac284dfe09d1f7154a572031d6445 Mon Sep 17 00:00:00 2001
From: Steven Luo <steven+android@steven676.net>
Date: Sun, 5 Jun 2011 02:29:04 -0700
Subject: [PATCH] Prevent race between IRCConnection dispose() and
 onDisconnect() when quitting

When the user asks for a disconnect from the ConversationActivity, there
is a race between the IRCConnection, which is waiting for the server to
acknowledge the QUIT before calling onDisconnect(), and the IRCService,
which will invoke dispose() on the IRCConnection when
checkServiceStatus() is called during the activity shutdown.  If the
dispose() wins, the thread running the onDisconnect() is terminated,
leading to the cleanup being unfinished.  This causes the disconnect
notification to be unreliable, and can result in the list of servers in
the ongoing notification to be out of sync with reality.

To fix this, introduce a new field isQuitting to the IRCConnection,
which is set to true when quitServer() is called and cleared once
onDisconnect() has finished.  If dispose() is called while isQuitting is
set, it sets disposeRequested instead of doing the dispose itself, and
onDisconnect() will call through to super.dispose() once it's finished.

Note that this requires a change to PircBot to allow the overriding of
quitServer(String message), which is declared final upstream.
---
 .../src/org/jibble/pircbot/PircBot.java       |  3 +-
 .../src/org/yaaic/irc/IRCConnection.java      | 38 ++++++++++++++++++-
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/application/src/org/jibble/pircbot/PircBot.java b/application/src/org/jibble/pircbot/PircBot.java
index 63e37c7..7ea9f34 100644
--- a/application/src/org/jibble/pircbot/PircBot.java
+++ b/application/src/org/jibble/pircbot/PircBot.java
@@ -357,7 +357,8 @@ public abstract class PircBot implements ReplyConstants {
      *
      * @param reason The reason for quitting the server.
      */
-    public final void quitServer(String reason) {
+    // XXX PircBot patch -- we need to override this method in Yaaic
+    public void quitServer(String reason) {
         this.sendRawLine("QUIT :" + reason);
     }
 
diff --git a/application/src/org/yaaic/irc/IRCConnection.java b/application/src/org/yaaic/irc/IRCConnection.java
index f8fe944..f644e89 100644
--- a/application/src/org/yaaic/irc/IRCConnection.java
+++ b/application/src/org/yaaic/irc/IRCConnection.java
@@ -53,6 +53,10 @@ public class IRCConnection extends PircBot
     private ArrayList<String> autojoinChannels;
     private Pattern mNickMatch;
 
+    private boolean isQuitting = false;
+    private boolean disposeRequested = false;
+    private Object isQuittingLock = new Object();
+
     /**
      * Create a new connection
      * 
@@ -1136,6 +1140,12 @@ public class IRCConnection extends PircBot
             );
             service.sendBroadcast(cIntent);
         }
+
+        synchronized(isQuittingLock) {
+            isQuitting = false;
+            if (disposeRequested)
+                super.dispose();
+        }
     }
 
     /**
@@ -1210,14 +1220,29 @@ public class IRCConnection extends PircBot
     @Override
     public void quitServer()
     {
+        quitServer(service.getSettings().getQuitMessage());
+    }
+
+    @Override
+    public void quitServer(final String message)
+    {
+        synchronized(isQuittingLock) {
+            isQuitting = true;
+        }
+
         new Thread() {
             @Override
             public void run() {
-                quitServer(service.getSettings().getQuitMessage());
+                superClassQuitServer(message);
             }
         }.start();
     }
 
+    private final void superClassQuitServer(String message)
+    {
+        super.quitServer(message);
+    }
+
     /**
      * Check whether the nickname has been mentioned.
      * 
@@ -1236,4 +1261,15 @@ public class IRCConnection extends PircBot
     {
         mNickMatch = Pattern.compile("(?:^|[\\s?!'�:;,.])"+Pattern.quote(getNick())+"(?:[\\s?!'�:;,.]|$)", Pattern.CASE_INSENSITIVE);
     }
+
+    @Override
+    public void dispose()
+    {
+        synchronized(isQuittingLock) {
+            if (isQuitting)
+                disposeRequested = true;
+            else
+                super.dispose();
+        }
+    }
 }