From cd05d5f5d83386463c9e90ed45fd102463a7c514 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sun, 25 Feb 2018 11:29:39 +0100 Subject: [PATCH 1/4] Call schedule() instead of directly invoking the scheduled executor in AbstractXMPPConnection. --- .../java/org/jivesoftware/smack/AbstractXMPPConnection.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 8c589db99..06af3d2ba 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -1546,7 +1546,7 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { } } }; - removeCallbacksService.schedule(new Runnable() { + schedule(new Runnable() { @Override public void run() { boolean removed = removeAsyncStanzaListener(packetListener); @@ -1601,7 +1601,7 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { } }; addSyncStanzaListener(packetListener, packetFilter); - removeCallbacksService.schedule(new Runnable() { + schedule(new Runnable() { @Override public void run() { removeSyncStanzaListener(packetListener); From 4f88f23f330c4acb917b4d0821ee4535ce374857 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 26 Feb 2018 10:23:02 +0100 Subject: [PATCH 2/4] Make scheduled executor service static --- .../smack/AbstractXMPPConnection.java | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 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 06af3d2ba..df05224ec 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -79,6 +79,7 @@ import org.jivesoftware.smack.parsing.ParsingExceptionCallback; import org.jivesoftware.smack.provider.ExtensionElementProvider; import org.jivesoftware.smack.provider.ProviderManager; import org.jivesoftware.smack.sasl.core.SASLAnonymous; +import org.jivesoftware.smack.util.Async; import org.jivesoftware.smack.util.DNSUtil; import org.jivesoftware.smack.util.Objects; import org.jivesoftware.smack.util.PacketParserUtils; @@ -238,8 +239,16 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { /** * This scheduled thread pool executor is used to remove pending callbacks. */ - private final ScheduledExecutorService removeCallbacksService = Executors.newSingleThreadScheduledExecutor( - new SmackExecutorThreadFactory(this, "Remove Callbacks")); + protected static final ScheduledExecutorService SCHEDULED_EXECUTOR_SERVICE = Executors.newSingleThreadScheduledExecutor( + new ThreadFactory() { + @Override + public Thread newThread(Runnable runnable) { + Thread thread = new Thread(runnable); + thread.setName("Smack Scheduled Executor Service"); + thread.setDaemon(true); + return thread; + } + }); /** * A cached thread pool executor service with custom thread factory to set meaningful names on the threads and set @@ -1407,7 +1416,6 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { // reference to their ExecutorService which prevents the ExecutorService from being // gc'ed. It is possible that the XMPPConnection instance is gc'ed while the // listenerExecutor ExecutorService call not be gc'ed until it got shut down. - removeCallbacksService.shutdownNow(); singleThreadedExecutorService.shutdownNow(); } catch (Throwable t) { LOGGER.log(Level.WARNING, "finalize() threw trhowable", t); @@ -1560,7 +1568,13 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { } else { exception = NoResponseException.newWith(AbstractXMPPConnection.this, replyFilter); } - exceptionCallback.processException(exception); + final Exception exceptionToProcess = exception; + Async.go(new Runnable() { + @Override + public void run() { + exceptionCallback.processException(exceptionToProcess); + } + }); } } }, timeout, TimeUnit.MILLISECONDS); @@ -1687,6 +1701,6 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { } protected final ScheduledFuture schedule(Runnable runnable, long delay, TimeUnit unit) { - return removeCallbacksService.schedule(runnable, delay, unit); + return SCHEDULED_EXECUTOR_SERVICE.schedule(runnable, delay, unit); } } From ecc53b1bc8eeec40641d0928f1050e530f08d0d2 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 26 Feb 2018 10:24:01 +0100 Subject: [PATCH 3/4] Add Manager.schedule(Runnable, long, TimeUnit) using Smack's scheduled executor service. --- .../src/main/java/org/jivesoftware/smack/Manager.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/Manager.java b/smack-core/src/main/java/org/jivesoftware/smack/Manager.java index 0b7924eb3..aee4bcbde 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/Manager.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/Manager.java @@ -1,6 +1,6 @@ /** * - * Copyright 2014 Florian Schmaus + * Copyright 2014-2018 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,8 @@ package org.jivesoftware.smack; import java.lang.ref.WeakReference; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; import org.jivesoftware.smack.SmackException.NotLoggedInException; import org.jivesoftware.smack.util.Objects; @@ -48,4 +50,8 @@ public abstract class Manager { } return connection; } + + protected static final ScheduledFuture schedule(Runnable runnable, long delay, TimeUnit unit) { + return AbstractXMPPConnection.SCHEDULED_EXECUTOR_SERVICE.schedule(runnable, delay, unit); + } } From 27808e9e8cc8f212e896c7f8c48e406b9f06755f Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 26 Feb 2018 10:24:35 +0100 Subject: [PATCH 4/4] Make PingManager use Manager.schedule() --- .../jivesoftware/smackx/ping/PingManager.java | 23 ++----------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/ping/PingManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/ping/PingManager.java index e8e66f8c5..90a548538 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/ping/PingManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/ping/PingManager.java @@ -1,6 +1,6 @@ /** * - * Copyright 2012-2017 Florian Schmaus + * Copyright 2012-2018 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,8 +20,6 @@ import java.util.Map; import java.util.Set; import java.util.WeakHashMap; import java.util.concurrent.CopyOnWriteArraySet; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.logging.Level; @@ -45,7 +43,6 @@ import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.IQ.Type; import org.jivesoftware.smack.packet.Stanza; import org.jivesoftware.smack.packet.XMPPError; -import org.jivesoftware.smack.util.SmackExecutorThreadFactory; import org.jivesoftware.smackx.disco.ServiceDiscoveryManager; import org.jivesoftware.smackx.ping.packet.Ping; @@ -112,8 +109,6 @@ public final class PingManager extends Manager { private final Set pingFailedListeners = new CopyOnWriteArraySet<>(); - private final ScheduledExecutorService executorService; - /** * The interval in seconds between pings are send to the users server. */ @@ -123,8 +118,6 @@ public final class PingManager extends Manager { private PingManager(XMPPConnection connection) { super(connection); - executorService = Executors.newSingleThreadScheduledExecutor( - new SmackExecutorThreadFactory(connection, "Ping")); ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(connection); sdm.addFeature(Ping.NAMESPACE); @@ -391,7 +384,7 @@ public final class PingManager extends Manager { int nextPingIn = pingInterval - delta; LOGGER.fine("Scheduling ServerPingTask in " + nextPingIn + " seconds (pingInterval=" + pingInterval + ", delta=" + delta + ")"); - nextAutomaticPing = executorService.schedule(pingServerRunnable, nextPingIn, TimeUnit.SECONDS); + nextAutomaticPing = schedule(pingServerRunnable, nextPingIn, TimeUnit.SECONDS); } } @@ -479,16 +472,4 @@ public final class PingManager extends Manager { } }; - @Override - protected void finalize() throws Throwable { - LOGGER.fine("finalizing PingManager: Shutting down executor service"); - try { - executorService.shutdown(); - } catch (Throwable t) { - LOGGER.log(Level.WARNING, "finalize() threw throwable", t); - } - finally { - super.finalize(); - } - } }