From 696cfc5f78dce1890e347d9c503dd3585d652d0b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 03:32:12 +0000 Subject: [PATCH 1/6] Initial plan From aef3cc79588a002b18ad79f3a9e0c793a1e3dbe2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 03:45:30 +0000 Subject: [PATCH 2/6] Fix Windows canvas deadlock: make destroyFrame synchronous and fix WM_PAINT handler - RESTService/addon.cc: Make destroyFrame synchronous on Windows/macOS like renderFrame. The child window is created on the JS thread (via synchronous renderFrame), so destroyFrame must also run on the JS thread to avoid cross-thread SendMessageA calls in ~WindowInformation() that block the minsky thread while holding minskyCmdMutex, causing a deadlock when the JS thread tries to acquire minskyCmdMutex. - model/windowInformation.cc: Add ValidateRect in windowProc WM_PAINT handler when GWLP_USERDATA is 0 to prevent infinite WM_PAINT loop when window is being destroyed. - model/windowInformation.cc: Add SWP_NOACTIVATE flag to SetWindowPos in blit() to prevent unnecessary window activation messages on every paint, reducing message cascade issues with screen sharing tools like Zoom. Co-authored-by: highperformancecoder <3075825+highperformancecoder@users.noreply.github.com> --- RESTService/addon.cc | 9 +++++++-- model/windowInformation.cc | 4 +++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/RESTService/addon.cc b/RESTService/addon.cc index 5e4eb2f0a..c8af6c73e 100644 --- a/RESTService/addon.cc +++ b/RESTService/addon.cc @@ -219,8 +219,13 @@ namespace minsky return String::New(env, utf_to_utf(doCommand(command, arguments))); } #if defined(_WIN32) || defined(MAC_OSX_TK) - // renderFrame needs to be called synchronously, otherwise inexplicable hangs occur on Windows. - if (command.ends_with(".renderFrame")) + // renderFrame and destroyFrame need to be called synchronously on the JS thread. + // The child window is created on the JS thread (via synchronous renderFrame), so + // destroyFrame must also run on the JS thread to avoid cross-thread SendMessageA + // calls in ~WindowInformation() that block the minsky thread while holding + // minskyCmdMutex, causing a deadlock when the JS thread subsequently tries to + // acquire minskyCmdMutex. + if (command.ends_with(".renderFrame") || command.ends_with(".destroyFrame")) return String::New(env, utf_to_utf(doCommand(command, arguments))); #endif if (minskyCommands.size()>20) diff --git a/model/windowInformation.cc b/model/windowInformation.cc index 7b13419b9..a769659e4 100644 --- a/model/windowInformation.cc +++ b/model/windowInformation.cc @@ -82,7 +82,7 @@ namespace minsky HDC dc=BeginPaint(winfo.childWindowId, &ps); BitBlt(dc, x, y, width,height,winfo.hdcMem,x,y,SRCCOPY); EndPaint(winfo.childWindowId, &ps); - SetWindowPos(winfo.childWindowId,HWND_TOP,winfo.offsetLeft,winfo.offsetTop,winfo.childWidth,winfo.childHeight,0); + SetWindowPos(winfo.childWindowId,HWND_TOP,winfo.offsetLeft,winfo.offsetTop,winfo.childWidth,winfo.childHeight,SWP_NOACTIVATE); #elif defined(USE_X11) static mutex blitting; const lock_guard lock(blitting); @@ -159,6 +159,8 @@ namespace minsky if (GetUpdateRect(hwnd,&r,false)) blit(*winfo, r.left, r.top, r.right-r.left, r.bottom-r.top); } + else + ValidateRect(hwnd, nullptr); // prevent infinite WM_PAINT loop when window is being destroyed return 0; case WM_NCHITTEST: return HTTRANSPARENT; From 9d7b0a7278b87b0b1e6f7b18f6f4cdf2569f8d80 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 04:33:19 +0000 Subject: [PATCH 3/6] Fix Windows canvas deadlock: drawNativeWindows lock scope and remove SetWindowPos from blit - RESTService/addon.cc: drawNativeWindows() now swaps nativeWindowsToRedraw under minskyCmdMutex then releases the lock before calling draw(), matching the existing macOSXDrawNativeWindows() pattern. Previously holding minskyCmdMutex across the entire draw() call could deadlock: GDI/Cairo operations inside draw() can need the JS thread's message pump while the JS thread is blocked trying to acquire minskyCmdMutex for a synchronous command. - model/windowInformation.cc: Removed SetWindowPos(HWND_TOP,...) from blit(). Calling it on every WM_PAINT fired WM_WINDOWPOSCHANGED messages that could cascade into further WM_PAINT messages and triggered screen-sharing hooks (e.g. Zoom) on every paint, keeping the JS thread's message pump busy and preventing it from returning to the libuv event loop to service minsky thread BlockingCall resolutions. - RESTService/addon.cc: Corrected the comment on synchronous destroyFrame dispatch: the real reason is that Windows requires a window to be destroyed on the same thread that created it, not (as previously stated) that minskyCmdMutex blocks windowProc. - model/windowInformation.cc: Expanded the ValidateRect else-branch comment to clarify when GWLP_USERDATA is zero (WindowInformation already torn down). Co-authored-by: highperformancecoder <3075825+highperformancecoder@users.noreply.github.com> --- RESTService/addon.cc | 27 ++++++++++++++++++--------- model/windowInformation.cc | 12 ++++++++++-- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/RESTService/addon.cc b/RESTService/addon.cc index c8af6c73e..074af4688 100644 --- a/RESTService/addon.cc +++ b/RESTService/addon.cc @@ -219,12 +219,12 @@ namespace minsky return String::New(env, utf_to_utf(doCommand(command, arguments))); } #if defined(_WIN32) || defined(MAC_OSX_TK) - // renderFrame and destroyFrame need to be called synchronously on the JS thread. - // The child window is created on the JS thread (via synchronous renderFrame), so - // destroyFrame must also run on the JS thread to avoid cross-thread SendMessageA - // calls in ~WindowInformation() that block the minsky thread while holding - // minskyCmdMutex, causing a deadlock when the JS thread subsequently tries to - // acquire minskyCmdMutex. + // renderFrame and destroyFrame must run synchronously on the JS thread because + // Windows requires that a window be created and destroyed on the same thread. + // Dispatching destroyFrame to the minsky background thread would call + // ~WindowInformation() (and its SendMessageA/SendMessageW) from a thread + // that does not own the child HWND, which is undefined behaviour and can + // cause hangs or crashes. if (command.ends_with(".renderFrame") || command.ends_with(".destroyFrame")) return String::New(env, utf_to_utf(doCommand(command, arguments))); #endif @@ -259,9 +259,19 @@ namespace minsky void drawNativeWindows() { - const lock_guard lock(minskyCmdMutex); + // Don't hold minskyCmdMutex while calling draw(): holding it across + // Cairo/GDI operations can deadlock if those operations need the JS + // thread's message pump while the JS thread is simultaneously blocked + // waiting to acquire minskyCmdMutex for a synchronous command. + // Swap out the pending-redraw set under the lock, then draw without it, + // matching the pattern used by macOSXDrawNativeWindows(). const Timer timer(timers["draw"]); - for (auto i: nativeWindowsToRedraw) + decltype(nativeWindowsToRedraw) windowsToRedraw; + { + const lock_guard lock(minskyCmdMutex); + windowsToRedraw.swap(nativeWindowsToRedraw); + } + for (auto i: windowsToRedraw) try { i->draw(); @@ -273,7 +283,6 @@ namespace minsky break; } catch (...) {break;} - nativeWindowsToRedraw.clear(); } // arrange for native window drawing to happen on node's main thread, required for MacOSX. diff --git a/model/windowInformation.cc b/model/windowInformation.cc index a769659e4..3c7c28366 100644 --- a/model/windowInformation.cc +++ b/model/windowInformation.cc @@ -82,7 +82,11 @@ namespace minsky HDC dc=BeginPaint(winfo.childWindowId, &ps); BitBlt(dc, x, y, width,height,winfo.hdcMem,x,y,SRCCOPY); EndPaint(winfo.childWindowId, &ps); - SetWindowPos(winfo.childWindowId,HWND_TOP,winfo.offsetLeft,winfo.offsetTop,winfo.childWidth,winfo.childHeight,SWP_NOACTIVATE); + // Note: SetWindowPos was previously called here on every blit to keep the + // child window at HWND_TOP, but that fired WM_WINDOWPOSCHANGED on every + // paint which could cascade into further WM_PAINT messages and interact + // badly with screen-sharing hooks (e.g. Zoom). The window is already + // positioned correctly at creation time in WindowInformation(). #elif defined(USE_X11) static mutex blitting; const lock_guard lock(blitting); @@ -160,7 +164,11 @@ namespace minsky blit(*winfo, r.left, r.top, r.right-r.left, r.bottom-r.top); } else - ValidateRect(hwnd, nullptr); // prevent infinite WM_PAINT loop when window is being destroyed + // GWLP_USERDATA is zeroed in ~WindowInformation before the window is + // closed, so reaching here means the WindowInformation has already been + // torn down. Validate the rect so Windows stops generating WM_PAINT + // messages for a window that no longer has a backing buffer. + ValidateRect(hwnd, nullptr); return 0; case WM_NCHITTEST: return HTTRANSPARENT; From 718825d3175d4a365d1d83deb838641becb37ef6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 04:53:25 +0000 Subject: [PATCH 4/6] Revert drawNativeWindows mutex release and destroyFrame synchronous dispatch per review feedback - Revert drawNativeWindows() back to holding minskyCmdMutex during draw() as intended: the mutex protects the model from being modified while a view is being redrawn, and releasing it before draw() would break that invariant. - Revert destroyFrame from the synchronous dispatch list: destroyFrame only posts WM_CLOSE to the window and does not fix the underlying deadlock. Only renderFrame needs to be dispatched synchronously on the JS thread. The only change retained in this PR is the removal of SetWindowPos from blit() in windowInformation.cc, which prevents cascaded WM_WINDOWPOSCHANGED/ WM_PAINT messages that kept the JS thread's message pump busy indefinitely. Co-authored-by: highperformancecoder <3075825+highperformancecoder@users.noreply.github.com> --- RESTService/addon.cc | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/RESTService/addon.cc b/RESTService/addon.cc index 074af4688..5e4eb2f0a 100644 --- a/RESTService/addon.cc +++ b/RESTService/addon.cc @@ -219,13 +219,8 @@ namespace minsky return String::New(env, utf_to_utf(doCommand(command, arguments))); } #if defined(_WIN32) || defined(MAC_OSX_TK) - // renderFrame and destroyFrame must run synchronously on the JS thread because - // Windows requires that a window be created and destroyed on the same thread. - // Dispatching destroyFrame to the minsky background thread would call - // ~WindowInformation() (and its SendMessageA/SendMessageW) from a thread - // that does not own the child HWND, which is undefined behaviour and can - // cause hangs or crashes. - if (command.ends_with(".renderFrame") || command.ends_with(".destroyFrame")) + // renderFrame needs to be called synchronously, otherwise inexplicable hangs occur on Windows. + if (command.ends_with(".renderFrame")) return String::New(env, utf_to_utf(doCommand(command, arguments))); #endif if (minskyCommands.size()>20) @@ -259,19 +254,9 @@ namespace minsky void drawNativeWindows() { - // Don't hold minskyCmdMutex while calling draw(): holding it across - // Cairo/GDI operations can deadlock if those operations need the JS - // thread's message pump while the JS thread is simultaneously blocked - // waiting to acquire minskyCmdMutex for a synchronous command. - // Swap out the pending-redraw set under the lock, then draw without it, - // matching the pattern used by macOSXDrawNativeWindows(). + const lock_guard lock(minskyCmdMutex); const Timer timer(timers["draw"]); - decltype(nativeWindowsToRedraw) windowsToRedraw; - { - const lock_guard lock(minskyCmdMutex); - windowsToRedraw.swap(nativeWindowsToRedraw); - } - for (auto i: windowsToRedraw) + for (auto i: nativeWindowsToRedraw) try { i->draw(); @@ -283,6 +268,7 @@ namespace minsky break; } catch (...) {break;} + nativeWindowsToRedraw.clear(); } // arrange for native window drawing to happen on node's main thread, required for MacOSX. From 9cf906f56e17dc780a9aa7fe7a27748bfd9cb1f0 Mon Sep 17 00:00:00 2001 From: Russell Standish Date: Thu, 19 Mar 2026 15:15:14 +1100 Subject: [PATCH 5/6] Fix warnings. --- RavelCAPI | 2 +- schema/optional.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/RavelCAPI b/RavelCAPI index edbdf36c6..84764879e 160000 --- a/RavelCAPI +++ b/RavelCAPI @@ -1 +1 @@ -Subproject commit edbdf36c6ce7f1a2cd597b06f8da0c529961cddb +Subproject commit 84764879eb36f7e52cef362561fe64d0f9077d95 diff --git a/schema/optional.h b/schema/optional.h index 6ad916586..5206f98da 100644 --- a/schema/optional.h +++ b/schema/optional.h @@ -77,7 +77,7 @@ namespace classdesc void xsd_generate(xsd_generate_t& g, const string& d, const minsky::Optional& a) { xsd_generate_t::Optional o(g,true); - T tmp; // a may be null + T tmp{}; // a may be null xsd_generate(g,d,tmp); } From 8cca9f11ae389ce545ebfbd25f19e6df9db45184 Mon Sep 17 00:00:00 2001 From: Russell Standish Date: Thu, 19 Mar 2026 17:07:57 +1100 Subject: [PATCH 6/6] Fix warnings. --- test/testMinsky.cc | 2 +- test/testPubTab.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/testMinsky.cc b/test/testMinsky.cc index 15cef020f..affccabf4 100644 --- a/test/testMinsky.cc +++ b/test/testMinsky.cc @@ -1270,7 +1270,7 @@ TEST(TensorOps, evalOpEvaluate) // renaming just the canvas variable should change it's type canvas.item=stockVar; - if (auto v=canvas.item->variableCast()) + if (canvas.item->variableCast()) { canvas.renameItem("foo"); EXPECT_TRUE(canvas.item && canvas.item->variableCast()); diff --git a/test/testPubTab.cc b/test/testPubTab.cc index a49a7e62b..7711ca274 100644 --- a/test/testPubTab.cc +++ b/test/testPubTab.cc @@ -157,7 +157,7 @@ TEST_F(PubTabSuite,removeSelf) load("1Free.mky"); // send the Godley table to the pub tab for (auto& i: model->items) - if (auto g=dynamic_cast(i.get())) + if (dynamic_cast(i.get())) { publicationTabs[0].items.emplace_back(i); break;