From 2d527dbef2026e5b4fee4cd7e5dd126405b95aee Mon Sep 17 00:00:00 2001 From: Danny van Heumen Date: Sun, 26 Jan 2014 22:27:23 +0100 Subject: [PATCH] Use Result instance for synchronization while getting list to handle spurious wakeups. --- .../impl/protocol/irc/IrcStack.java | 55 +++++++++++-------- .../impl/protocol/irc/Result.java | 17 ++++++ .../impl/protocol/irc/ResultTest.java | 11 ++++ 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/src/net/java/sip/communicator/impl/protocol/irc/IrcStack.java b/src/net/java/sip/communicator/impl/protocol/irc/IrcStack.java index 1653c1bb7..36cbe54b0 100644 --- a/src/net/java/sip/communicator/impl/protocol/irc/IrcStack.java +++ b/src/net/java/sip/communicator/impl/protocol/irc/IrcStack.java @@ -363,17 +363,21 @@ public List getServerChatRoomList() { if (this.channellist.instance == null) { - List list = new LinkedList(); - synchronized (list) + Result, Exception> listSignal = + new Result, Exception>( + new LinkedList()); + synchronized (listSignal) { try { this.irc.addListener(new ChannelListListener(this.irc, - list)); + listSignal)); this.irc.rawMessage("LIST"); - LOGGER.trace("Start waiting for list ..."); - // FIXME incorrect way of waiting (spurious wakeups) - list.wait(); + while (!listSignal.isDone()) + { + LOGGER.trace("Start waiting for list ..."); + listSignal.wait(); + } LOGGER.trace("Done waiting for list."); } catch (InterruptedException e) @@ -381,7 +385,7 @@ public List getServerChatRoomList() LOGGER.warn("INTERRUPTED while waiting for list.", e); } } - this.channellist.instance = list; + this.channellist.instance = listSignal.getValue(); } LOGGER.trace("Finished retrieve server chat room list."); return Collections.unmodifiableList(this.channellist.instance); @@ -1339,14 +1343,21 @@ private static class ChannelListListener /** * Reference to the provided list instance. */ - private List channels; + private Result, Exception> signal; - private ChannelListListener(IRCApi api, List list) + /** + * Constructor for channel list listener. + * @param api irc-api library instance + * @param list signal for sync signaling + */ + private ChannelListListener(IRCApi api, + Result, Exception> list) { if (api == null) - throw new IllegalArgumentException("IRC api cannot be null"); + throw new IllegalArgumentException( + "IRC api instance cannot be null"); this.api = api; - this.channels = list; + this.signal = list; } /** @@ -1360,35 +1371,35 @@ private ChannelListListener(IRCApi api, List list) @Override public void onServerNumericMessage(ServerNumericMessage msg) { - if (this.channels == null) + if (this.signal.isDone()) return; - + switch (msg.getNumericCode()) { case 321: - synchronized (this.channels) + synchronized (this.signal) { - this.channels.clear(); + this.signal.getValue().clear(); } break; case 322: String channel = parse(msg.getText()); if (channel != null) { - synchronized (this.channels) + synchronized (this.signal) { - this.channels.add(channel); + this.signal.getValue().add(channel); } } break; case 323: - synchronized (this.channels) + synchronized (this.signal) { - this.channels.notifyAll(); - // Done collecting channels. Delete list reference and - // remove listener and then we're done. - this.channels = null; + // Done collecting channels. Remove listener and then we're + // done. this.api.deleteListener(this); + this.signal.setDone(); + this.signal.notifyAll(); } break; // TODO Add support for REPLY 416: LIST :output too large, truncated diff --git a/src/net/java/sip/communicator/impl/protocol/irc/Result.java b/src/net/java/sip/communicator/impl/protocol/irc/Result.java index 6a0ccee46..956b2dc6a 100644 --- a/src/net/java/sip/communicator/impl/protocol/irc/Result.java +++ b/src/net/java/sip/communicator/impl/protocol/irc/Result.java @@ -30,6 +30,23 @@ public class Result */ private E exception = null; + /** + * Constructor for result without initial value + */ + public Result() + { + } + + /** + * Constructor for result with initial value + * + * @param initialValue initial value + */ + public Result(T initialValue) + { + this.value = initialValue; + } + /** * Check whether it is actually done. * diff --git a/test/net/java/sip/communicator/impl/protocol/irc/ResultTest.java b/test/net/java/sip/communicator/impl/protocol/irc/ResultTest.java index 64b6127b2..611fd9e50 100644 --- a/test/net/java/sip/communicator/impl/protocol/irc/ResultTest.java +++ b/test/net/java/sip/communicator/impl/protocol/irc/ResultTest.java @@ -13,6 +13,17 @@ public void testConstruction() Assert.assertNull(result.getValue()); Assert.assertNull(result.getException()); } + + public void testConstructionWithInitialValue() + { + Object initial = new Object(); + Result result = + new Result(initial); + Assert.assertNotNull(result); + Assert.assertFalse(result.isDone()); + Assert.assertSame(initial, result.getValue()); + Assert.assertNull(result.getException()); + } public void testSetDone() {