Synchronous or Asynchronous? That is the question...

Happy Equinox, All!

I'm writing a plugin that will query MySQL once a minute and based on the query result, may send a message to one or more players. I have read a lot of warnings against accessing the Bukkit API while using asynchronous tasks. Of course, I would want to read the player list. However, I would not want my plugin blocking the server processes if the query took a second or two.

1. How can I access the Bukkit (1.7.2 beta) player list safely from an asynchronous task?
2. Can a MySQL query become blocking to CraftBukkit processes or induce lag if run every minute?
3. Can I safely share variables like a boolean status between an asynchronous task and an event handler?

Thanks and play harder!
Cheers!

 

1. no
2. yes
3. no

if you want to process querry in async - process in async but then fetch results from sync task, evoid blocking main thread in any way, this will cause issues at random.

 

So, I keyed off of the Bukkit API docs in the Tips for Thread Safety.

3. An asynchronous task can schedule a synchronous task. ​

4. A synchronous task can schedule an asynchronous task.​

​

In onEnable() I added... Runs every 10 seconds after an initial 10 second delay.

Code:java

 
Bukkit.getScheduler().runTaskTimerAsynchronously(this, new ReminderTask(this), 200L, 200L);
 

I added this class which implements Runnable

Code:java

 
public class ReminderTask implements Runnable {
 
   private final Reminder plugin;
 
   public ReminderTask(Reminder plugin) {
      this.plugin = plugin;
   }
 
   @Override
   public void run() {
      Bukkit.getScheduler().runTask(plugin, new Runnable() {
         @Override
         public void run() {
            plugin.getLogger().info("Running ReminderTask");
         }
      });
   }
}
 

Seems to agree with other patterns I looked at. So I guess my answer is now yes, yes and yes. Ha!

 

@Chrisbotcom Well, you're creating 2 Runnable classes there. You have an actual class with that file and then you have an inner class when you do new Runnable() {class stuff here}.​

​

When you schedule a task in the onEnable like you did, it'll call the public void run() method every time you schedule it to tick. No need to schedule another task inside of there.​

​

And RawCode was almost correct, should be no yes yes. Just learn to pass objects ​

 

1. No, player lists are definitely NOT thread-safe. Instead, use the runTask() or a runSync() methods (which can be used in an async method because they are thread-safe) to get a list of players.
2. Yes, helps if you run it async.
3. Yes, you can cache the data or store it in an external variable using a synchronized method and then have an event handler read it.

 

java sync blocks have nice feature, all reads\writes must be synced, if you sync some object at one place and not sync at other place, you still can hit aba\concurrent modification\race condition and similar issues, since other thread wont attempt to sync and just read\write object ignoring possible modifications from other threads.

variables can be shared, but this wont be safe.
since playerlist not synchronized in vanilla implementation, IO from concurrent threads always unsafe no matter how you going to implement it (ever CAS can fail sometimes)
drtshock skyrimfan1

Actually this is very very sad, ever bukkit stuff dont know how java concurrency works:

