Merge branch 'master' of github.com:Flowdalic/Smack

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);

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 extends Stanza> S parseStanza(String stanza) throws Exception {
+    public static <S extends Stanza> 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);

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.
+                    // See if the session sweeper thread is scheduled. If not, start it.
-                    if (sessionsSweeper == null) {
+                    maybeWindUpSessionSweeper();
-                        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();
-                    }
                 }
 
                 // 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<Entry<String, LocalCommand>> it = executingCommands.entrySet().iterator(); it.hasNext();) {
+                Entry<String, LocalCommand> 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.
      *