Skip to content

Actually terminate (clean up) the Microsoft UIA Abstraction remote operations library on NVDA exit.#16222

Merged
seanbudd merged 3 commits into
rcfrom
terminateUIARemote
Feb 25, 2024
Merged

Actually terminate (clean up) the Microsoft UIA Abstraction remote operations library on NVDA exit.#16222
seanbudd merged 3 commits into
rcfrom
terminateUIARemote

Conversation

@michaelDCurran

@michaelDCurran michaelDCurran commented Feb 24, 2024

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #16072

Summary of the issue:

As seen in a dump file provided by @irrah68 when debugging #16072 , NVDA can hang on exit while trying to destruct an instance of the CUIAutomation8 COMClass object:

Call stack
Details
win32u.dll!_NtUserMsgWaitForMultipleObjectsEx@20()	Unknown
user32.dll!MsgWaitForMultipleObjectsEx()	Unknown
combase.dll!CCliModalLoop::BlockFn(void * * ahEvent, unsigned long cEvents, unsigned long * lpdwSignaled) Line 2116	C++
combase.dll!ClassicSTAThreadWaitForHandles(unsigned long dwFlags, unsigned long dwTimeout, unsigned long cHandles, void * * pHandles, unsigned long * pdwIndex) Line 54	C++
combase.dll!CoWaitForMultipleHandles(unsigned long dwFlags, unsigned long dwTimeout, unsigned long cHandles, void * * pHandles, unsigned long * lpdwindex) Line 126	C++
combase.dll!CGIPTable::_RevokeInterfaceFromGlobal(unsigned long dwCookie, bool bPostedRevoke, bool waitIfBusy) Line 1010	C++
combase.dll!CGIPTable::RevokeInterfaceFromGlobal(unsigned long dwCookie) Line 935	C++
uiautomationcore.dll!ViewManagerEventTracker::Uninitialize(void)	Unknown
uiautomationcore.dll!ClientEventManager::~ClientEventManager(void)	Unknown
uiautomationcore.dll!CUIAutomation::~CUIAutomation(void)	Unknown
uiautomationcore.dll!ATL::CComObject::`scalar deleting destructor'(unsigned int)	Unknown
uiautomationcore.dll!ATL::CComObject::Release(void)	Unknown
[Inline Frame] UIARemote.dll!wil::com_ptr_t::{dtor}() Line 260	C++
[Inline Frame] UIARemote.dll!wil::object_without_destructor_on_shutdown>::{dtor}() Line 2209	C++
UIARemote.dll!UiaOperationAbstraction::`anonymous namespace'::`dynamic atexit destructor for 'g_automation''()	C++
ucrtbase.dll!(void)()	Unknown
ucrtbase.dll!__crt_seh_guarded_call::operator()<,(void) &,(void)>()	Unknown
ucrtbase.dll!__execute_onexit_table()	Unknown
UIARemote.dll!__scrt_dllmain_uninitialize_c() Line 398	C++
UIARemote.dll!dllmain_crt_process_detach(const bool is_terminating) Line 182	C++
In short, it looks as if the destruction of the CUIAutomation8 COMClass object held by the UIA abstraction Remote Operations library locks up as it is happening from a different thread from where it was created. It was originally created in NvDA's UIA MTA thread, but it is being automatically cleaned up by the CRT of NVDA's UIARemote.dll in NVDA's main thread. I'm not entirely sure why this is only a problem on @irrah68's machine, and why it is more likely to happen with the UIA Rate Limited event handler in #14888. Either way though, NVDA is not cleaning it up correctly.

Description of user facing changes

NVDA is less likely to hang on NvDA exit.

Description of development approach

  • Add a cleanup function to UIARemote.dll which calls the UIA abstraction remote operations library's cleanup function. This function frees all remote contexts, but most importantly, releases the CUIAutomation8 COMClass object which was passed in on initialization.
  • Add a terminate function to UIAHandler/remote.py which calls UIARemote.dll's cleanup function.
  • at the end of UIA's MTA thread in Python, after removing event handlers, finally call UIAHandler.remtoe.terminate, ensuring that the UIA abstraction remote ops library is correctly cleaned up in the UIA MTA thread, where it was originally initialized.

Testing strategy:

  • Restarted NVDA multiple times. Ensured the NvDA's log showed the appropriate log messages in UIARemote.dll's cleanup method.
  • @irrah68 has tested a try build of this pr based on master and has confirmed the issue has been fixed for them.

Known issues with pull request:

None known.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@michaelDCurran michaelDCurran requested a review from a team as a code owner February 24, 2024 12:36
@michaelDCurran michaelDCurran requested review from seanbudd and removed request for a team February 24, 2024 12:36
@CyrilleB79

Copy link
Copy Markdown
Contributor

@michaelDCurran please modify the issue number being fixed (twice) in the initial description. #16072 and not #16074.

@michaelDCurran

Copy link
Copy Markdown
Member Author

@CyrilleB79 oops, thanks. I've fixed this up now.

@michaelDCurran michaelDCurran added this to the 2023.3.4 milestone Feb 25, 2024
@seanbudd seanbudd changed the title Actually terminate (clean up) the Microsoft UIA Abstraction remote operations librry on NVDA exit. Actually terminate (clean up) the Microsoft UIA Abstraction remote operations library on NVDA exit. Feb 25, 2024
seanbudd
seanbudd previously approved these changes Feb 25, 2024
Comment thread nvdaHelper/UIARemote/UIARemote.cpp Outdated
Comment thread nvdaHelper/UIARemote/UIARemote.cpp Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants