Netflix / dgs-intellij-plugin

Apache License 2.0
22 stars 11 forks source link

Avoid calling StubIndex recursively which could cause a freeze in IDEA #72

Closed ljchen1 closed 1 year ago

ljchen1 commented 1 year ago

Currently, in com.netflix.dgs.plugin.services.internal.DgsServiceImpl.getDgsComponentIndex, I see dgs plugin code calls stubIndex recursively:

https://github.com/Netflix/dgs-intellij-plugin/blob/main/src/main/java/com/netflix/dgs/plugin/services/internal/DgsServiceImpl.java#L120-L129

            StubIndexKey<String, KtAnnotationEntry> key = KotlinAnnotationsIndex.getInstance().getKey();
            stubIndex.processAllKeys(key, project, annotation -> {
                if (annotations.contains(annotation)) {
                    StubIndex.getElements(key, annotation, project, GlobalSearchScope.projectScope(project), KtAnnotationEntry.class).forEach(dataFetcherAnnotation -> {
                        UAnnotation uElement = (UAnnotation) UastContextKt.toUElement(dataFetcherAnnotation);
                        if(uElement != null) {
                            processor.process(uElement);
                        }
                    });
                }
                return true;
            });

This would cause a freeze in IDEA because the IDEA doesn't support calling StubIndex recursively.

See here for details: https://youtrack.jetbrains.com/issue/IDEA-321133/ https://youtrack.jetbrains.com/issue/IDEA-322496/Linux-Wayland-All-IDEA-windows-become-unresponsive-when-a-newly-re-opened-project-begins-indexing#focus=Comments-27-7454887.0-0

Also, you could change the code to avoid calling StubIndex recursively like this: https://youtrack.jetbrains.com/issue/IDEA-321133/IDE-freezes-during-auto-completion-due-to-calling-StubIndex-recursively-and-causing-deadlock#focus=Comments-27-7403810.0-0

mr-nothing commented 1 year ago

I can confirm Intellij DGS plugin cause idea to freeze. There is nothing can be done but disabling plugin. Updated Idea today to latest version and plugin to 1.3.0 but the issue is still there. The first attempt to open Idea ended with a hung :(

srinivasankavitha commented 1 year ago

Thanks for reporting the issue. We'll look into fixing this.

On Mon, Jun 26, 2023 at 3:19 AM mr-nothing @.***> wrote:

I can confirm Intellij DGS plugin cause idea to freeze. There is nothing can be done but disabling plugin. Updated Idea today to latest version and plugin to 1.3.0 but the issue is still there. The first attempt to open Idea ended with a hung :(

— Reply to this email directly, view it on GitHub https://github.com/Netflix/dgs-intellij-plugin/issues/72#issuecomment-1607158607, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJ5JPXKFT4XG4NA45NBQDWTXNFOZHANCNFSM6AAAAAAZRFWVDI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

danielchemko commented 1 year ago

I can confirm this is happening locally as well. I can provide a stack trace of Idea in this state if needed.

srinivasankavitha commented 1 year ago

Could you provide the version of InteliiJ you are using? I wonder if this is related to an update.

ljchen1 commented 1 year ago

All these users are using 2023.1.x (one is using 2023.1.2 and two are using 2023.1.3) from the YouTrack. I have attached an example of freeze logs that doesn't include any sensitive info.

threadDumps-freeze-20230613-140659-IU-231.9011.34.zip

Below are more details from the Intellij developer's investigation: AFAIK, you could avoid using the StubIndex recursively currently as a workaround for this issue before IntelliJ supports calling StubIndex recursively in IDEA-321133 but maybe there be some other ways to fix this issue from your side:

From threadumps I would say it is indeed quite similar to IDEA-321133: details are different, but it is still due to recursive indexes lookup, which leads to non-stable order or index lock acquisitions.

I.e. here we have "Invoker.0" thread, there netflix plugin code lookups some stub index (StubIndexEx.processAllKeys()) -- which acquires apt index lock. And deeper down the stack there is a second attempt to lookup another index (KotlinScriptFqnIndex?), which tries to acquire its own lock -- but that lock is already owned by "Indexing" (id=1605) thread.

"Indexing" (id=1605) thread is indexing some composite index, and again: it acquires index1 lock during MapReduceIndex.updateWithMap(), and inside that lock deeper the stack it tries to run another MapReduceIndex.updateWithMap() for another index2 -- which also requires lock, but that lock is already held by (it seems to) "Invoker.0" thread.

So this turn deadlock is (seems to) due to different order of index processing (and index's lock acquisition) between "Indexing" thread, and the plugin recursive lookup. Which is slightly different scenario than in attached ticket, there conflict was between global storage lock, and per-index lock.

But in both cases 'offender' is recursive lookup: i.e. we can't predict in which order clients lookup indexes in recursion, hence order of per-lock acquisitions will always be unstable. We, in theory, could fix the order inside the platform code -- seems like we have done so mostly -- but can't do that for client code.

zapodot commented 1 year ago

👍🏼 I have the same issue and ended up disabling the DGS plugin alltogether. I am currently using IU-231.9161.38 on MacOS (aarch64)

srinivasankavitha commented 1 year ago

Thanks. I’ll fix this. A release should be available in the next day or soSent from my iPhoneOn Jun 29, 2023, at 5:43 AM, Sondre Eikanger Kvalø @.***> wrote: 👍🏼 I have the same issue and ended up disabling the DGS plugin alltogether. I am currently using IU-231.9161.38 on MacOS (aarch64)

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>