runelite-client: add ClientShutdown event

This should hopefully make the client not corrupt it's cache randomly,
and prevents the config sets from racing shutdown

http-api/src/main/java/net/runelite/http/api/config/ConfigClient.java

@@ -29,6 +29,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
 import net.runelite.http.api.RuneLiteAPI;
 import okhttp3.Call;
 import okhttp3.Callback;
@@ -77,8 +78,10 @@ public class ConfigClient
 		}
 	}
 
-	public void set(String key, String value)
+	public CompletableFuture<Void> set(String key, String value)
 	{
+		CompletableFuture<Void> future = new CompletableFuture<>();
+
 		HttpUrl url = RuneLiteAPI.getApiBase().newBuilder()
 			.addPathSegment("config")
 			.addPathSegment(key)
@@ -98,6 +101,7 @@ public class ConfigClient
 			public void onFailure(Call call, IOException e)
 			{
 				logger.warn("Unable to synchronize configuration item", e);
+				future.completeExceptionally(e);
 			}
 
 			@Override
@@ -105,12 +109,17 @@ public class ConfigClient
 			{
 				response.close();
 				logger.debug("Synchronized configuration value '{}' to '{}'", key, value);
+				future.complete(null);
 			}
 		});
+
+		return future;
 	}
 
-	public void unset(String key)
+	public CompletableFuture<Void> unset(String key)
 	{
+		CompletableFuture<Void> future = new CompletableFuture<>();
+
 		HttpUrl url = RuneLiteAPI.getApiBase().newBuilder()
 			.addPathSegment("config")
 			.addPathSegment(key)
@@ -130,6 +139,7 @@ public class ConfigClient
 			public void onFailure(Call call, IOException e)
 			{
 				logger.warn("Unable to unset configuration item", e);
+				future.completeExceptionally(e);
 			}
 
 			@Override
@@ -137,7 +147,10 @@ public class ConfigClient
 			{
 				response.close();
 				logger.debug("Unset configuration value '{}'", key);
+				future.complete(null);
 			}
 		});
+
+		return future;
 	}
 }

runelite-client/src/main/java/net/runelite/client/ClientSessionManager.java

@@ -35,6 +35,8 @@ import javax.inject.Singleton;
 import lombok.extern.slf4j.Slf4j;
 import net.runelite.api.Client;
 import net.runelite.api.GameState;
+import net.runelite.client.eventbus.Subscribe;
+import net.runelite.client.events.ClientShutdown;
 import net.runelite.client.util.RunnableExceptionLogger;
 
 @Singleton
@@ -70,9 +72,12 @@ public class ClientSessionManager
 		scheduledFuture = executorService.scheduleWithFixedDelay(RunnableExceptionLogger.wrap(this::ping), 1, 10, TimeUnit.MINUTES);
 	}
 
-	public void shutdown()
+	@Subscribe
+	private void onClientShutdown(ClientShutdown e)
 	{
-		if (sessionId != null)
+		scheduledFuture.cancel(true);
+
+		e.waitFor(executorService.submit(() ->
 		{
 			try
 			{
@@ -83,9 +88,7 @@ public class ClientSessionManager
 				log.warn(null, ex);
 			}
 			sessionId = null;
-		}
+		}));
-		
-		scheduledFuture.cancel(true);
 	}
 
 	private void ping()

runelite-client/src/main/java/net/runelite/client/RuneLite.java

@@ -324,11 +324,12 @@ public class RuneLite
 
 		// Start client session
 		clientSessionManager.start();
+		eventBus.register(clientSessionManager);
 
 		SplashScreen.stage(.75, null, "Starting core interface");
 
 		// Initialize UI
-		clientUI.init(this);
+		clientUI.init();
 
 		// Initialize Discord service
 		discordService.init();
@@ -341,6 +342,8 @@ public class RuneLite
 		eventBus.register(drawManager);
 		eventBus.register(infoBoxManager);
 		eventBus.register(tooltipManager);
