-
-
Notifications
You must be signed in to change notification settings - Fork 769
Fix JAB freeze when navigating in JetBrains IDEs #19934
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
base: master
Are you sure you want to change the base?
Changes from all commits
6f4a189
485c368
d4ef0bb
ee21363
e2638e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| # A part of NonVisual Desktop Access (NVDA) | ||
| # Copyright (C) 2007-2025 NV Access Limited, Peter Vágner, Renaud Paquay, Babbage B.V. | ||
| # Copyright (C) 2007-2026 NV Access Limited, Peter Vágner, Renaud Paquay, Babbage B.V. | ||
| # This file is covered by the GNU General Public License. | ||
| # See the file COPYING for more details. | ||
|
|
||
|
|
@@ -548,7 +548,34 @@ def _fixBridgeFuncs(): | |
|
|
||
|
|
||
| def internalQueueFunction(func, *args, **kwargs): | ||
| internalFunctionQueue.put_nowait((func, args, kwargs)) | ||
| """Queue a function for execution on the main thread. | ||
|
|
||
| When the queue is full, the oldest event is evicted to make room. | ||
| JOBJECT64 handles owned by evicted events are released to avoid leaks. | ||
|
|
||
| .. note:: | ||
| All queued handler functions except :func:`enterJavaWindow_helper` | ||
| follow the convention ``(vmID, accContext, ...)``, where ``args[1]`` | ||
| is a JOBJECT64 handle. If a new handler with a different signature | ||
| is added, the eviction cleanup below must be updated. | ||
| """ | ||
| try: | ||
| internalFunctionQueue.put_nowait((func, args, kwargs)) | ||
| except queue.Full: | ||
| try: | ||
| evictedFunc, evictedArgs, _evictedKwargs = internalFunctionQueue.get_nowait() | ||
| # Release the JOBJECT64 accContext handle from the evicted event. | ||
| # All handlers except enterJavaWindow_helper have args[1] as accContext. | ||
| if evictedFunc is not enterJavaWindow_helper and len(evictedArgs) >= 2: | ||
| bridgeDll.releaseJavaObject(evictedArgs[0], evictedArgs[1]) | ||
| except queue.Empty: | ||
| pass | ||
| try: | ||
| internalFunctionQueue.put_nowait((func, args, kwargs)) | ||
| except queue.Full: | ||
| log.debugWarning("JAB internal function queue full, failed to re-queue after eviction") | ||
| return | ||
| log.debugWarning("JAB internal function queue full, evicted oldest event") | ||
| core.requestPump() | ||
|
Comment on lines
550
to
579
|
||
|
|
||
|
|
||
|
|
@@ -959,6 +986,8 @@ def internal_event_activeDescendantChange(vmID, event, source, oldDescendant, ne | |
| sourceContext = JABContext(hwnd=hwnd, vmID=vmID, accContext=source) | ||
| if internal_hasFocus(sourceContext): | ||
| internalQueueFunction(event_gainFocus, vmID, newDescendant, hwnd) | ||
| else: | ||
| bridgeDll.releaseJavaObject(vmID, newDescendant) | ||
| for accContext in [event, oldDescendant]: | ||
| bridgeDll.releaseJavaObject(vmID, accContext) | ||
|
|
||
|
|
@@ -972,8 +1001,13 @@ def internal_hasFocus(sourceContext): | |
|
|
||
|
|
||
| @AccessBridge_PropertyNameChangeFP | ||
| def event_nameChange(vmID, event, source, oldVal, newVal): | ||
| jabContext = JABContext(vmID=vmID, accContext=source) | ||
| def internal_event_nameChange(vmID, event, source, oldVal, newVal): | ||
| internalQueueFunction(event_nameChange, vmID, source) | ||
| bridgeDll.releaseJavaObject(vmID, event) | ||
|
|
||
|
|
||
| def event_nameChange(vmID, accContext): | ||
| jabContext = JABContext(vmID=vmID, accContext=accContext) | ||
| if jabContext.hwnd: | ||
| focus = api.getFocusObject() | ||
| obj = ( | ||
|
|
@@ -985,12 +1019,16 @@ def event_nameChange(vmID, event, source, oldVal, newVal): | |
| eventHandler.queueEvent("nameChange", obj) | ||
| else: | ||
| log.debugWarning("Unable to obtain window handle for accessible context") | ||
| bridgeDll.releaseJavaObject(vmID, event) | ||
|
|
||
|
|
||
| @AccessBridge_PropertyDescriptionChangeFP | ||
| def event_descriptionChange(vmID, event, source, oldVal, newVal): | ||
| jabContext = JABContext(vmID=vmID, accContext=source) | ||
| def internal_event_descriptionChange(vmID, event, source, oldVal, newVal): | ||
| internalQueueFunction(event_descriptionChange, vmID, source) | ||
| bridgeDll.releaseJavaObject(vmID, event) | ||
|
|
||
|
|
||
| def event_descriptionChange(vmID, accContext): | ||
| jabContext = JABContext(vmID=vmID, accContext=accContext) | ||
| if jabContext.hwnd: | ||
| focus = api.getFocusObject() | ||
| obj = ( | ||
|
|
@@ -1002,12 +1040,16 @@ def event_descriptionChange(vmID, event, source, oldVal, newVal): | |
| eventHandler.queueEvent("descriptionChange", obj) | ||
| else: | ||
| log.debugWarning("Unable to obtain window handle for accessible context") | ||
| bridgeDll.releaseJavaObject(vmID, event) | ||
|
|
||
|
|
||
| @AccessBridge_PropertyValueChangeFP | ||
| def event_valueChange(vmID, event, source, oldVal, newVal): | ||
| jabContext = JABContext(vmID=vmID, accContext=source) | ||
| def internal_event_valueChange(vmID, event, source, oldVal, newVal): | ||
| internalQueueFunction(event_valueChange, vmID, source) | ||
| bridgeDll.releaseJavaObject(vmID, event) | ||
|
|
||
|
|
||
| def event_valueChange(vmID, accContext): | ||
| jabContext = JABContext(vmID=vmID, accContext=accContext) | ||
| if jabContext.hwnd: | ||
| focus = api.getFocusObject() | ||
| obj = ( | ||
|
|
@@ -1019,7 +1061,6 @@ def event_valueChange(vmID, event, source, oldVal, newVal): | |
| eventHandler.queueEvent("valueChange", obj) | ||
| else: | ||
| log.debugWarning("Unable to obtain window handle for accessible context") | ||
| bridgeDll.releaseJavaObject(vmID, event) | ||
|
|
||
|
|
||
| @AccessBridge_PropertyStateChangeFP | ||
|
|
@@ -1155,9 +1196,9 @@ def initialize(): | |
| # Register java events | ||
| bridgeDll.setFocusGainedFP(internal_event_focusGained) | ||
| bridgeDll.setPropertyActiveDescendentChangeFP(internal_event_activeDescendantChange) | ||
| bridgeDll.setPropertyNameChangeFP(event_nameChange) | ||
| bridgeDll.setPropertyDescriptionChangeFP(event_descriptionChange) | ||
| bridgeDll.setPropertyValueChangeFP(event_valueChange) | ||
| bridgeDll.setPropertyNameChangeFP(internal_event_nameChange) | ||
| bridgeDll.setPropertyDescriptionChangeFP(internal_event_descriptionChange) | ||
| bridgeDll.setPropertyValueChangeFP(internal_event_valueChange) | ||
| bridgeDll.setPropertyStateChangeFP(internal_event_stateChange) | ||
| bridgeDll.setPropertyCaretChangeFP(internal_event_caretChange) | ||
| isRunning = True | ||
|
|
@@ -1174,6 +1215,9 @@ def terminate(): | |
| return | ||
| bridgeDll.setFocusGainedFP(None) | ||
| bridgeDll.setPropertyActiveDescendentChangeFP(None) | ||
| bridgeDll.setPropertyNameChangeFP(None) | ||
| bridgeDll.setPropertyDescriptionChangeFP(None) | ||
| bridgeDll.setPropertyValueChangeFP(None) | ||
| bridgeDll.setPropertyStateChangeFP(None) | ||
| bridgeDll.setPropertyCaretChangeFP(None) | ||
| h = bridgeDll._handle | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| # A part of NonVisual Desktop Access (NVDA) | ||
| # Copyright (C) 2006-2025 NV Access Limited, Leonard de Ruijter, Joseph Lee, Renaud Paquay, pvagner, hwf1324 | ||
| # Copyright (C) 2006-2026 NV Access Limited, Leonard de Ruijter, Joseph Lee, Renaud Paquay, pvagner, hwf1324 | ||
| # This file is covered by the GNU General Public License. | ||
| # See the file COPYING for more details. | ||
|
|
||
|
|
@@ -248,13 +248,44 @@ def findOverlayClasses(self, clsList: list[NVDAObject]) -> None: | |
| clsList.append(ComboBox) | ||
| elif role == "table": | ||
| clsList.append(Table) | ||
| elif self.parent and isinstance(self.parent, Table) and self.parent._jabTableInfo: | ||
| elif self._hasTableParent(): | ||
| clsList.append(TableCell) | ||
| elif role == "progress bar": | ||
| clsList.append(ProgressBar) | ||
|
|
||
| clsList.append(JAB) | ||
|
|
||
| def _hasTableParent(self) -> bool: | ||
| """Lightweight check if the immediate parent is a Table. | ||
|
|
||
| On the fast path (parent already cached), returns immediately | ||
| without any bridge calls. On the first call, checks the parent's | ||
| role via a lightweight bridge call and only creates a full NVDAObject | ||
| when the role is "table" (which involves additional bridge calls). | ||
| """ | ||
| if hasattr(self, "_parent"): | ||
| parent = self._parent | ||
| return parent is not None and isinstance(parent, Table) and parent._jabTableInfo | ||
|
Comment on lines
+258
to
+268
|
||
| parentContext = self.jabContext.getAccessibleParentFromContext() | ||
| if not parentContext: | ||
| return False | ||
| try: | ||
| parentInfo = parentContext.getAccessibleContextInfo() | ||
| except RuntimeError: | ||
| log.debugWarning("Could not get accessible context info for parent", exc_info=True) | ||
| return False | ||
| if parentInfo.role_en_US != "table": | ||
| return False | ||
| if self.indexInParent is None: | ||
| # Without indexInParent we cannot construct a valid JAB parent; | ||
| # _get_parent would also fall back to the Window ancestor here. | ||
| return False | ||
| # Parent is a table — create the full parent object reusing the context | ||
| # we already hold, so _get_parent doesn't make a redundant bridge call. | ||
| self._parent = JAB(jabContext=parentContext) | ||
| parent = self._parent | ||
| return parent is not None and isinstance(parent, Table) and parent._jabTableInfo | ||
|
|
||
| @classmethod | ||
| def kwargsFromSuper(cls, kwargs, relation: str | None = None) -> bool: | ||
| jabContext = None | ||
|
|
||
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.
In the overflow path, if
put_nowaitstill raisesqueue.Fullafter evicting one item, the function returns without releasing the JOBJECT64 handle owned by the new (dropped) event. Since queued handlers generally treatargs[1]as the ownedaccContextreference, this can leak JNI refs under contention/multiple producer threads. Consider releasing the dropped event’s handle on this failure path (excludingenterJavaWindow_helper), similar to the eviction cleanup.