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.
---
This seems to be a longstanding bug that had no consequences until the
notifications patchset. The patch applies on top of that set.
application/src/org/jibble/pircbot/PircBot.java | 3 +-
application/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 dc031c2..1ffc164 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
*
@@ -1124,6 +1128,12 @@ public class IRCConnection extends PircBot
);
service.sendBroadcast(cIntent);
}
+
+ synchronized(isQuittingLock) {
+ isQuitting = false;
+ if (disposeRequested)
+ super.dispose();
+ }
}
/**
@@ -1198,14 +1208,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.
*
@@ -1224,4 +1249,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();
+ }
+ }
}
--
1.7.2.5
