Skip to content

Commit 3379207

Browse files
fix(embed): prevent hung embeds from freezing host
When an embed's UI thread hangs, shared input queues made NotepadAI wait on every click and activation. Drop ownership on probe timeout, run post-release Win32 work off the GUI thread, and track pending geometry so the host stays usable until the foreign app answers again.
1 parent 480b362 commit 3379207

9 files changed

Lines changed: 464 additions & 53 deletions

‎src/EmbeddedWindowWin32.cpp‎

Lines changed: 77 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,23 @@
1111
#include <Windows.h>
1212
#include <commctrl.h>
1313

14+
#include <system_error>
15+
#include <thread>
16+
1417
namespace {
18+
// Style, placement and show changes are synchronous sends to the window's
19+
// thread. After a release nothing waits for them, so they run here: a hung or
20+
// busy app must not hold the GUI thread while its tab closes.
21+
template<typename Steps>
22+
void runOffGuiThread(const Steps &steps)
23+
{
24+
try {
25+
std::thread(steps).detach();
26+
} catch (const std::system_error &) {
27+
steps();
28+
}
29+
}
30+
1531
constexpr wchar_t EmbedPropertyName[] = L"NotepadAI.EmbeddedWindow.Token";
1632
constexpr UINT_PTR kCaptionGuardId = 0x4e414945;
1733

@@ -250,36 +266,42 @@ bool restoreExact(const NativeWindowState &s, quintptr token)
250266
// tab is safe to reap. It is the SOLE gate on the return value: a cosmetic
251267
// failure below must not report the window as still embedded, or the
252268
// controller vetoes the close and the host takes the window right back.
269+
// Already unowned while it was hung: setting the same owner again fails.
253270
HWND originalOwner = reinterpret_cast<HWND>(s.originalOwner);
254-
if (!ensureTopLevel(hwnd) || !setLong(hwnd, GWLP_HWNDPARENT, s.originalOwner)
271+
if (!ensureTopLevel(hwnd)
272+
|| (GetWindow(hwnd, GW_OWNER) != originalOwner && !setLong(hwnd, GWLP_HWNDPARENT, s.originalOwner))
255273
|| GetWindow(hwnd, GW_OWNER) != originalOwner) {
256274
qWarning("EmbeddedWindow: restoreExact could not drop ownership (err=%lu)", GetLastError());
257275
return false;
258276
}
259277

260-
// Cosmetic best-effort restoration; each is gated on the token so a window
261-
// recycled after detach is never touched, but none affect the result.
262-
if (sameWindow(s, token))
263-
setLong(hwnd, GWL_STYLE, s.originalStyle);
264-
if (sameWindow(s, token))
265-
setLong(hwnd, GWL_EXSTYLE, s.originalExStyle);
266-
if (sameWindow(s, token)) {
267-
HWND z = (s.originalExStyle & WS_EX_TOPMOST) ? HWND_TOPMOST : HWND_NOTOPMOST;
268-
SetWindowPos(hwnd, z, s.rectLeft, s.rectTop, s.rectRight - s.rectLeft,
269-
s.rectBottom - s.rectTop, SWP_NOACTIVATE | SWP_FRAMECHANGED);
270-
}
271-
if (s.placementValid && sameWindow(s, token)) {
272-
WINDOWPLACEMENT placement = placementFrom(s);
273-
SetWindowPlacement(hwnd, &placement);
274-
}
275-
if (sameWindow(s, token)) {
276-
const int show = s.placementValid ? static_cast<int>(s.placementShowCmd) : SW_SHOWNOACTIVATE;
277-
ShowWindowAsync(hwnd, s.originallyVisible ? (show == SW_HIDE ? SW_SHOWNOACTIVATE : show) : SW_HIDE);
278-
}
279-
// Untag is best-effort: if the window was recycled the new owner keeps its
280-
// property; our host no longer tracks it so no re-grab can occur.
281-
if (sameWindow(s, token))
282-
RemovePropW(hwnd, EmbedPropertyName);
278+
// Cosmetic best-effort restoration, off the GUI thread; each is gated on the
279+
// token so a window recycled after detach is never touched, but none affect
280+
// the result.
281+
runOffGuiThread([s, token]() {
282+
HWND hwnd = reinterpret_cast<HWND>(s.handle);
283+
if (sameWindow(s, token))
284+
setLong(hwnd, GWL_STYLE, s.originalStyle);
285+
if (sameWindow(s, token))
286+
setLong(hwnd, GWL_EXSTYLE, s.originalExStyle);
287+
if (sameWindow(s, token)) {
288+
HWND z = (s.originalExStyle & WS_EX_TOPMOST) ? HWND_TOPMOST : HWND_NOTOPMOST;
289+
SetWindowPos(hwnd, z, s.rectLeft, s.rectTop, s.rectRight - s.rectLeft,
290+
s.rectBottom - s.rectTop, SWP_NOACTIVATE | SWP_FRAMECHANGED);
291+
}
292+
if (s.placementValid && sameWindow(s, token)) {
293+
WINDOWPLACEMENT placement = placementFrom(s);
294+
SetWindowPlacement(hwnd, &placement);
295+
}
296+
if (sameWindow(s, token)) {
297+
const int show = s.placementValid ? static_cast<int>(s.placementShowCmd) : SW_SHOWNOACTIVATE;
298+
ShowWindowAsync(hwnd, s.originallyVisible ? (show == SW_HIDE ? SW_SHOWNOACTIVATE : show) : SW_HIDE);
299+
}
300+
// Untag is best-effort: if the window was recycled the new owner keeps
301+
// its property; our host no longer tracks it so no re-grab can occur.
302+
if (sameWindow(s, token))
303+
RemovePropW(hwnd, EmbedPropertyName);
304+
});
283305
return true;
284306
#else
285307
Q_UNUSED(s) Q_UNUSED(token)
@@ -301,24 +323,28 @@ bool failSafeDetach(const NativeWindowState &s, quintptr token)
301323
return false;
302324

303325
// The release-defining step first (see restoreExact).
304-
if (!ensureTopLevel(hwnd) || !setLong(hwnd, GWLP_HWNDPARENT, 0) || GetWindow(hwnd, GW_OWNER)) {
326+
if (!ensureTopLevel(hwnd) || (GetWindow(hwnd, GW_OWNER) && !setLong(hwnd, GWLP_HWNDPARENT, 0))
327+
|| GetWindow(hwnd, GW_OWNER)) {
305328
qWarning("EmbeddedWindow: failSafeDetach could not drop ownership (err=%lu)", GetLastError());
306329
return false;
307330
}
308331

309-
// Best-effort cosmetic cleanup, always token-gated.
310-
if (sameWindow(s, token))
311-
setLong(hwnd, GWL_STYLE, s.originalStyle & ~WS_CHILD);
312-
if (sameWindow(s, token))
313-
setLong(hwnd, GWL_EXSTYLE, s.originalExStyle);
314-
if (sameWindow(s, token)) {
315-
const UINT flags = SWP_NOACTIVATE | SWP_FRAMECHANGED
316-
| (s.originallyVisible ? SWP_SHOWWINDOW : SWP_HIDEWINDOW);
317-
SetWindowPos(hwnd, HWND_TOP, s.rectLeft, s.rectTop,
318-
s.rectRight - s.rectLeft, s.rectBottom - s.rectTop, flags);
319-
}
320-
if (sameWindow(s, token))
321-
RemovePropW(hwnd, EmbedPropertyName);
332+
// Best-effort cosmetic cleanup, off the GUI thread, always token-gated.
333+
runOffGuiThread([s, token]() {
334+
HWND hwnd = reinterpret_cast<HWND>(s.handle);
335+
if (sameWindow(s, token))
336+
setLong(hwnd, GWL_STYLE, s.originalStyle & ~WS_CHILD);
337+
if (sameWindow(s, token))
338+
setLong(hwnd, GWL_EXSTYLE, s.originalExStyle);
339+
if (sameWindow(s, token)) {
340+
const UINT flags = SWP_NOACTIVATE | SWP_FRAMECHANGED
341+
| (s.originallyVisible ? SWP_SHOWWINDOW : SWP_HIDEWINDOW);
342+
SetWindowPos(hwnd, HWND_TOP, s.rectLeft, s.rectTop,
343+
s.rectRight - s.rectLeft, s.rectBottom - s.rectTop, flags);
344+
}
345+
if (sameWindow(s, token))
346+
RemovePropW(hwnd, EmbedPropertyName);
347+
});
322348
return true;
323349
#else
324350
Q_UNUSED(s) Q_UNUSED(token)
@@ -367,6 +393,19 @@ GeometryCheck checkGeometry(quintptr targetHandle, quintptr hostHandle, quintptr
367393
#endif
368394
}
369395

396+
bool releaseOwnerWhileHung(quintptr targetHandle, quintptr token)
397+
{
398+
#ifdef Q_OS_WIN
399+
HWND target = reinterpret_cast<HWND>(targetHandle);
400+
return ownsWindow(targetHandle, token)
401+
&& (!GetWindow(target, GW_OWNER) || setLong(target, GWLP_HWNDPARENT, 0))
402+
&& !GetWindow(target, GW_OWNER);
403+
#else
404+
Q_UNUSED(targetHandle) Q_UNUSED(token)
405+
return false;
406+
#endif
407+
}
408+
370409
quintptr rootWindow(quintptr windowHandle)
371410
{
372411
#ifdef Q_OS_WIN

‎src/EmbeddedWindowWin32.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,12 @@ bool failSafeDetach(const NativeWindowState &state, quintptr token);
144144
GeometryCheck checkGeometry(quintptr targetHandle, quintptr hostHandle, quintptr token,
145145
EmbedGeometry *wanted);
146146

147+
// The embed's thread stopped processing messages: drop our ownership so the
148+
// two input queues are no longer shared (with them shared, NotepadAI's clicks
149+
// and activation wait behind the hung thread). Sends nothing to the window, so
150+
// it returns at once. The next checkGeometry takes ownership back.
151+
bool releaseOwnerWhileHung(quintptr targetHandle, quintptr token);
152+
147153
quintptr rootWindow(quintptr windowHandle);
148154
quintptr foregroundWindow();
149155
// True if `window` is (inside) an embedded foreign window owned by `topLevel`.

0 commit comments

Comments
 (0)