Shynixn / MCCoroutine

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

Using a suspending Listener throws invisible exceptions #100

Closed RAVINGAR closed 1 year ago

RAVINGAR commented 1 year ago

Issue

So this is a bit of a strange issue I've encountered; when using a Suspending Listener (A listener registered with registerSuspendingEvents()), multiple Exceptions are thrown when listening to certain events and then are caught somewhere such that they don't appear in the server's log. The only way I was able to catch onto this was by analysing Spark's profiler and seeing that significant CPU time was being taken by jdk.internal.reflect.GeneratedMethodAccessor.invoke(). After additional testing I was eventually able to get logs which pointed to event listeners. I tracked it to the ChunkLoadEvent and PhysicsEvent.

(For the record, I am listening to these events with a SuspendingListener. I tested with and without the suspend keyword on the event listener functions but still encountered the issue)

From guess work I used a normal Listener interface and the Exceptions were now gone and so was the extra CPU time consumed by them.

Background of the Plugin

The plugin itself is a custom plugin that when loading chunks will scan each chunk and based on certain block and biome predicates replace an above-ground block with a custom vegetation (a custom model that drops unique drops). There is a lot of concurrency involved is the main point.

Steps to Reproduce

Evidence

Spark Profiler - USING Suspending Listener https://spark.lucko.me/EkXjQTa7Vp Screen Shot 2023-05-11 at 12 59 41 pm

Spark Profiler - USING Normal Listener https://spark.lucko.me/1UJTdh3uGz Screen Shot 2023-05-11 at 1 00 46 pm

Shynixn commented 1 year ago

Hello!

Thanks for the detailed report. I think this is related how MCCoroutine currently handles mixing suspend and non suspend functions in SuspendingListener. This line shows how MCCoroutine attempts to call an event method with suspend parameter first, waits for an IllegalArgumentException and then tries it again with no suspend parameter. This seems to be causing performance problems..

This is indeed a bug and should be fixed. In the meanwhile, you could use an ordinary listener and use plugin.launch{} where you need suspend operations.

Shynixn commented 1 year ago