From 98a3c46e9ae66299859357c4e20f493ba5d0f667 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 13 Oct 2014 10:45:00 +0200 Subject: [PATCH] Add XMPPConnection.createPacketcollectorAndSend(PacketFilter, Packet) Using createPacketCollector(filter); sendPacket(packet); was error prone, i.e. the PacketCollector could leak if sendPacket() would throw an exception and the user forgot to call PacketCollector.cancel(). For cases where createPacketCollectorAndSend(IQ) is not sufficient (because we don't send IQs), createPacketCollectorAndSend(PacketFilter, Packet) is now used, which does take care that the PacketCollector does not leak if sendPacket() throws an Exception. --- .../smack/AbstractXMPPConnection.java | 26 ++++++++----------- .../jivesoftware/smack/XMPPConnection.java | 20 ++++++++++++++ .../filetransfer/FaultTolerantNegotiator.java | 7 +++-- .../smackx/filetransfer/StreamNegotiator.java | 3 +-- .../smackx/iqregister/AccountManager.java | 3 +-- .../smackx/muc/MultiUserChat.java | 12 +++------ .../smackx/workgroup/agent/AgentSession.java | 17 +++++------- .../smackx/workgroup/user/Workgroup.java | 6 ++--- 8 files changed, 47 insertions(+), 47 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java index 4ddac7757..793b5b852 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -422,26 +422,14 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { // Note that we can not use IQReplyFilter here, since the users full JID is not yet // available. It will become available right after the resource has been successfully bound. Bind bindResource = Bind.newSet(resource); - PacketCollector packetCollector = createPacketCollector(new PacketIDFilter(bindResource)); - try { - sendPacket(bindResource); - } catch (NotConnectedException e) { - packetCollector.cancel(); - throw e; - } + PacketCollector packetCollector = createPacketCollectorAndSend(new PacketIDFilter(bindResource), bindResource); Bind response = packetCollector.nextResultOrThrow(); user = response.getJid(); setServiceName(XmppStringUtils.parseDomain(user)); if (hasFeature(Session.ELEMENT, Session.NAMESPACE) && !getConfiguration().isLegacySessionDisabled()) { Session session = new Session(); - packetCollector = createPacketCollector(new PacketIDFilter(session)); - try { - sendPacket(session); - } catch (NotConnectedException e) { - packetCollector.cancel(); - throw e; - } + packetCollector = createPacketCollectorAndSend(new PacketIDFilter(session), session); packetCollector.nextResultOrThrow(); } } @@ -639,12 +627,20 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { public PacketCollector createPacketCollectorAndSend(IQ packet) throws NotConnectedException { PacketFilter packetFilter = new IQReplyFilter(packet, this); // Create the packet collector before sending the packet + PacketCollector packetCollector = createPacketCollectorAndSend(packetFilter, packet); + return packetCollector; + } + + @Override + public PacketCollector createPacketCollectorAndSend(PacketFilter packetFilter, Packet packet) + throws NotConnectedException { + // Create the packet collector before sending the packet PacketCollector packetCollector = createPacketCollector(packetFilter); try { // Now we can send the packet as the collector has been created sendPacket(packet); } - catch (NotConnectedException e) { + catch (NotConnectedException | RuntimeException e) { packetCollector.cancel(); throw e; } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java index 66f9f1813..093f36677 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java @@ -208,6 +208,26 @@ public interface XMPPConnection { * a specific result. * * @param packetFilter the packet filter to use. + * @param packet the packet to send right after the collector got created + * @return a new packet collector. + */ + public PacketCollector createPacketCollectorAndSend(PacketFilter packetFilter, Packet packet) + throws NotConnectedException; + + /** + * Creates a new packet collector for this connection. A packet filter + * determines which packets will be accumulated by the collector. A + * PacketCollector is more suitable to use than a {@link PacketListener} + * when you need to wait for a specific result. + *

+ * Note: If you send a Packet right after using this method, then + * consider using + * {@link #createPacketCollectorAndSend(PacketFilter, Packet)} instead. + * Otherwise make sure cancel the PacketCollector in every case, e.g. even + * if an exception is thrown, or otherwise you may leak the PacketCollector. + *

+ * + * @param packetFilter the packet filter to use. * @return a new packet collector. */ public PacketCollector createPacketCollector(PacketFilter packetFilter); diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FaultTolerantNegotiator.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FaultTolerantNegotiator.java index 6feb8e04d..3a9792ca5 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FaultTolerantNegotiator.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FaultTolerantNegotiator.java @@ -80,10 +80,9 @@ public class FaultTolerantNegotiator extends StreamNegotiator { } public InputStream createIncomingStream(StreamInitiation initiation) throws SmackException { - PacketCollector collector = connection.createPacketCollector( - getInitiationPacketFilter(initiation.getFrom(), initiation.getSessionID())); - - connection.sendPacket(super.createInitiationAccept(initiation, getNamespaces())); + PacketCollector collector = connection.createPacketCollectorAndSend( + getInitiationPacketFilter(initiation.getFrom(), initiation.getSessionID()), + super.createInitiationAccept(initiation, getNamespaces())); ExecutorService threadPoolExecutor = Executors.newFixedThreadPool(2); CompletionService service diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/StreamNegotiator.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/StreamNegotiator.java index f4f3db1b3..92dc82878 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/StreamNegotiator.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/StreamNegotiator.java @@ -79,8 +79,7 @@ public abstract class StreamNegotiator { // establish collector to await response PacketCollector collector = connection - .createPacketCollector(getInitiationPacketFilter(initiation.getFrom(), initiation.getSessionID())); - connection.sendPacket(response); + .createPacketCollectorAndSend(getInitiationPacketFilter(initiation.getFrom(), initiation.getSessionID()), response); Packet streamMethodInitiation = collector.nextResultOrThrow(); diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java index 212a49065..1111731c1 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java @@ -290,8 +290,7 @@ public class AccountManager extends Manager { } private PacketCollector createPacketCollectorAndSend(IQ req) throws NotConnectedException { - PacketCollector collector = connection().createPacketCollector(new PacketIDFilter(req.getPacketID())); - connection().sendPacket(req); + PacketCollector collector = connection().createPacketCollectorAndSend(new PacketIDFilter(req.getPacketID()), req); return collector; } } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java index f0a6ed0e7..ac707e635 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java @@ -469,9 +469,7 @@ public class MultiUserChat { + nickname), new PacketTypeFilter(Presence.class)); PacketCollector response = null; - response = connection.createPacketCollector(responseFilter); - // Send join packet. - connection.sendPacket(joinPresence); + response = connection.createPacketCollectorAndSend(responseFilter, joinPresence); // Wait up to a certain number of seconds for a reply. Presence presence = (Presence) response.nextResultOrThrow(timeout); @@ -1080,9 +1078,7 @@ public class MultiUserChat { new AndFilter( FromMatchesFilter.createFull(room + "/" + nickname), new PacketTypeFilter(Presence.class)); - PacketCollector response = connection.createPacketCollector(responseFilter); - // Send join packet. - connection.sendPacket(joinPresence); + PacketCollector response = connection.createPacketCollectorAndSend(responseFilter, joinPresence); // Wait up to a certain number of seconds for a reply. If there is a negative reply, an // exception will be thrown response.nextResultOrThrow(); @@ -1905,9 +1901,7 @@ public class MultiUserChat { return subject.equals(msg.getSubject()); } }); - PacketCollector response = connection.createPacketCollector(responseFilter); - // Send change subject packet. - connection.sendPacket(message); + PacketCollector response = connection.createPacketCollectorAndSend(responseFilter, message); // Wait up to a certain number of seconds for a reply. response.nextResultOrThrow(); } diff --git a/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/agent/AgentSession.java b/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/agent/AgentSession.java index f0a92624e..9fbcc9621 100644 --- a/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/agent/AgentSession.java +++ b/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/agent/AgentSession.java @@ -301,9 +301,8 @@ public class AgentSession { presence.addExtension(new DefaultPacketExtension(AgentStatus.ELEMENT_NAME, AgentStatus.NAMESPACE)); - PacketCollector collector = this.connection.createPacketCollector(new AndFilter(new PacketTypeFilter(Presence.class), FromMatchesFilter.create(workgroupJID))); - - connection.sendPacket(presence); + PacketCollector collector = this.connection.createPacketCollectorAndSend(new AndFilter( + new PacketTypeFilter(Presence.class), FromMatchesFilter.create(workgroupJID)), presence); presence = (Presence)collector.nextResultOrThrow(); @@ -401,11 +400,9 @@ public class AgentSession { presence.addExtension(agentStatus); presence.addExtension(new MetaData(this.metaData)); - PacketCollector collector = this.connection.createPacketCollector(new AndFilter( + PacketCollector collector = this.connection.createPacketCollectorAndSend(new AndFilter( new PacketTypeFilter(Presence.class), - FromMatchesFilter.create(workgroupJID))); - - this.connection.sendPacket(presence); + FromMatchesFilter.create(workgroupJID)), presence); collector.nextResultOrThrow(); } @@ -447,10 +444,8 @@ public class AgentSession { } presence.addExtension(new MetaData(this.metaData)); - PacketCollector collector = this.connection.createPacketCollector(new AndFilter(new PacketTypeFilter(Presence.class), - FromMatchesFilter.create(workgroupJID))); - - this.connection.sendPacket(presence); + PacketCollector collector = this.connection.createPacketCollectorAndSend(new AndFilter(new PacketTypeFilter(Presence.class), + FromMatchesFilter.create(workgroupJID)), presence); collector.nextResultOrThrow(); } diff --git a/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/user/Workgroup.java b/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/user/Workgroup.java index 3aeacb296..831fc7ac0 100644 --- a/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/user/Workgroup.java +++ b/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/user/Workgroup.java @@ -180,10 +180,8 @@ public class Workgroup { directedPresence.setTo(workgroupJID); PacketFilter typeFilter = new PacketTypeFilter(Presence.class); PacketFilter fromFilter = FromMatchesFilter.create(workgroupJID); - PacketCollector collector = connection.createPacketCollector(new AndFilter(fromFilter, - typeFilter)); - - connection.sendPacket(directedPresence); + PacketCollector collector = connection.createPacketCollectorAndSend(new AndFilter(fromFilter, + typeFilter), directedPresence); Presence response = (Presence)collector.nextResultOrThrow(); return Presence.Type.available == response.getType();