dkandalov / live-plugin

IntelliJ plugin for writing IntelliJ plugins at runtime ⚡️
https://plugins.jetbrains.com/plugin/7282
Apache License 2.0
858 stars 67 forks source link

Upgrading to 231.* #155

Closed BFergerson closed 1 year ago

BFergerson commented 1 year ago

IntelliJ 231.* introduces PluginAwareClassLoader.getPluginCoroutineScope. I'm not entirely sure how to deal with that change so I'm curious if you've noticed it and if you have an idea how to handle it.

p.s. thanks for adding the bug id to https://github.com/dkandalov/live-plugin/blob/286eb7a16de6e43bb78fb68c595600f4f2743ab5/src/main/liveplugin/implementation/pluginrunner/PluginClassLoader_Fork.java#L88

I ran into that issue too but had no clue what it meant: https://github.com/sourceplusplus/jetbrains-commander/blob/3e128bb6b60936debf3515e06e3d0f13fff4411b/src/main/liveplugin/implementation/pluginrunner/PluginClassLoader_Fork.java#L395-L402

        //todo: understand why this is needed
        if (className.equals("kotlin.coroutines.jvm.internal.ContinuationImpl")) {
            try {
                ClassLoader cl = Class.forName("kotlin.coroutines.jvm.internal.ContinuationImpl").getClassLoader();
                return cl instanceof PathClassLoader;
            } catch (Throwable ignore) {
            }
        }
dkandalov commented 1 year ago

Hi.

In short, returning new coroutine scope is good enough. I did this private CoroutineScope scope = CoroutineScopeKt.CoroutineScope(SupervisorJob(null)); but I might re-fork PluginClassLoader at some point as it was converted to Kotlin in IJ codebase.

Overall, it looks like each plugin is getting its own coroutine scope which is used for component initialisation and dynamic plugin unloading (I'm just really looking at com.intellij.ide.plugins.cl.PluginAwareClassLoader#getPluginCoroutineScope usages).

I guess that's a part of an attempt to move off JVM threads in IJ codebase. For example, there is experimental com.intellij.openapi.progress.TasksKt#runBlockingModal() with action: suspend CoroutineScope.() -> T. I can't see this making plugin development any easier though 🤷

BFergerson commented 1 year ago

Thanks for that information. I see you've already upgraded PluginClassLoader_Fork for 231.* so I'll go ahead and close this.