Shynixn / MCCoroutine

MCCoroutine is a library, which adds extensive support for Kotlin Coroutines for Minecraft Server environments.
Other
211 stars 19 forks source link

Potential error on reload #32

Closed AllanWang closed 3 years ago

AllanWang commented 3 years ago

I'm testing out coroutines by routing all events to a shared flow. I'm getting the following error on /reload:

Caused by: java.util.zip.ZipException: ZipFile invalid LOC header (bad signature)
        at java.util.zip.ZipFile$ZipFileInputStream.initDataOffset(ZipFile.java:1003) ~[?:?]
        at java.util.zip.ZipFile$ZipFileInputStream.read(ZipFile.java:1013) ~[?:?]
        at java.util.zip.ZipFile$ZipFileInflaterInputStream.fill(ZipFile.java:468) ~[?:?]
        at java.util.zip.InflaterInputStream.read(InflaterInputStream.java:159) ~[?:?]
        at java.io.FilterInputStream.read(FilterInputStream.java:107) ~[?:?]
        at com.google.common.io.ByteStreams.copy(ByteStreams.java:106) ~[spigot-1.16.5.jar:3016-Spigot-73fb609-ea050e6]
        at com.google.common.io.ByteStreams.toByteArray(ByteStreams.java:166) ~[spigot-1.16.5.jar:3016-Spigot-73fb609-ea050e6]
        at org.bukkit.plugin.java.PluginClassLoader.findClass(PluginClassLoader.java:135) ~[spigot-1.16.5.jar:3016-Spigot-73fb609-ea050e6]
        at org.bukkit.plugin.java.PluginClassLoader.findClass(PluginClassLoader.java:96) ~[spigot-1.16.5.jar:3016-Spigot-73fb609-ea050e6]
        at java.lang.ClassLoader.loadClass(ClassLoader.java:589) ~[?:?]
        at java.lang.ClassLoader.loadClass(ClassLoader.java:522) ~[?:?]
        at kotlinx.coroutines.JobSupport.getChildren(JobSupport.kt:947) ~[?:?]
        at kotlinx.coroutines.JobKt__JobKt.cancelChildren(Job.kt:628) ~[?:?]
        at kotlinx.coroutines.JobKt.cancelChildren(Unknown Source) ~[?:?]
        at kotlinx.coroutines.JobKt__JobKt.cancelChildren$default(Job.kt:627) ~[?:?]
        at kotlinx.coroutines.JobKt.cancelChildren$default(Unknown Source) ~[?:?]
        at com.github.shynixn.mccoroutine.service.CoroutineSessionImpl.dispose(CoroutineSessionImpl.kt:57) ~[?:?]
        at com.github.shynixn.mccoroutine.impl.MCCoroutineImpl.disable(MCCoroutineImpl.kt:32) ~[?:?]
        at com.github.shynixn.mccoroutine.listener.PluginListener.onPluginDisable(PluginListener.kt:19) ~[?:?]
        at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]
        at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:?]
        at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:?]
        at java.lang.reflect.Method.invoke(Method.java:566) ~[?:?]
        at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:315) ~[spigot-1.16.5.jar:3016-Spigot-73fb609-ea050e6]

Perhaps this is related to how the scope is disposed? Or more specifically, usage of Plugin.findClazz

Shynixn commented 3 years ago

I don't think this is related to the Plugin.findClazz of MCCoroutine but you are right that something breaks during scope disposing. It seems like the PluginClassLoader of Bukkit fails to load a Class of Kotlin Coroutines on the job.cancelChildren call of MCCoroutine.

Did you replace your Plugin.jar during runtime?

If you did then it is not an error per say because replacing the jar file during runtime will always cause unintended side effects which MCCoroutine might not be able to solve. If you did not replace your jar file before executing reload then it is indeed an error.

A possible workaround might be simply calling job.cancelChildren() at plugin enable inside MCCoroutine, so that all required classes for disposing are already cached by the classloader when it is finally required on PluginDisable.

You could try to simply call this.scope.cancelChildren() at the beginning of your fun onEnable() and take a look if it helps.

AllanWang commented 3 years ago

I've done a few more tests, and this is a problem with coroutines in general. Not sure if we need to await on cancel. I'm calling it during onDisable, though I wonder if it matters at all. Upon reload, I assume the plugin will get a new scope, so calling it during onEnable before anything else might not do anything.

Shynixn commented 3 years ago

Can you please share your sample code how to replicate this error?