MCCoroutine icon indicating copy to clipboard operation
MCCoroutine copied to clipboard

Close the coroutine session after JavaPlugin#onDisable has been called

Open vytskalt opened this issue 1 year ago • 5 comments

In my JavPlugin#onDisable method, I would like to save the data of a player to a database. However when I try to do that, I get this error:

java.lang.RuntimeException: Plugin wool-wars attempt to start a new coroutine session while being disabled. Dispatchers such as plugin.minecraftDispatcher and plugin.asyncDispatcher are already disposed at this point and cannot be used!

I think it would be great if the coroutine session was only closed after the onDisable method was called, so we could do things like I described earlier.

vytskalt avatar Sep 20 '22 18:09 vytskalt

I think this issue needs a lot of technical explanations. I'll post it here and link it in the wiki later on.

TL;DR: (If you just need the solution):

  • In onDisable you are encouraged to use runBlocking and this is fine. (or use onDisableAsync)
  • Use Dispatchers.IO because the async bukkit scheduler is already shutdown at this point
  override fun onDisable() {
        runBlocking {
            println("Pre save")

            withContext(Dispatchers.IO){
                println("Simulate save")
                Thread.sleep(5000)
            }

            println("postSave")
        }
    }

Long version:

If we take all coroutine stuff away and just work with the Bukkit Scheduler, you will quickly run into exceptions if you try to start new tasks (synch and async) from the onDisable method.

override fun onDisable() {
        val plugin = this
        println("Pre save")

        Bukkit.getScheduler().runTaskAsynchronously(plugin, Runnable {
            println("Simulate save")
            Thread.sleep(5000)

            Bukkit.getScheduler().runTask(plugin, Runnable {
                println("Post save")
            })
        })
}
[21:43:46] [Server thread/INFO]: [MCCoroutine-Sample] Disabling MCCoroutine-Sample v2.5.0
[21:43:46] [Server thread/INFO]: Pre save
[21:43:46] [Server thread/ERROR]: Error occurred while disabling MCCoroutine-Sample v2.5.0 (Is it up to date?)
org.bukkit.plugin.IllegalPluginAccessException: Plugin attempted to register task while disabled

As MCCoroutine wraps the Bukkit Scheduler, we simply cannot use it and we cannot disable it later. Even if we could (I did some testing), we would run into the following problems:

  • On plugin reload, the tasks would run in the background and may interfere with the newly loaded plugin (dangerous)
  • On server shutdown, the server would continue to stop all processes and may interrupt your running background proccesses (data corruption is highly likely)

In order to avoid both of these problems, we can lock the main thread using runBlocking and use other dispatchers which are not bound to the BukkitScheduler e.g. Dispatchers.IO to achieve our goal. See the first section for the correct code.

Final notes

I need to adjust the exception message to give developers a hint how to solve it.

Shynixn avatar Sep 20 '22 19:09 Shynixn

Maybe add a new launch method which will run runBlocking instead if the plugin is disabled? So we don't need to do something like this:

if (!plugin.isEnabled) {
  runBlocking { /* do something */ }
} else {
  plugin.launch { /* do the same thing */ }
}

vytskalt avatar Sep 21 '22 14:09 vytskalt

Hm... why would you need that in the first place? Just call runBlocking once in your onDisable function and call your other suspend functions.

override fun onDisable() {
        runBlocking {
            println("Pre save")
            doSave()
            println("postSave")
        }
 }

suspend fun doSave(){
         withContext(Dispatchers.IO){
                println("Simulate save")
                Thread.sleep(5000)
        }
}

Shynixn avatar Sep 21 '22 18:09 Shynixn

Hm... why would you need that in the first place? Just call runBlocking once in your onDisable function and call your other suspend functions.

override fun onDisable() {
        runBlocking {
            println("Pre save")
            doSave()
            println("postSave")
        }
 }

suspend fun doSave(){
         withContext(Dispatchers.IO){
                println("Simulate save")
                Thread.sleep(5000)
        }
}

Doing that isn't really possible right now for me because of how my plugin is designed. I guess I could just rewrite it though

vytskalt avatar Sep 22 '22 18:09 vytskalt

I see. Sorry, but I cannot implement this now because it would cause other issues. (e.g. deadlocks). I want to keep the coroutines as pure as possible. However, I'll think about it and I may come up with a good solution in the future.

You could add your code as an extension function in your plugin, so you do not need to rewrite eveything.

if (!plugin.isEnabled) {
  runBlocking { /* do something */ }
} else {
  plugin.launch { /* do the same thing */ }
}
´´´
 

Shynixn avatar Sep 22 '22 19:09 Shynixn