-
Notifications
You must be signed in to change notification settings - Fork 274
fix(amazonq): fix connection manager deadlock when q ListAvailableProfiles is long #5547
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…files is long
`CodeWhispererStatusBarWidget` attempts to query active connections, but lock is being held while trying to list profiles
```
"AWT-EventQueue-0" prio=0 tid=0x0 nid=0x0 blocked
java.lang.Thread.State: BLOCKED
on software.aws.toolkits.jetbrains.core.credentials.DefaultToolkitConnectionManager@37995400 owned by "ApplicationImpl pooled thread 5 @coroutine#17104" Id=128
at software.aws.toolkits.jetbrains.core.credentials.DefaultToolkitConnectionManager.activeConnectionForFeature(DefaultToolkitConnectionManager.kt)
at software.aws.toolkits.jetbrains.services.amazonq.QUtilsKt.calculateIfIamIdentityCenterConnection(QUtils.kt:18)
at software.aws.toolkits.jetbrains.services.codewhisperer.customization.DefaultCodeWhispererModelConfigurator.activeCustomization(CodeWhispererModelConfigurator.kt:171)
at software.aws.toolkits.jetbrains.services.codewhisperer.status.CodeWhispererStatusBarWidget.getSelectedValue(CodeWhispererStatusBarWidget.kt:114)
at com.intellij.openapi.wm.impl.status.MultipleTextValues.beforeUpdate(IdeStatusBarImpl.kt:983)
```
```
- "coroutine#17104":BlockingCoroutine{Active}@7dced952, state: RUNNING [CoroutineId(17104), BlockingEventLoop@2da7461c]
at java.base/jdk.internal.misc.Unsafe.park(Native Method)
at java.base/java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:269)
at java.base/java.util.concurrent.CompletableFuture$Signaller.block(CompletableFuture.java:1866)
at java.base/java.util.concurrent.ForkJoinPool.unmanagedBlock(ForkJoinPool.java:4013)
at java.base/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3961)
at java.base/java.util.concurrent.CompletableFuture.timedGet(CompletableFuture.java:1939)
at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2095)
at migration.software.aws.toolkits.jetbrains.core.AwsResourceCache$Companion.wait(AwsResourceCache.kt:150)
at migration.software.aws.toolkits.jetbrains.core.AwsResourceCache$Companion.access$wait(AwsResourceCache.kt:145)
at migration.software.aws.toolkits.jetbrains.core.AwsResourceCache.getResourceNow(AwsResourceCache.kt:92)
at migration.software.aws.toolkits.jetbrains.core.AwsResourceCache.getResourceNow(AwsResourceCache.kt:105)
at software.aws.toolkits.jetbrains.services.amazonq.profile.QRegionProfileManager.listRegionProfiles(QRegionProfileManager.kt:70)
at software.aws.toolkits.jetbrains.services.amazonq.profile.QRegionProfileManager.validateProfile$lambda$0(QRegionProfileManager.kt:50)
at software.aws.toolkits.core.utils.ExceptionUtils.tryOrNull(ExceptionUtils.kt:11)
at software.aws.toolkits.jetbrains.services.amazonq.profile.QRegionProfileManager.validateProfile(QRegionProfileManager.kt:49)
at software.aws.toolkits.jetbrains.services.amazonq.toolwindow.AmazonQToolWindowFactory$createToolWindowContent$2.activeConnectionChanged(AmazonQToolWindowFactory.kt:68)
[...]
at software.aws.toolkits.jetbrains.core.credentials.DefaultToolkitConnectionManager.switchConnection(DefaultToolkitConnectionManager.kt:159)
```
| openMeetQPage(project) | ||
| QRegionProfileManager.getInstance().validateProfile(project) | ||
| ApplicationManager.getApplication().executeOnPooledThread { | ||
| QRegionProfileManager.getInstance().validateProfile(project) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i thoguht we already had this in project startup https://github.com/aws/aws-toolkit-jetbrains/blob/main/plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/startup/AmazonQStartupActivity.kt#L57
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really need this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both --
so if we are checking in project startup, do we still need this one then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i feel we shouldn't need this call at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we might need to recheck the behavior when a user logs out and logs back in with completely different profiles. From my testing, validating only during project startup doesn't seem to be sufficient. Please feel free to correct me if I’m missing something!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implement it in a different way
- log out clear the connection to profile arn cache, because anyway when you login, you have to reselect the profile
- remove
validateConnectiononConnectionChanged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the reasoning behind clearing the cache on logout. That said, I think we still need to call listProfiles inside onConnectionChanged. Otherwise, prepareChatContent won’t be able to determine whether to open the WebView or Q panel. Let me know what you think.
https://github.com/aws/aws-toolkit-jetbrains/blob/main/plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/toolwindow/AmazonQToolWindowFactory.kt#L126-L127
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes the list on prepareChatContent is separate from what this PR is addressing
Qodana Community for JVMIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at [email protected]
|
CodeWhispererStatusBarWidgetattempts to query active connections, but lock is being held while trying to list profilesLicense
I confirm that my contribution is made under the terms of the Apache 2.0 license.