+		eventBus.register(configManager);
+		eventBus.register(discordService);
 
 		if (!isOutdated)
 		{
@@ -373,13 +376,6 @@ public class RuneLite
 		clientUI.show();
 	}
 
-	public void shutdown()
-	{
-		configManager.sendConfig();
-		clientSessionManager.shutdown();
-		discordService.close();
-	}
-
 	@VisibleForTesting
 	public static void setInjector(Injector injector)
 	{

runelite-client/src/main/java/net/runelite/client/config/ConfigManager.java

@@ -57,9 +57,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
+import javax.annotation.Nullable;
 import javax.inject.Inject;
 import javax.inject.Named;
 import javax.inject.Singleton;
@@ -68,6 +71,8 @@ import net.runelite.api.coords.WorldPoint;
 import net.runelite.client.RuneLite;
 import net.runelite.client.account.AccountSession;
 import net.runelite.client.eventbus.EventBus;
+import net.runelite.client.eventbus.Subscribe;
+import net.runelite.client.events.ClientShutdown;
 import net.runelite.client.events.ConfigChanged;
 import net.runelite.client.util.ColorUtil;
 import net.runelite.http.api.config.ConfigClient;
@@ -675,27 +680,39 @@ public class ConfigManager
 		return object.toString();
 	}
 
-	public void sendConfig()
+	@Subscribe(priority = 100)
+	private void onClientShutdown(ClientShutdown e)
 	{
+		Future<Void> f = sendConfig();
+		if (f != null)
+		{
+			e.waitFor(f);
+		}
+	}
+
+	@Nullable
+	private CompletableFuture<Void> sendConfig()
+	{
+		CompletableFuture<Void> future = null;
 		boolean changed;
 		synchronized (pendingChanges)
 		{
 			if (client != null)
 			{
-				for (Map.Entry<String, String> entry : pendingChanges.entrySet())
+				future = CompletableFuture.allOf(pendingChanges.entrySet().stream().map(entry ->
 				{
 					String key = entry.getKey();
 					String value = entry.getValue();
 
 					if (Strings.isNullOrEmpty(value))
 					{
-						client.unset(key);
+						return client.unset(key);
 					}
 					else
 					{
-						client.set(key, value);
+						return client.set(key, value);
 					}
-				}
+				}).toArray(CompletableFuture[]::new));
 			}
 			changed = !pendingChanges.isEmpty();
 			pendingChanges.clear();
@@ -712,5 +729,7 @@ public class ConfigManager
 				log.warn("unable to save configuration file", ex);
 			}
 		}
+
+		return future;
 	}
 }

runelite-client/src/main/java/net/runelite/client/events/ClientShutdown.java

