Skip to content

Commit

Permalink
Fix semaphore not updated in pair could make Q unusable (#5341)
Browse files Browse the repository at this point in the history
* Fix semaphore not updated in pair could make Q unusable

* detekt

* remove previous listener when states are updated
  • Loading branch information
andrewyuq authored Feb 7, 2025
1 parent 5416099 commit e3cfc8c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,9 @@ import com.intellij.ui.popup.AbstractPopup
import com.intellij.ui.popup.PopupFactoryImpl
import com.intellij.util.messages.Topic
import com.intellij.util.ui.UIUtil
import kotlinx.coroutines.sync.Semaphore
import software.amazon.awssdk.services.codewhispererruntime.model.Import
import software.amazon.awssdk.services.codewhispererruntime.model.Reference
import software.aws.toolkits.core.utils.debug
import software.aws.toolkits.core.utils.error
import software.aws.toolkits.core.utils.getLogger
import software.aws.toolkits.jetbrains.services.codewhisperer.editor.CodeWhispererEditorManager
import software.aws.toolkits.jetbrains.services.codewhisperer.layout.CodeWhispererLayoutConfig.addHorizontalGlue
Expand Down Expand Up @@ -88,8 +86,8 @@ import javax.swing.JLabel
class CodeWhispererPopupManager {
val popupComponents = CodeWhispererPopupComponents()

// Act like a semaphore: one increment only corresponds to one decrement
var allowEditsDuringSuggestionPreview = Semaphore(MAX_EDIT_SOURCE_DURING_SUGGESTION_PREVIEW)
var allowTypingDuringSuggestionPreview = false
var allowIntelliSenseDuringSuggestionPreview = false
var sessionContext = SessionContext()
private set

Expand Down Expand Up @@ -251,18 +249,11 @@ class CodeWhispererPopupManager {

// Don't want to block or throw any kinds of exceptions here if it can continue to provide suggestions
fun dontClosePopupAndRun(runnable: () -> Unit) {
if (allowEditsDuringSuggestionPreview.tryAcquire()) {
try {
runnable()
} finally {
try {
allowEditsDuringSuggestionPreview.release()
} catch (e: Exception) {
LOG.error(e) { "Failed to release allowEditsDuringSuggestionPreview semaphore" }
}
}
} else {
LOG.error { "Failed to acquire allowEditsDuringSuggestionPreview semaphore" }
try {
allowTypingDuringSuggestionPreview = true
runnable()
} finally {
allowTypingDuringSuggestionPreview = false
}
}

Expand Down Expand Up @@ -511,7 +502,7 @@ class CodeWhispererPopupManager {
val editor = states.requestContext.editor
val codewhispererSelectionListener: SelectionListener = object : SelectionListener {
override fun selectionChanged(event: SelectionEvent) {
if (allowEditsDuringSuggestionPreview.availablePermits == MAX_EDIT_SOURCE_DURING_SUGGESTION_PREVIEW) {
if (!allowTypingDuringSuggestionPreview && !allowIntelliSenseDuringSuggestionPreview) {
cancelPopup(states.popup)
}
super.selectionChanged(event)
Expand All @@ -527,7 +518,7 @@ class CodeWhispererPopupManager {
if (!delete) return
if (editor.caretModel.offset == event.offset) {
changeStates(states, 0)
} else if (allowEditsDuringSuggestionPreview.availablePermits == MAX_EDIT_SOURCE_DURING_SUGGESTION_PREVIEW) {
} else if (!allowTypingDuringSuggestionPreview && !allowIntelliSenseDuringSuggestionPreview) {
cancelPopup(states.popup)
}
}
Expand All @@ -536,7 +527,7 @@ class CodeWhispererPopupManager {

val codewhispererCaretListener: CaretListener = object : CaretListener {
override fun caretPositionChanged(event: CaretEvent) {
if (allowEditsDuringSuggestionPreview.availablePermits == MAX_EDIT_SOURCE_DURING_SUGGESTION_PREVIEW) {
if (!allowTypingDuringSuggestionPreview && !allowIntelliSenseDuringSuggestionPreview) {
cancelPopup(states.popup)
}
super.caretPositionChanged(event)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ import com.intellij.codeInsight.lookup.LookupEvent
import com.intellij.codeInsight.lookup.LookupListener
import com.intellij.codeInsight.lookup.LookupManagerListener
import com.intellij.codeInsight.lookup.impl.LookupImpl
import software.aws.toolkits.core.utils.error
import com.intellij.openapi.util.Disposer
import software.aws.toolkits.core.utils.getLogger
import software.aws.toolkits.jetbrains.services.codewhisperer.model.InvocationContext
import software.aws.toolkits.jetbrains.services.codewhisperer.popup.CodeWhispererPopupManager
import software.aws.toolkits.jetbrains.services.codewhisperer.popup.listeners.CodeWhispererPopupIntelliSenseAcceptListener.Companion.LOG
import software.aws.toolkits.jetbrains.services.codewhisperer.service.CodeWhispererInvocationStatus

class CodeWhispererPopupIntelliSenseAcceptListener(private val states: InvocationContext) : LookupManagerListener {
Expand All @@ -28,10 +27,8 @@ class CodeWhispererPopupIntelliSenseAcceptListener(private val states: Invocatio
}

fun addIntelliSenseAcceptListener(lookup: Lookup, states: InvocationContext) {
if (!CodeWhispererPopupManager.getInstance().allowEditsDuringSuggestionPreview.tryAcquire()) {
LOG.error { "Failed to acquire allowEditsDuringSuggestionPreview semaphore" }
}
lookup.addLookupListener(object : LookupListener {
CodeWhispererPopupManager.getInstance().allowIntelliSenseDuringSuggestionPreview = true
val listener = object : LookupListener {
override fun itemSelected(event: LookupEvent) {
if (!CodeWhispererInvocationStatus.getInstance().isDisplaySessionActive() ||
!(event.lookup as LookupImpl).isShown
Expand All @@ -52,11 +49,12 @@ fun addIntelliSenseAcceptListener(lookup: Lookup, states: InvocationContext) {

private fun cleanup() {
lookup.removeLookupListener(this)
try {
CodeWhispererPopupManager.getInstance().allowEditsDuringSuggestionPreview.release()
} catch (e: Exception) {
LOG.error(e) { "Failed to release allowEditsDuringSuggestionPreview semaphore" }
}
CodeWhispererPopupManager.getInstance().allowIntelliSenseDuringSuggestionPreview = false
}
})
}
lookup.addLookupListener(listener)
Disposer.register(states) {
lookup.removeLookupListener(listener)
CodeWhispererPopupManager.getInstance().allowIntelliSenseDuringSuggestionPreview = false
}
}

0 comments on commit e3cfc8c

Please sign in to comment.