Skip to content

Commit

Permalink
Fix freeze when pressing enter to dismiss a dialog while focused in a…
Browse files Browse the repository at this point in the history
… text box in Mozilla applications.

In this case, the window dies before the message is sent, but the return value of PostMessage wasn't being checked. Therefore, we were waiting forever for an event that would never be set.
However, it's also possible that the thread might die after the message is sent but before it is handled. Therefore, switch to using SendMessageCallback, which calls the callback with a 0 result if the thread dies.
This also fixes a leak, since event handles in the previous code weren't being closed.
Fixes #6422.
  • Loading branch information
jcsteh committed Oct 5, 2016
1 parent 1692ac7 commit 657fb56
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 22 deletions.
4 changes: 2 additions & 2 deletions nvdaHelper/remote/IA2Support.cpp
Expand Up @@ -291,7 +291,7 @@ bool findContentDescendant(IAccessible2* pacc2, long what, long* descendantID, l

error_status_t nvdaInProcUtils_IA2Text_findContentDescendant(handle_t bindingHandle, const unsigned long windowHandle, long parentID, long what, long* descendantID, long* descendantOffset) {
HWND hwnd=(HWND)UlongToHandle(windowHandle);
auto func=[&] {
auto func=[&](void* data){
IAccessible* pacc=NULL;
VARIANT varChild;
AccessibleObjectFromEvent((HWND)hwnd,OBJID_CLIENT,parentID,&pacc,&varChild);
Expand All @@ -307,6 +307,6 @@ error_status_t nvdaInProcUtils_IA2Text_findContentDescendant(handle_t bindingHan
findContentDescendant(pacc2,what,descendantID,descendantOffset);
pacc2->Release();
};
execInWindow((HWND)hwnd,func);
execInWindow((HWND)hwnd,func,NULL);
return 0;
}
34 changes: 20 additions & 14 deletions nvdaHelper/remote/inProcess.cpp
Expand Up @@ -117,14 +117,6 @@ LRESULT CALLBACK inProcess_getMessageHook(int code, WPARAM wParam, LPARAM lParam
if(code<0||wParam==PM_NOREMOVE) {
return CallNextHookEx(0,code,wParam,lParam);
}
MSG* pmsg=(MSG*)lParam;
if(pmsg->message==wm_execInWindow) {
execInWindow_funcType* func=(execInWindow_funcType*)(pmsg->wParam);
if(func) (*func)();
// Signal completion to execInWindow.
SetEvent((HANDLE)pmsg->lParam);
return 0;
}
//Hookprocs may unregister or register hooks themselves, so we must copy the hookprocs before executing
windowsHookRegistry_t hookProcs=inProcess_registeredGetMessageWindowsHooks;
for(windowsHookRegistry_t::iterator i=hookProcs.begin();i!=hookProcs.end();++i) {
Expand All @@ -138,6 +130,13 @@ LRESULT CALLBACK inProcess_callWndProcHook(int code, WPARAM wParam,LPARAM lParam
if(code<0) {
return CallNextHookEx(0,code,wParam,lParam);
}
CWPSTRUCT* pcwp=(CWPSTRUCT*)lParam;
if(pcwp->message==wm_execInWindow) {
execInWindow_funcType* func=(execInWindow_funcType*)(pcwp->wParam);
void* data=(void*)(pcwp->lParam);
if(func) (*func)(data);
return 0;
}
//Hookprocs may unregister or register hooks themselves, so we must copy the hookprocs before executing
windowsHookRegistry_t hookProcs=inProcess_registeredCallWndProcWindowsHooks;
for(windowsHookRegistry_t::iterator i=hookProcs.begin();i!=hookProcs.end();++i) {
Expand All @@ -157,11 +156,18 @@ void CALLBACK inProcess_winEventCallback(HWINEVENTHOOK hookID, DWORD eventID, HW
}
}

void execInWindow(HWND hwnd, execInWindow_funcType func) {
void execInWindow(HWND hwnd, execInWindow_funcType func,void* data) {
// Using SendMessage here causes outgoing cross-process COM calls to fail with RPC_E_CANTCALLOUT_ININPUTSYNCCALL,
// which breaks us for Firefox multi-process. See Mozilla bug 1297549 comment 14.
// Use PostMessage instead.
HANDLE event=CreateEvent(NULL,TRUE,FALSE,NULL);
PostMessage(hwnd,wm_execInWindow,(WPARAM)&func,(LPARAM)event);
WaitForSingleObject(event,INFINITE);
// which breaks us for Firefox multi-process. See Mozilla bug 1297549 comments 14 and 18.
// Use SendMessageCallback instead.
SENDASYNCPROC callback = [] (HWND hwnd, UINT msg, ULONG_PTR data, LRESULT result) {
// Signal the waiting message loop to exit.
PostQuitMessage(0);
};
if (!SendMessageCallback(hwnd,wm_execInWindow,(WPARAM)&func,(LPARAM)data, callback, NULL))
return;
MSG msg;
// Wait in a message loop until the callback fires and sends WM_QUIT.
// We also abort the loop if WaitMessage fails for some reason.
while (GetMessage(&msg, NULL, WM_QUIT, WM_QUIT) > 0);
}
4 changes: 2 additions & 2 deletions nvdaHelper/remote/inProcess.h
Expand Up @@ -11,8 +11,8 @@ void inProcess_initialize();
void inProcess_terminate();

#include <functional>
typedef std::function<void()> execInWindow_funcType;
void execInWindow(HWND hwnd, execInWindow_funcType func);
typedef std::function<void(void*)> execInWindow_funcType;
void execInWindow(HWND hwnd, execInWindow_funcType func,void* data);


#endif
8 changes: 4 additions & 4 deletions nvdaHelper/vbufBase/backend.cpp
Expand Up @@ -34,12 +34,12 @@ void VBufBackend_t::initialize() {
int renderThreadID=GetWindowThreadProcessId((HWND)UlongToHandle(rootDocHandle),NULL);
LOG_DEBUG(L"render threadID "<<renderThreadID);
registerWindowsHook(WH_CALLWNDPROC,destroy_callWndProcHook);
auto func = [&] {
auto func = [&] (void* data) {
LOG_DEBUG(L"Calling renderThread_initialize on backend at "<<this);
this->renderThread_initialize();
};
LOG_DEBUG(L"Calling execInWindow");
execInWindow((HWND)UlongToHandle(rootDocHandle),func);
execInWindow((HWND)UlongToHandle(rootDocHandle),func, NULL);
LOG_DEBUG(L"execInWindow complete");
}

Expand Down Expand Up @@ -230,12 +230,12 @@ void VBufBackend_t::terminate() {
LOG_DEBUG(L"Render thread not terminated yet");
int renderThreadID=GetWindowThreadProcessId((HWND)UlongToHandle(rootDocHandle),NULL);
LOG_DEBUG(L"render threadID "<<renderThreadID);
auto func = [&] {
auto func = [&] (void* data) {
LOG_DEBUG(L"Calling renderThread_terminate on backend at "<<this);
this->renderThread_terminate();
};
LOG_DEBUG(L"Calling execInWindow");
execInWindow((HWND)UlongToHandle(rootDocHandle),func);
execInWindow((HWND)UlongToHandle(rootDocHandle),func, NULL);
LOG_DEBUG(L"execInWindow complete");
} else {
LOG_DEBUG(L"render thread already terminated");
Expand Down

0 comments on commit 657fb56

Please sign in to comment.