@@ -0,0 +1,72 @@
+/*
+ * Copyright (c) 2020 Abex
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice, this
+ *    list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package net.runelite.client.events;
+
+import java.time.Duration;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import lombok.Value;
+import lombok.extern.slf4j.Slf4j;
+
+@Value
+@Slf4j
+public class ClientShutdown
+{
+	private Queue<Future<?>> tasks = new ConcurrentLinkedQueue<>();
+
+	public void waitFor(Future<?> future)
+	{
+		tasks.add(future);
+	}
+
+	public void waitForAllConsumers(Duration totalTimeout)
+	{
+		long deadline = System.nanoTime() + totalTimeout.toNanos();
+		for (Future<?> task; (task = tasks.poll()) != null; )
+		{
+			long timeout = deadline - System.nanoTime();
+			if (timeout < 0)
+			{
+				log.warn("Timed out waiting for task completion");
+				return;
+			}
+
+			try
+			{
+				task.get(timeout, TimeUnit.NANOSECONDS);
+			}
+			catch (ThreadDeath d)
+			{
+				throw d;
+			}
+			catch (Throwable t)
+			{
+				log.warn("Error during shutdown: ", t);
+			}
+		}
+	}
+}

runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java

@@ -42,7 +42,10 @@ import java.awt.TrayIcon;
 import java.awt.event.InputEvent;
 import java.awt.event.KeyEvent;
 import java.awt.event.MouseEvent;
+import java.awt.event.WindowAdapter;
+import java.awt.event.WindowEvent;
 import java.awt.image.BufferedImage;
+import java.time.Duration;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 import javax.inject.Provider;
@@ -64,11 +67,12 @@ import net.runelite.api.Constants;
 import net.runelite.api.GameState;
 import net.runelite.api.Player;
 import net.runelite.api.Point;
+import net.runelite.client.eventbus.EventBus;
+import net.runelite.client.events.ClientShutdown;
 import net.runelite.client.events.ConfigChanged;
 import net.runelite.api.events.GameStateChanged;
 import net.runelite.api.widgets.Widget;
 import net.runelite.api.widgets.WidgetInfo;
-import net.runelite.client.RuneLite;
 import net.runelite.client.RuneLiteProperties;
 import net.runelite.client.callback.ClientThread;
 import net.runelite.client.config.ConfigManager;
@@ -116,6 +120,8 @@ public class ClientUI
 	private final Applet client;
 	private final ConfigManager configManager;
 	private final Provider<ClientThread> clientThreadProvider;
+	private final EventBus eventBus;
+
 	private final CardLayout cardLayout = new CardLayout();
 	private final Rectangle sidebarButtonPosition = new Rectangle();
 	private boolean withTitleBar;
@@ -141,7 +147,8 @@ public class ClientUI
 		MouseManager mouseManager,
 		@Nullable Applet client,
 		ConfigManager configManager,
-		Provider<ClientThread> clientThreadProvider)
+		Provider<ClientThread> clientThreadProvider,
+		EventBus eventBus)
 	{
 		this.config = config;
 		this.keyManager = keyManager;
@@ -149,6 +156,7 @@ public class ClientUI
 		this.client = client;
 		this.configManager = configManager;
 		this.clientThreadProvider = clientThreadProvider;
+		this.eventBus = eventBus;
 	}
 
 	@Subscribe
@@ -289,10 +297,8 @@ public class ClientUI
 
 	/**
 	 * Initialize UI.
-	 * @param runelite runelite instance that will be shut down on exit
-	 * @throws Exception exception that can occur during creation of the UI
 	 */
