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 221a986b4..41890d14f 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -1251,7 +1251,7 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { try { stanza = PacketParserUtils.parseStanza(parser, incomingStreamXmlEnvironment); } - catch (Exception e) { + catch (XmlPullParserException | SmackParsingException | IOException e) { CharSequence content = PacketParserUtils.parseContentDepth(parser, parserDepth); UnparseableStanza message = new UnparseableStanza(content, e); diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java b/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java index e80c6efcc..520e4ee87 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java @@ -55,6 +55,7 @@ import org.jivesoftware.smack.xml.XmlPullParser; import org.jivesoftware.smack.xml.XmlPullParserException; import org.jxmpp.jid.Jid; +import org.jxmpp.stringprep.XmppStringprepException; /** * Utility class that helps to parse packets. Any parsing packets method that must be shared @@ -89,7 +90,7 @@ public class PacketParserUtils { } @SuppressWarnings("unchecked") - public static S parseStanza(String stanza) throws Exception { + public static S parseStanza(String stanza) throws XmlPullParserException, SmackParsingException, IOException { return (S) parseStanza(getParserFor(stanza), null); } @@ -101,9 +102,11 @@ public class PacketParserUtils { * @param parser * @param outerXmlEnvironment the outer XML environment (optional). * @return a stanza which is either a Message, IQ or Presence. - * @throws Exception + * @throws XmlPullParserException + * @throws SmackParsingException + * @throws IOException */ - public static Stanza parseStanza(XmlPullParser parser, XmlEnvironment outerXmlEnvironment) throws Exception { + public static Stanza parseStanza(XmlPullParser parser, XmlEnvironment outerXmlEnvironment) throws XmlPullParserException, SmackParsingException, IOException { ParserUtils.assertAtStartTag(parser); final String name = parser.getName(); switch (name) { @@ -508,9 +511,12 @@ public class PacketParserUtils { * @param parser the XML parser, positioned at the start of an IQ packet. * @param outerXmlEnvironment the outer XML environment (optional). * @return an IQ object. - * @throws Exception + * @throws XmlPullParserException + * @throws XmppStringprepException + * @throws IOException + * @throws SmackParsingException */ - public static IQ parseIQ(XmlPullParser parser, XmlEnvironment outerXmlEnvironment) throws Exception { + public static IQ parseIQ(XmlPullParser parser, XmlEnvironment outerXmlEnvironment) throws XmlPullParserException, XmppStringprepException, IOException, SmackParsingException { ParserUtils.assertAtStartTag(parser); final int initialDepth = parser.getDepth(); XmlEnvironment iqXmlEnvironment = XmlEnvironment.from(parser, outerXmlEnvironment); diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/commands/AdHocCommandManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/commands/AdHocCommandManager.java index 8e60d6594..dd05857ae 100755 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/commands/AdHocCommandManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/commands/AdHocCommandManager.java @@ -20,10 +20,13 @@ package org.jivesoftware.smackx.commands; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Collection; +import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -126,15 +129,6 @@ public final class AdHocCommandManager extends Manager { private final ServiceDiscoveryManager serviceDiscoveryManager; - /** - * Thread that reaps stale sessions. - */ - // FIXME The session sweeping is horrible implemented. The thread will never stop running. A different approach must - // be implemented. For example one that does stop reaping sessions and the thread if there are no more, and restarts - // the reaping process on demand. Or for every command a scheduled task should be created that removes the session - // if it's timed out. See SMACK-624. - private Thread sessionsSweeper; - private AdHocCommandManager(XMPPConnection connection) { super(connection); this.serviceDiscoveryManager = ServiceDiscoveryManager.getInstanceFor(connection); @@ -186,8 +180,6 @@ public final class AdHocCommandManager extends Manager { } } }); - - sessionsSweeper = null; } /** @@ -380,49 +372,8 @@ public final class AdHocCommandManager extends Manager { // available for the next call response.setStatus(Status.executing); executingCommands.put(sessionId, command); - // See if the session reaping thread is started. If not, start it. - if (sessionsSweeper == null) { - sessionsSweeper = new Thread(new Runnable() { - @Override - public void run() { - while (true) { - for (String sessionId : executingCommands.keySet()) { - LocalCommand command = executingCommands.get(sessionId); - // Since the command could be removed in the meanwhile - // of getting the key and getting the value - by a - // processed packet. We must check if it still in the - // map. - if (command != null) { - long creationStamp = command.getCreationDate(); - // Check if the Session data has expired (default is - // 10 minutes) - // To remove it from the session list it waits for - // the double of the of time out time. This is to - // let - // the requester know why his execution request is - // not accepted. If the session is removed just - // after the time out, then whe the user request to - // continue the execution he will received an - // invalid session error and not a time out error. - if (System.currentTimeMillis() - creationStamp > SESSION_TIMEOUT * 1000 * 2) { - // Remove the expired session - executingCommands.remove(sessionId); - } - } - } - try { - Thread.sleep(1000); - } - catch (InterruptedException ie) { - // Ignore. - } - } - } - - }); - sessionsSweeper.setDaemon(true); - sessionsSweeper.start(); - } + // See if the session sweeper thread is scheduled. If not, start it. + maybeWindUpSessionSweeper(); } // Sends the response packet @@ -558,6 +509,47 @@ public final class AdHocCommandManager extends Manager { } } + private boolean sessionSweeperScheduled; + + private final Runnable sessionSweeper = () -> { + final long currentTime = System.currentTimeMillis(); + synchronized (this) { + for (Iterator> it = executingCommands.entrySet().iterator(); it.hasNext();) { + Entry entry = it.next(); + LocalCommand command = entry.getValue(); + + long creationStamp = command.getCreationDate(); + // Check if the Session data has expired (default is 10 minutes) + // To remove it from the session list it waits for the double of + // the of time out time. This is to let + // the requester know why his execution request is + // not accepted. If the session is removed just + // after the time out, then once the user requests to + // continue the execution he will received an + // invalid session error and not a time out error. + if (currentTime - creationStamp > SESSION_TIMEOUT * 1000 * 2) { + // Remove the expired session + it.remove(); + } + } + + sessionSweeperScheduled = false; + } + + if (!executingCommands.isEmpty()) { + maybeWindUpSessionSweeper(); + } + }; + + private synchronized void maybeWindUpSessionSweeper() { + if (sessionSweeperScheduled) { + return; + } + + sessionSweeperScheduled = true; + schedule(sessionSweeper, 10, TimeUnit.SECONDS); + } + /** * Responds an error with an specific condition. *