Bukkit thread safety checks and fixes

src/main/java/fr/xephi/authme/AuthMe.java

@@ -31,6 +31,7 @@
 import fr.xephi.authme.settings.properties.SecuritySettings;
 import fr.xephi.authme.task.CleanupTask;
 import fr.xephi.authme.task.purge.PurgeService;
+import fr.xephi.authme.util.BukkitThreadSafety;
 import fr.xephi.authme.util.ExceptionUtils;
 import org.bukkit.Server;
 import org.bukkit.command.Command;
@@ -171,6 +172,10 @@ public void onEnable() {
 
  // Successful message
  logger.info("AuthMe " + getPluginVersion() + " build n." + getPluginBuildNumber() + " successfully enabled!");
+ // Start catching wrong sync/async calls
+ if (System.getProperty("authme.disableThreadSafetyChecks") == null) {
+ BukkitThreadSafety.setEnabled(true);
+ }
 
  // Purge on start if enabled
  PurgeService purgeService = injector.getSingleton(PurgeService.class);

src/main/java/fr/xephi/authme/annotation/MightBeAsync.java

@@ -0,0 +1,11 @@
+package fr.xephi.authme.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
+@Retention(RetentionPolicy.SOURCE)
+public @interface MightBeAsync {
+}

src/main/java/fr/xephi/authme/annotation/ShouldBeAsync.java

@@ -0,0 +1,11 @@
+package fr.xephi.authme.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
+@Retention(RetentionPolicy.SOURCE)
+public @interface ShouldBeAsync {
+}

src/main/java/fr/xephi/authme/command/executable/authme/AccountsCommand.java

@@ -32,40 +32,34 @@ public void executeCommand(final CommandSender sender, List<String> arguments) {
 
  // Assumption: a player name cannot contain '.'
  if (playerName.contains(".")) {
- bukkitService.runTaskAsynchronously(new Runnable() {
- @Override
- public void run() {
- List<String> accountList = dataSource.getAllAuthsByIp(playerName);
- if (accountList.isEmpty()) {
- sender.sendMessage("[AuthMe] This IP does not exist in the database.");
- } else if (accountList.size() == 1) {
- sender.sendMessage("[AuthMe] " + playerName + " is a single account player");
- } else {
- outputAccountsList(sender, playerName, accountList);
- }
+ bukkitService.runTaskAsynchronously(() -> {
+ List<String> accountList = dataSource.getAllAuthsByIp(playerName);
+ if (accountList.isEmpty()) {
+ sender.sendMessage("[AuthMe] This IP does not exist in the database.");
+ } else if (accountList.size() == 1) {
+ sender.sendMessage("[AuthMe] " + playerName + " is a single account player");
+ } else {
+ outputAccountsList(sender, playerName, accountList);
  }
  });
  } else {
- bukkitService.runTaskAsynchronously(new Runnable() {
- @Override
- public void run() {
- PlayerAuth auth = dataSource.getAuth(playerName.toLowerCase());
- if (auth == null) {
- commonService.send(sender, MessageKey.UNKNOWN_USER);
- return;
- } else if (auth.getLastIp() == null) {
- sender.sendMessage("No known last IP address for player");
- return;
- }
+ bukkitService.runTaskAsynchronously(() -> {
+ PlayerAuth auth = dataSource.getAuth(playerName.toLowerCase());
+ if (auth == null) {
+ commonService.send(sender, MessageKey.UNKNOWN_USER);
+ return;
+ } else if (auth.getLastIp() == null) {
+ sender.sendMessage("No known last IP address for player");
+ return;
+ }
 
- List<String> accountList = dataSource.getAllAuthsByIp(auth.getLastIp());
- if (accountList.isEmpty()) {
- commonService.send(sender, MessageKey.UNKNOWN_USER);
- } else if (accountList.size() == 1) {
- sender.sendMessage("[AuthMe] " + playerName + " is a single account player");
- } else {
- outputAccountsList(sender, playerName, accountList);
- }
+ List<String> accountList = dataSource.getAllAuthsByIp(auth.getLastIp());
+ if (accountList.isEmpty()) {
+ commonService.send(sender, MessageKey.UNKNOWN_USER);
+ } else if (accountList.size() == 1) {
+ sender.sendMessage("[AuthMe] " + playerName + " is a single account player");
+ } else {
+ outputAccountsList(sender, playerName, accountList);
  }
  });
  }

src/main/java/fr/xephi/authme/command/executable/authme/ConverterCommand.java

@@ -5,7 +5,7 @@
 import com.google.common.collect.ImmutableSortedMap;
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.command.ExecutableCommand;
-import fr.xephi.authme.datasource.converter.Converter;
+import fr.xephi.authme.datasource.converter.AbstractConverter;
 import fr.xephi.authme.datasource.converter.CrazyLoginConverter;
 import fr.xephi.authme.datasource.converter.LoginSecurityConverter;
 import fr.xephi.authme.datasource.converter.MySqlToSqlite;
@@ -30,7 +30,7 @@
 public class ConverterCommand implements ExecutableCommand {
 
  @VisibleForTesting
- static final Map<String, Class<? extends Converter>> CONVERTERS = getConverters();
+ static final Map<String, Class<? extends AbstractConverter>> CONVERTERS = getConverters();
 
  private final ConsoleLogger logger = ConsoleLoggerFactory.get(ConverterCommand.class);
 
@@ -41,37 +41,34 @@ public class ConverterCommand implements ExecutableCommand {
  private BukkitService bukkitService;
 
  @Inject
- private Factory<Converter> converterFactory;
+ private Factory<AbstractConverter> converterFactory;
 
  @Override
  public void executeCommand(CommandSender sender, List<String> arguments) {
- Class<? extends Converter> converterClass = getConverterClassFromArgs(arguments);
+ Class<? extends AbstractConverter> converterClass = getConverterClassFromArgs(arguments);
  if (converterClass == null) {
  sender.sendMessage("Converters: " + String.join(", ", CONVERTERS.keySet()));
  return;
  }
 
  // Get the proper converter instance
- final Converter converter = converterFactory.newInstance(converterClass);
+ final AbstractConverter converter = converterFactory.newInstance(converterClass);
 
  // Run the convert job
- bukkitService.runTaskAsynchronously(new Runnable() {
- @Override
- public void run() {
- try {
- converter.execute(sender);
- } catch (Exception e) {
- commonService.send(sender, MessageKey.ERROR);
- logger.logException("Error during conversion:", e);
- }
+ bukkitService.runTaskAsynchronously(() -> {
+ try {
+ converter.execute(sender);
+ } catch (Exception e) {
+ commonService.send(sender, MessageKey.ERROR);
+ logger.logException("Error during conversion:", e);
  }
  });
 
  // Show a status message
  sender.sendMessage("[AuthMe] Successfully started " + arguments.get(0));
  }
 
- private static Class<? extends Converter> getConverterClassFromArgs(List<String> arguments) {
+ private static Class<? extends AbstractConverter> getConverterClassFromArgs(List<String> arguments) {
  return arguments.isEmpty()
  ? null
  : CONVERTERS.get(arguments.get(0).toLowerCase());
@@ -82,8 +79,8 @@ private static Class<? extends Converter> getConverterClassFromArgs(List<String>
  *
  * @return map with all available converters
  */
- private static Map<String, Class<? extends Converter>> getConverters() {
- return ImmutableSortedMap.<String, Class<? extends Converter>>naturalOrder()
+ private static Map<String, Class<? extends AbstractConverter>> getConverters() {
+ return ImmutableSortedMap.<String, Class<? extends AbstractConverter>>naturalOrder()
  .put("xauth", XAuthConverter.class)
  .put("crazylogin", CrazyLoginConverter.class)
  .put("rakamak", RakamakConverter.class)

src/main/java/fr/xephi/authme/command/executable/authme/PurgePlayerCommand.java

@@ -1,5 +1,6 @@
 package fr.xephi.authme.command.executable.authme;
 
+import fr.xephi.authme.annotation.ShouldBeAsync;
 import fr.xephi.authme.command.ExecutableCommand;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.service.BukkitService;
@@ -29,10 +30,11 @@ public class PurgePlayerCommand implements ExecutableCommand {
  @Override
  public void executeCommand(CommandSender sender, List<String> arguments) {
  String option = arguments.size() > 1 ? arguments.get(1) : null;
- bukkitService.runTaskOptionallyAsync(
+ bukkitService.runTaskAsynchronously(
  () -> executeCommand(sender, arguments.get(0), option));
  }
 
+ @ShouldBeAsync
  private void executeCommand(CommandSender sender, String name, String option) {
  if ("force".equals(option) || !dataSource.isAuthAvailable(name)) {
  OfflinePlayer offlinePlayer = bukkitService.getOfflinePlayer(name);

src/main/java/fr/xephi/authme/datasource/AbstractSqlDataSource.java

@@ -4,6 +4,8 @@
 import ch.jalu.datasourcecolumns.data.DataSourceValueImpl;
 import ch.jalu.datasourcecolumns.data.DataSourceValues;
 import ch.jalu.datasourcecolumns.predicate.AlwaysTruePredicate;
+import fr.xephi.authme.util.BukkitThreadSafety;
+import fr.xephi.authme.annotation.ShouldBeAsync;
 import fr.xephi.authme.data.auth.PlayerAuth;
 import fr.xephi.authme.datasource.columnshandler.AuthMeColumns;
 import fr.xephi.authme.datasource.columnshandler.AuthMeColumnsHandler;
@@ -27,7 +29,9 @@ public abstract class AbstractSqlDataSource implements DataSource {
  protected AuthMeColumnsHandler columnsHandler;
 
  @Override
+ @ShouldBeAsync
  public boolean isAuthAvailable(String user) {
+ BukkitThreadSafety.shouldBeAsync();
  try {
  return columnsHandler.retrieve(user, AuthMeColumns.NAME).rowExists();
  } catch (SQLException e) {
@@ -37,7 +41,9 @@ public boolean isAuthAvailable(String user) {
  }
 
  @Override
+ @ShouldBeAsync
  public HashedPassword getPassword(String user) {
+ BukkitThreadSafety.shouldBeAsync();
  try {
  DataSourceValues values = columnsHandler.retrieve(user, AuthMeColumns.PASSWORD, AuthMeColumns.SALT);
  if (values.rowExists()) {
@@ -50,15 +56,19 @@ public HashedPassword getPassword(String user) {
  }
 
  @Override
+ @ShouldBeAsync
  public boolean saveAuth(PlayerAuth auth) {
+ BukkitThreadSafety.shouldBeAsync();
  return columnsHandler.insert(auth,
  AuthMeColumns.NAME, AuthMeColumns.NICK_NAME, AuthMeColumns.PASSWORD, AuthMeColumns.SALT,
  AuthMeColumns.EMAIL, AuthMeColumns.REGISTRATION_DATE, AuthMeColumns.REGISTRATION_IP,
  AuthMeColumns.UUID);
  }
 
  @Override
+ @ShouldBeAsync
  public boolean hasSession(String user) {
+ BukkitThreadSafety.shouldBeAsync();
  try {
  DataSourceValue<Integer> result = columnsHandler.retrieve(user, AuthMeColumns.HAS_SESSION);
  return result.rowExists() && Integer.valueOf(1).equals(result.getValue());
@@ -69,31 +79,41 @@ public boolean hasSession(String user) {
  }
 
  @Override
+ @ShouldBeAsync
  public boolean updateSession(PlayerAuth auth) {
+ BukkitThreadSafety.shouldBeAsync();
  return columnsHandler.update(auth, AuthMeColumns.LAST_IP, AuthMeColumns.LAST_LOGIN, AuthMeColumns.NICK_NAME);
  }
 
  @Override
+ @ShouldBeAsync
  public boolean updatePassword(PlayerAuth auth) {
+ BukkitThreadSafety.shouldBeAsync();
  return updatePassword(auth.getNickname(), auth.getPassword());
  }
 
  @Override
+ @ShouldBeAsync
  public boolean updatePassword(String user, HashedPassword password) {
+ BukkitThreadSafety.shouldBeAsync();
  return columnsHandler.update(user,
  with(AuthMeColumns.PASSWORD, password.getHash())
  .and(AuthMeColumns.SALT, password.getSalt()).build());
  }
 
  @Override
+ @ShouldBeAsync
  public boolean updateQuitLoc(PlayerAuth auth) {
+ BukkitThreadSafety.shouldBeAsync();
  return columnsHandler.update(auth,
  AuthMeColumns.LOCATION_X, AuthMeColumns.LOCATION_Y, AuthMeColumns.LOCATION_Z,
  AuthMeColumns.LOCATION_WORLD, AuthMeColumns.LOCATION_YAW, AuthMeColumns.LOCATION_PITCH);
  }
 
  @Override
+ @ShouldBeAsync
  public List<String> getAllAuthsByIp(String ip) {
+ BukkitThreadSafety.shouldBeAsync();
  try {
  return columnsHandler.retrieve(eq(AuthMeColumns.LAST_IP, ip), AuthMeColumns.NAME);
  } catch (SQLException e) {
@@ -103,17 +123,23 @@ public List<String> getAllAuthsByIp(String ip) {
  }
 
  @Override
+ @ShouldBeAsync
  public int countAuthsByEmail(String email) {
+ BukkitThreadSafety.shouldBeAsync();
  return columnsHandler.count(eqIgnoreCase(AuthMeColumns.EMAIL, email));
  }
 
  @Override
+ @ShouldBeAsync
  public boolean updateEmail(PlayerAuth auth) {
+ BukkitThreadSafety.shouldBeAsync();
  return columnsHandler.update(auth, AuthMeColumns.EMAIL);
  }
 
  @Override
+ @ShouldBeAsync
  public boolean isLogged(String user) {
+ BukkitThreadSafety.shouldBeAsync();
  try {
  DataSourceValue<Integer> result = columnsHandler.retrieve(user, AuthMeColumns.IS_LOGGED);
  return result.rowExists() && Integer.valueOf(1).equals(result.getValue());
@@ -124,42 +150,58 @@ public boolean isLogged(String user) {
  }
 
  @Override
+ @ShouldBeAsync
  public void setLogged(String user) {
+ BukkitThreadSafety.shouldBeAsync();
  columnsHandler.update(user, AuthMeColumns.IS_LOGGED, 1);
  }
 
  @Override
+ @ShouldBeAsync
  public void setUnlogged(String user) {
+ BukkitThreadSafety.shouldBeAsync();
  columnsHandler.update(user, AuthMeColumns.IS_LOGGED, 0);
  }
 
  @Override
+ @ShouldBeAsync
  public void grantSession(String user) {
+ BukkitThreadSafety.shouldBeAsync();
  columnsHandler.update(user, AuthMeColumns.HAS_SESSION, 1);
  }
 
  @Override
+ @ShouldBeAsync
  public void revokeSession(String user) {
+ BukkitThreadSafety.shouldBeAsync();
  columnsHandler.update(user, AuthMeColumns.HAS_SESSION, 0);
  }
 
  @Override
+ @ShouldBeAsync
  public void purgeLogged() {
+ BukkitThreadSafety.shouldBeAsync();
  columnsHandler.update(eq(AuthMeColumns.IS_LOGGED, 1), AuthMeColumns.IS_LOGGED, 0);
  }
 
  @Override
+ @ShouldBeAsync
  public int getAccountsRegistered() {
+ BukkitThreadSafety.shouldBeAsync();
  return columnsHandler.count(new AlwaysTruePredicate<>());
  }
 
  @Override
+ @ShouldBeAsync
  public boolean updateRealName(String user, String realName) {
+ BukkitThreadSafety.shouldBeAsync();
  return columnsHandler.update(user, AuthMeColumns.NICK_NAME, realName);
  }
 
  @Override
+ @ShouldBeAsync
  public DataSourceValue<String> getEmail(String user) {
+ BukkitThreadSafety.shouldBeAsync();
  try {
  return columnsHandler.retrieve(user, AuthMeColumns.EMAIL);
  } catch (SQLException e) {