1
0
mirror of https://github.com/moparisthebest/Yaaic synced 2024-11-22 17:02:21 -05:00

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.
This commit is contained in:
Steven Luo 2011-06-05 02:29:04 -07:00 committed by Sebastian Kaspari
parent a9621b66b4
commit aaf3f3e332
2 changed files with 39 additions and 2 deletions

View File

@ -357,7 +357,8 @@ public abstract class PircBot implements ReplyConstants {
* *
* @param reason The reason for quitting the server. * @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); this.sendRawLine("QUIT :" + reason);
} }

View File

@ -53,6 +53,10 @@ public class IRCConnection extends PircBot
private ArrayList<String> autojoinChannels; private ArrayList<String> autojoinChannels;
private Pattern mNickMatch; private Pattern mNickMatch;
private boolean isQuitting = false;
private boolean disposeRequested = false;
private Object isQuittingLock = new Object();
/** /**
* Create a new connection * Create a new connection
* *
@ -1136,6 +1140,12 @@ public class IRCConnection extends PircBot
); );
service.sendBroadcast(cIntent); service.sendBroadcast(cIntent);
} }
synchronized(isQuittingLock) {
isQuitting = false;
if (disposeRequested)
super.dispose();
}
} }
/** /**
@ -1210,14 +1220,29 @@ public class IRCConnection extends PircBot
@Override @Override
public void quitServer() public void quitServer()
{ {
quitServer(service.getSettings().getQuitMessage());
}
@Override
public void quitServer(final String message)
{
synchronized(isQuittingLock) {
isQuitting = true;
}
new Thread() { new Thread() {
@Override @Override
public void run() { public void run() {
quitServer(service.getSettings().getQuitMessage()); superClassQuitServer(message);
} }
}.start(); }.start();
} }
private final void superClassQuitServer(String message)
{
super.quitServer(message);
}
/** /**
* Check whether the nickname has been mentioned. * Check whether the nickname has been mentioned.
* *
@ -1236,4 +1261,15 @@ public class IRCConnection extends PircBot
{ {
mNickMatch = Pattern.compile("(?:^|[\\s?!'<27>:;,.])"+Pattern.quote(getNick())+"(?:[\\s?!'<27>:;,.]|$)", Pattern.CASE_INSENSITIVE); mNickMatch = Pattern.compile("(?:^|[\\s?!'<27>:;,.])"+Pattern.quote(getNick())+"(?:[\\s?!'<27>:;,.]|$)", Pattern.CASE_INSENSITIVE);
} }
@Override
public void dispose()
{
synchronized(isQuittingLock) {
if (isQuitting)
disposeRequested = true;
else
super.dispose();
}
}
} }