-	public void init(final RuneLite runelite) throws Exception
+	public void init() throws Exception
 	{
 		SwingUtilities.invokeAndWait(() ->
 		{
@@ -317,14 +323,36 @@ public class ClientUI
 			frame.setLocationRelativeTo(frame.getOwner());
 			frame.setResizable(true);
 
-			SwingUtil.addGracefulExitCallback(frame,
+			frame.setDefaultCloseOperation(JFrame.DO_NOTHING_ON_CLOSE);
-				() ->
+			frame.addWindowListener(new WindowAdapter()
+			{
+				@Override
+				public void windowClosing(WindowEvent event)
 				{
-					saveClientBoundsConfig();
+					int result = JOptionPane.OK_OPTION;
-					runelite.shutdown();
+
-				},
+					if (showWarningOnExit())
-				this::showWarningOnExit
+					{
-			);
+						try
+						{
+							result = JOptionPane.showConfirmDialog(
+								frame,
+								"Are you sure you want to exit?", "Exit",
+								JOptionPane.OK_CANCEL_OPTION,
+								JOptionPane.QUESTION_MESSAGE);
+						}
+						catch (Exception e)
+						{
+							log.warn("Unexpected exception occurred while check for confirm required", e);
+						}
+					}
+
+					if (result == JOptionPane.OK_OPTION)
+					{
+						shutdownClient();
+					}
+				}
+			});
 
 			container = new JPanel();
 			container.setLayout(new BoxLayout(container, BoxLayout.X_AXIS));
@@ -541,6 +569,45 @@ public class ClientUI
 		return false;
 	}
 
+	private void shutdownClient()
+	{
+		saveClientBoundsConfig();
+		ClientShutdown csev = new ClientShutdown();
+		eventBus.post(csev);
+		new Thread(() ->
+		{
+			csev.waitForAllConsumers(Duration.ofSeconds(10));
+
+			if (client != null)
+			{
+				// The client can call System.exit when it's done shutting down
+				// if it doesn't though, we want to exit anyway, so race it
+				int clientShutdownWaitMS;
+				if (client instanceof Client)
+				{
+					((Client) client).stopNow();
+					clientShutdownWaitMS = 1000;
+				}
+				else
+				{
+					// it will continue rendering for about 4 seconds before attempting shutdown if its vanilla
+					client.stop();
+					frame.setVisible(false);
+					clientShutdownWaitMS = 6000;
+				}
+
+				try
+				{
+					Thread.sleep(clientShutdownWaitMS);
+				}
+				catch (InterruptedException ignored)
+				{
+				}
+			}
+			System.exit(0);
+		}, "RuneLite Shutdown").start();
+	}
+
 	/**
 	 * Paint this component to target graphics
 	 *
@@ -597,7 +664,7 @@ public class ClientUI
 	}
 
 	/**
-	 * Changes cursor for client window. Requires ${@link ClientUI#init(RuneLite)} to be called first.
+	 * Changes cursor for client window. Requires ${@link ClientUI#init()} to be called first.
 	 * FIXME: This is working properly only on Windows, Linux and Mac are displaying cursor incorrectly
 	 * @param image cursor image
 	 * @param name  cursor name

runelite-client/src/main/java/net/runelite/client/util/SwingUtil.java

@@ -39,27 +39,21 @@ import java.awt.Toolkit;
 import java.awt.TrayIcon;
 import java.awt.event.MouseAdapter;
 import java.awt.event.MouseEvent;
-import java.awt.event.WindowAdapter;
-import java.awt.event.WindowEvent;
 import java.awt.image.BufferedImage;
 import java.util.Enumeration;
-import java.util.concurrent.Callable;
 import java.util.function.BiConsumer;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.swing.AbstractButton;
 import javax.swing.ImageIcon;
 import javax.swing.JButton;
-import javax.swing.JFrame;
 import javax.swing.JMenuItem;
-import javax.swing.JOptionPane;
 import javax.swing.JPopupMenu;
 import javax.swing.LookAndFeel;
 import javax.swing.SwingUtilities;
 import javax.swing.ToolTipManager;
 import javax.swing.UIManager;
 import javax.swing.UnsupportedLookAndFeelException;
-import static javax.swing.WindowConstants.DO_NOTHING_ON_CLOSE;
 import javax.swing.plaf.FontUIResource;
 import lombok.extern.slf4j.Slf4j;
 import net.runelite.client.ui.ColorScheme;
@@ -189,48 +183,6 @@ public class SwingUtil
 		return trayIcon;
 	}
 
-	/**
-	 * Add graceful exit callback.
-	 *
-	 * @param frame           the frame
-	 * @param callback        the callback
-	 * @param confirmRequired the confirm required
-	 */
-	public static void addGracefulExitCallback(@Nonnull final JFrame frame, @Nonnull final Runnable callback, @Nonnull final Callable<Boolean> confirmRequired)
-	{
-		frame.setDefaultCloseOperation(DO_NOTHING_ON_CLOSE);
-		frame.addWindowListener(new WindowAdapter()
-		{
-			@Override
-			public void windowClosing(WindowEvent event)
-			{
-				int result = JOptionPane.OK_OPTION;
-
-				try
-				{
-					if (confirmRequired.call())
-					{
-						result = JOptionPane.showConfirmDialog(
-							frame,
-							"Are you sure you want to exit?", "Exit",
-							JOptionPane.OK_CANCEL_OPTION,
-							JOptionPane.QUESTION_MESSAGE);
-					}
-				}
-				catch (Exception e)
-				{
-					log.warn("Unexpected exception occurred while check for confirm required", e);
-				}
-
-				if (result == JOptionPane.OK_OPTION)
-				{
-					callback.run();
-					System.exit(0);
-				}
-			}
-		});
-	}
-
 	/**
 	 * Create swing button from navigation button.
 	 *