From 1ae487610f71f189ac10d54600c0694bb549ecb1 Mon Sep 17 00:00:00 2001 From: haha Date: Wed, 24 Jun 2026 22:28:25 +0800 Subject: [PATCH 1/2] fix(windows-ime): eliminate thread deadlock causing AppHang in host processes Replace SendMessageTimeoutW with PostMessage + WaitForMultipleObjects in SubmitTextFromPipe. The old code caused a deadlock when Deactivate() (called by TSF on the owner thread) blocked on thread_.join() while the pipe thread was blocked on SendMessageTimeoutW waiting for the owner thread's message pump. The fix introduces a shutdown_event that Stop() signals before joining, so the pipe thread can be unblocked from its wait even if the owner thread is blocked. This prevents the 2-second deadlock that caused AppHangB1 in Explorer, Chrome, Clash Verge, and other host processes. Fixes: #665 --- .../app/windows-ime/src/ipc_client.cpp | 24 +++++++- openless-all/app/windows-ime/src/ipc_client.h | 1 + .../app/windows-ime/src/text_service.cpp | 55 +++++++++++++++---- .../app/windows-ime/src/text_service.h | 3 +- 4 files changed, 69 insertions(+), 14 deletions(-) diff --git a/openless-all/app/windows-ime/src/ipc_client.cpp b/openless-all/app/windows-ime/src/ipc_client.cpp index 55c42d15..91767008 100644 --- a/openless-all/app/windows-ime/src/ipc_client.cpp +++ b/openless-all/app/windows-ime/src/ipc_client.cpp @@ -336,10 +336,15 @@ std::wstring EscapeJsonString(const std::wstring& value) { } // namespace -OpenLessPipeServer::OpenLessPipeServer() = default; +OpenLessPipeServer::OpenLessPipeServer() + : shutdown_event_(CreateEventW(nullptr, TRUE, FALSE, nullptr)) {} OpenLessPipeServer::~OpenLessPipeServer() { Stop(); + if (shutdown_event_ != nullptr) { + CloseHandle(shutdown_event_); + shutdown_event_ = nullptr; + } } void OpenLessPipeServer::Start(OpenLessTextService* service) { @@ -348,6 +353,11 @@ void OpenLessPipeServer::Start(OpenLessTextService* service) { } stop_requested_.store(false); + + if (shutdown_event_ != nullptr) { + ResetEvent(shutdown_event_); + } + pipe_name_ = PipeNameForCurrentThread(); service_ = service; service_->AddRef(); @@ -356,12 +366,21 @@ void OpenLessPipeServer::Start(OpenLessTextService* service) { void OpenLessPipeServer::Stop() { stop_requested_.store(true); + + if (shutdown_event_ != nullptr) { + SetEvent(shutdown_event_); + } + if (thread_.joinable()) { CancelSynchronousIo(thread_.native_handle()); WakePipe(); thread_.join(); } + if (shutdown_event_ != nullptr) { + ResetEvent(shutdown_event_); + } + if (service_ != nullptr) { service_->Release(); service_ = nullptr; @@ -458,7 +477,8 @@ void OpenLessPipeServer::HandleSubmitLine(HANDLE pipe, const std::string& line) } const HRESULT hr = - service_->SubmitTextFromPipe(message.session_id, message.text); + service_->SubmitTextFromPipe(message.session_id, message.text, + shutdown_event_); if (SUCCEEDED(hr)) { WriteResult(pipe, message.session_id, L"committed", nullptr); } else { diff --git a/openless-all/app/windows-ime/src/ipc_client.h b/openless-all/app/windows-ime/src/ipc_client.h index 909be569..e6d10646 100644 --- a/openless-all/app/windows-ime/src/ipc_client.h +++ b/openless-all/app/windows-ime/src/ipc_client.h @@ -32,6 +32,7 @@ class OpenLessPipeServer { std::thread thread_; std::mutex pipe_mutex_; HANDLE pipe_handle_ = INVALID_HANDLE_VALUE; + HANDLE shutdown_event_ = nullptr; std::wstring pipe_name_; OpenLessTextService* service_ = nullptr; }; diff --git a/openless-all/app/windows-ime/src/text_service.cpp b/openless-all/app/windows-ime/src/text_service.cpp index f303eac1..3eb0c800 100644 --- a/openless-all/app/windows-ime/src/text_service.cpp +++ b/openless-all/app/windows-ime/src/text_service.cpp @@ -20,6 +20,7 @@ struct SubmitTextRequest { std::shared_ptr async_completion; bool wait_for_async_completion = false; HRESULT result = E_UNEXPECTED; + HANDLE completion_event = nullptr; }; HRESULT WaitForAsyncEditCompletion( @@ -133,7 +134,8 @@ STDMETHODIMP OpenLessTextService::Deactivate() { HRESULT OpenLessTextService::SubmitTextFromPipe( const std::wstring& session_id, - const std::wstring& text) { + const std::wstring& text, + HANDLE shutdown_event) { if (GetCurrentThreadId() == owner_thread_id_) { return CommitTextOnOwnerThread(session_id, text, nullptr, nullptr); } @@ -145,21 +147,49 @@ HRESULT OpenLessTextService::SubmitTextFromPipe( SubmitTextRequest request; request.session_id = &session_id; request.text = &text; - DWORD_PTR message_result = 0; - const LRESULT sent = SendMessageTimeoutW( - message_window_, kSubmitTextMessage, 0, - reinterpret_cast(&request), SMTO_ABORTIFHUNG, - kSubmitTextTimeoutMs, &message_result); - if (sent == 0) { + request.completion_event = + CreateEventW(nullptr, TRUE, FALSE, nullptr); + if (request.completion_event == nullptr) { + return HRESULT_FROM_WIN32(GetLastError()); + } + + if (!PostMessageW(message_window_, kSubmitTextMessage, 0, + reinterpret_cast(&request))) { const DWORD error = GetLastError(); - return HRESULT_FROM_WIN32(error != ERROR_SUCCESS ? error : ERROR_TIMEOUT); + CloseHandle(request.completion_event); + return HRESULT_FROM_WIN32(error); + } + + HANDLE wait_handles[2] = {}; + DWORD wait_count = 0; + + wait_handles[wait_count] = request.completion_event; + ++wait_count; + + if (shutdown_event != nullptr) { + wait_handles[wait_count] = shutdown_event; + ++wait_count; } - if (request.wait_for_async_completion) { - return WaitForAsyncEditCompletion(request.async_completion); + const DWORD wait_result = WaitForMultipleObjects( + wait_count, wait_handles, FALSE, kSubmitTextTimeoutMs); + + HRESULT result; + if (wait_result == WAIT_OBJECT_0) { + result = request.result; + if (request.wait_for_async_completion) { + result = WaitForAsyncEditCompletion(request.async_completion); + } + } else if (wait_count > 1 && wait_result == WAIT_OBJECT_0 + 1) { + result = HRESULT_FROM_WIN32(ERROR_CANCELLED); + } else if (wait_result == WAIT_TIMEOUT) { + result = HRESULT_FROM_WIN32(ERROR_TIMEOUT); + } else { + result = HRESULT_FROM_WIN32(GetLastError()); } - return request.result; + CloseHandle(request.completion_event); + return result; } HRESULT OpenLessTextService::StartIpcServer() { @@ -325,6 +355,9 @@ LRESULT CALLBACK OpenLessTextService::MessageWindowProc(HWND window, request->result = service->CommitTextOnOwnerThread( *request->session_id, *request->text, &request->async_completion, &request->wait_for_async_completion); + if (request->completion_event != nullptr) { + SetEvent(request->completion_event); + } return 1; } diff --git a/openless-all/app/windows-ime/src/text_service.h b/openless-all/app/windows-ime/src/text_service.h index c69190d0..b47323cb 100644 --- a/openless-all/app/windows-ime/src/text_service.h +++ b/openless-all/app/windows-ime/src/text_service.h @@ -27,7 +27,8 @@ class OpenLessTextService final : public ITfTextInputProcessorEx { DWORD flags) override; HRESULT SubmitTextFromPipe(const std::wstring& session_id, - const std::wstring& text); + const std::wstring& text, + HANDLE shutdown_event = nullptr); private: HRESULT StartIpcServer(); From 03c7ef7a912b8a9eca770fa6cbe690ded466a197 Mon Sep 17 00:00:00 2001 From: haha Date: Thu, 25 Jun 2026 15:12:25 +0800 Subject: [PATCH 2/2] fix(windows-ime): prevent use-after-free on timeout / shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Heap-allocate SubmitTextRequest with atomic ref-counting (ref_count=2: pipe thread + queued message). Store string copies instead of pointers so the request's data survives HandleSubmitLine's return when the wait times out. Drain pending kSubmitTextMessage in DestroyMessageWindow to release any reference left by a shutdown-race message. The pipe thread releases its reference after WaitForMultipleObjects; the message thread releases its reference after processing (or the PeekMessage drain during teardown). Whichever releases last closes the event handle and deletes the request — no dangling pointer, no stale SetEvent. --- .../app/windows-ime/src/text_service.cpp | 72 ++++++++++++++----- 1 file changed, 55 insertions(+), 17 deletions(-) diff --git a/openless-all/app/windows-ime/src/text_service.cpp b/openless-all/app/windows-ime/src/text_service.cpp index 3eb0c800..af3bee49 100644 --- a/openless-all/app/windows-ime/src/text_service.cpp +++ b/openless-all/app/windows-ime/src/text_service.cpp @@ -1,5 +1,6 @@ #include "text_service.h" +#include #include #include @@ -15,12 +16,25 @@ constexpr UINT kSubmitTextMessage = WM_APP + 1; constexpr UINT kSubmitTextTimeoutMs = 2000; struct SubmitTextRequest { - const std::wstring* session_id = nullptr; - const std::wstring* text = nullptr; + // Owned copies — not pointers into another thread's stack. + std::wstring session_id; + std::wstring text; std::shared_ptr async_completion; bool wait_for_async_completion = false; HRESULT result = E_UNEXPECTED; HANDLE completion_event = nullptr; + // Two owners at creation: pipe thread (waiting on completion_event) and + // the queued window message. Each owner calls Release() when done; the + // last one to Release() closes the event and deletes the request. + std::atomic ref_count{2}; + void Release() { + if (ref_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { + if (completion_event != nullptr) { + CloseHandle(completion_event); + } + delete this; + } + } }; HRESULT WaitForAsyncEditCompletion( @@ -144,26 +158,32 @@ HRESULT OpenLessTextService::SubmitTextFromPipe( return E_UNEXPECTED; } - SubmitTextRequest request; - request.session_id = &session_id; - request.text = &text; - request.completion_event = + auto* request = new (std::nothrow) SubmitTextRequest; + if (request == nullptr) { + return E_OUTOFMEMORY; + } + + request->session_id = session_id; + request->text = text; + request->completion_event = CreateEventW(nullptr, TRUE, FALSE, nullptr); - if (request.completion_event == nullptr) { + if (request->completion_event == nullptr) { + delete request; return HRESULT_FROM_WIN32(GetLastError()); } if (!PostMessageW(message_window_, kSubmitTextMessage, 0, - reinterpret_cast(&request))) { + reinterpret_cast(request))) { const DWORD error = GetLastError(); - CloseHandle(request.completion_event); + CloseHandle(request->completion_event); + delete request; return HRESULT_FROM_WIN32(error); } HANDLE wait_handles[2] = {}; DWORD wait_count = 0; - wait_handles[wait_count] = request.completion_event; + wait_handles[wait_count] = request->completion_event; ++wait_count; if (shutdown_event != nullptr) { @@ -176,9 +196,9 @@ HRESULT OpenLessTextService::SubmitTextFromPipe( HRESULT result; if (wait_result == WAIT_OBJECT_0) { - result = request.result; - if (request.wait_for_async_completion) { - result = WaitForAsyncEditCompletion(request.async_completion); + result = request->result; + if (request->wait_for_async_completion) { + result = WaitForAsyncEditCompletion(request->async_completion); } } else if (wait_count > 1 && wait_result == WAIT_OBJECT_0 + 1) { result = HRESULT_FROM_WIN32(ERROR_CANCELLED); @@ -188,7 +208,9 @@ HRESULT OpenLessTextService::SubmitTextFromPipe( result = HRESULT_FROM_WIN32(GetLastError()); } - CloseHandle(request.completion_event); + // Release the pipe-thread reference. The message-thread reference, if + // still outstanding (timeout / shutdown paths), will clean up later. + request->Release(); return result; } @@ -230,6 +252,18 @@ HRESULT OpenLessTextService::EnsureMessageWindow() { void OpenLessTextService::DestroyMessageWindow() { if (message_window_ != nullptr) { + // Drain any kSubmitTextMessage still in the queue so the associated + // SubmitTextRequest references are released before the window is gone. + // These are left over when SubmitTextFromPipe exited via timeout or + // shutdown before the owner thread could dispatch the message. + MSG msg; + while (PeekMessageW(&msg, message_window_, kSubmitTextMessage, + kSubmitTextMessage, PM_REMOVE)) { + auto* request = reinterpret_cast(msg.lParam); + if (request != nullptr) { + request->Release(); + } + } DestroyWindow(message_window_); message_window_ = nullptr; } @@ -347,17 +381,21 @@ LRESULT CALLBACK OpenLessTextService::MessageWindowProc(HWND window, GetWindowLongPtrW(window, GWLP_USERDATA)); if (message == kSubmitTextMessage && service != nullptr) { auto* request = reinterpret_cast(lparam); - if (request == nullptr || request->session_id == nullptr || - request->text == nullptr) { + if (request == nullptr || request->session_id.empty() || + request->text.empty()) { + if (request != nullptr) { + request->Release(); + } return 0; } request->result = service->CommitTextOnOwnerThread( - *request->session_id, *request->text, &request->async_completion, + request->session_id, request->text, &request->async_completion, &request->wait_for_async_completion); if (request->completion_event != nullptr) { SetEvent(request->completion_event); } + request->Release(); return 1; }