When using /stop server executes the shutdown sequence twice (and hangs if players are online)

This issue relates to (duplicates/adds to?): MC-63802

When using /stop server executes the shutdown sequence twice (and hangs if players are online).

There is actually two (closely connected) issues here:
1. The shutdown sequence shouldn't be processed twice when the server got shut down with /stop
2. The second shutdown sequence hangs when closing the open connections

When stopping a server with at least one player online the console output will look like this:

Console Output

[23:10:19] [Server thread/INFO]: Panda4994 joined the game
[23:10:22] [Server thread/INFO]: [Panda4994: Stopping the server]
[23:10:22] [Server thread/INFO]: Stopping server
[23:10:22] [Server thread/INFO]: Saving players
[23:10:22] [Server thread/INFO]: Saving worlds
[23:10:22] [Server thread/INFO]: Saving chunks for level 'mcpworld'/Overworld
[23:10:22] [Server thread/INFO]: Saving chunks for level 'mcpworld'/Nether
[23:10:22] [Server thread/INFO]: Saving chunks for level 'mcpworld'/The End
[23:10:22] [Server Shutdown Thread/INFO]: Stopping server
[23:10:22] [Server Shutdown Thread/INFO]: Saving players

The process doesn't stop after that (This might be dependent on the enviroment).

As you can see "Stopping server" appears twice.
Once executed by the "Server thread" and once by the "Server Shutdown Thread" while the second one hangs after saving the players.

The first time it runs it's the normal shutdown from when the server gets stopped normally (It's MCP-Names but should be understandable what's meant).

MinecraftServer.java

public void run() {
	try {
		...
		while (this.serverRunning) {	// Main loop
			...
		}
		...
	}
	catch (...) {...}
	finally {
		...
		this.stopServer();	// What I call shutdown sequence. Doing these things: "Stopping server", "Saving players", "Saving worlds"
		this.serverStopped = true;
		...
		this.systemExitNow();	// Just calls System.exit(0);
	}
}

The second one is a shutdown hook which gets initialized at (pretty much) the end of the main method.

MinecraftServer.java

public static void main(String[] args) {
	...
	Runtime.getRuntime().addShutdownHook(new Thread("Server Shutdown Thread") {
		public void run() {
			dedicatedServerInstance.stopServer();	// What I call shutdown sequence. Doing these things: "Stopping server", "Saving players", "Saving worlds"
		}
	});
	...
}

Fix for issue 1.
A fix for the issue that the shutdown sequence is run twice is rather simple.

Just add a check to the shutdown hook that it only runs if the shutdown sequence didn't run before.
Like this:

MinecraftServer.java (changed)

public static void main(String[] args) {
	...
	Runtime.getRuntime().addShutdownHook(new Thread("Server Shutdown Thread") {
		public void run() {
			if (!dedicatedServerInstance.isServerStopped()) {
				dedicatedServerInstance.stopServer();
			}
		}
	});
	...
}

This also fixes the server hanging when using /stop as the shutdown hook only runs if the first shutdown failed.
The second shutdown sequence is still broken though.

Fix for issue 2.
For the problem that the shutdown hook hangs I don't have a fix yet.
But I tracked down the issue a bit.
It hangs after closing the connections to the players while creating a thread.

NetHandlerPlayServer.java

public void kickPlayerFromServer(String reason) {
	...
	// Futures.getUnchecked(...) never returns when called by the shutdown hook
	Futures.getUnchecked(this.serverController.addScheduledTask(new Runnable() {
		public void run() {
			NetHandlerPlayServer.this.netManager.checkDisconnected();
		}
	}));
}

I assume that it hangs in a deadlock or something like that since the documentation of addShutdownHook() warns about it (http://docs.oracle.com/javase/7/docs/api/java/lang/Runtime.html#addShutdownHook%28java.lang.Thread%29).
I didn't look into Futures too much so I'm not sure where exactly it's coming from.

The closest to a fix I got for that would be to remove the kicking of player in the shutdown hook as it doesn't work at the moment anyways.

But as long as /stop works fine again and doesn't mess up our scripts to start and stop server I'm fine with it 🙂