Code:

	public static long AsyncField = 0xBA1l;
	public static Object lock = new Object();
	
	static public void main(String[] args) throws Throwable {
		
		final Thread a = new Thread(){
			public void run(){
				while(true)
				{
					//synchronized(lock)
					//{
					AsyncField++;
					//}
//bukkit dont have sync blocks at random places by default
//without sync block at every location it wont work
//run code with and without comments on sync block, remember results, have fun, read oracle specs, go outside, answer is no yes no
				}
			}
		};
		
		final Thread b = new Thread(){
			public void run(){
				while(true)
				{
					synchronized(lock)
					{
						System.out.println(AsyncField);
						System.out.println(AsyncField);
					}
					
					
					try{Thread.sleep(1000);}catch(Exception e){};
				}
			}
		};
		
		a.start();
		b.start();

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

1. First, you schedule a task to run synchronously which gets an array of players
   Code:

   ...
   @Override public void run() {
       final Player[] players = server.getOnlinePlayers();
   }

   Then, you schedule an asynchronous task that stores a reference to the array:
   Code:

   public class SomeSQL extends BukkitRunnable {
       private final Player[] players;
   
       public SomeSQL(Player[] players) { this.players = players; }
   }

   Code:

   ...
   @Override public void run() {
       final Player[] players = server.getOnlinePlayers();
       new SomeSQL(players).runTaskAsynchronously(plugin);
   }

2. Inside SomeSQL, write your 'blocking' operation
3. If you have a variable you need to share, consult the javadocs for AtomicBoolean (or whichever type you need)

 

You do see that the first Task is Async and the second is Sync, right? I've added blocking code and a state checking boolean to prevent the Async function from running if it is still blocking. I'm running it up on my test server now. Zero lag, processes are stable, memory seems happy and the CPU is not overtasked.

Sync (synchronized) block or runTask() synchronized method. 6 of one or a half-dozen of the other. Makes no difference. Its still synchronized. See http://stackoverflow.com/questions/574240/synchronized-block-vs-synchronized-method.

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

Nevertheless, by enclosing your call in a synchronous task, you can mitigate the blocking code. It looks cleaner and is guaranteed to be thread-safe. Calling "synchronous" as a paramater or using it as a block doesn't necessarily make accessing the Bukkit API thread-safe within the method. See Wolvereness above post for how you can access the player list safely.

As for the others, SQL you always want to run asynchronously so it doesn't clog the main thread, and to share a variable asynchronously, cache it within an AtomicClass variable (like AtomicInteger, AtomicBoolean, etc.)

 

If you insist on the "first" task being asynchronous, then make your playerlist call synchronous!

Code:

...
@Override public void run() { // Asynchronous!
    BukkitScheduler scheduler = plugin.getServer().getScheduler();
    Player[] players = scheduler.callSyncMethod(new Callable<Player[]>() {
        public Player[] call() {
            return plugin.getServer().getOnlinePlayers();
        }
    }).get(); // Somewhere, add a try-catch
}

/edit: In the spirit of recent events, the same solution in Java 8:

Code:

...
@Override public void run() { // Asynchronous!
    BukkitScheduler scheduler = plugin.getServer().getScheduler();
    Player[] players = scheduler.callSyncMethod(() -> plugin.getServer().getOnlinePlayers()).get(); // Somewhere, add a try-catch
}

 

So, here's what I have now. This Runnable is called as asynchronous from onLoad() like I posted above.

Code:java

 
public class ReminderTask implements Runnable {
 
    private final Reminder plugin;
    private boolean isRunning;
 
    public ReminderTask(Reminder plugin) {
    this.plugin = plugin;
    }
 
    @Override
    public void run() {
 
        // Based on code examples, this is safe.
        if (isRunning) {
            plugin.getLogger().info("ReminderTask is busy.");
            return;
        }
 
        isRunning = true;
 
        Player[] playerList;
 
        // A direct call to the Bukkit API requires a synchronized call.
        synchronized (this) {
            playerList = Bukkit.getOnlinePlayers();
        }
 
        // Do something very busy.
        String sql = "SELECT * FROM reminders";
        PreparedStatement preparedStatement = null;
        for (int i = 0; i < 1000; i++) {
            try {
                preparedStatement = plugin.db.prepareStatement(sql);
                preparedStatement.executeQuery();
            } catch (SQLException e) {
                // TODO Auto-generated catch block
               e.printStackTrace();
            }
        }
 
        // Do something else ridiculously busy
        for (int i = 0; i < 1000; i++) {
            for(Player player : playerList) {
                plugin.getLogger().info(player.getName());
            }
        }
 
        isRunning = false;
    }
}
 

 

No. Stop doing that. You don't understand what a "synchronized" block means, nor do you understand what "synchronous" and "asynchronous" mean in the context of Bukkit. Look at my solution:

It clearly shows you how to call something synchronous from an asynchronous thread.

 

Wait! No! Stop that!


The method callSyncMethod(Plugin, Callable<T>) in the type BukkitScheduler is not applicable for the arguments (new Callable<Player[]>(){})