Skip to content

fix (WeaselUI): Crash when the Weasel Panel creation is re-entried. #1839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cqjjjzr wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix (WeaselUI): Crash when the Weasel Panel creation is re-entried. #1839

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 10 additions & 14 deletions WeaselUI/WeaselUI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,23 +83,19 @@ VOID CALLBACK UIImpl::OnTimer(_In_ HWND hwnd,
}

bool UI::Create(HWND parent) {
if (pimpl_) {
pimpl_->panel.Create(
parent, 0, 0, WS_POPUP,
WS_EX_TOOLWINDOW | WS_EX_TOPMOST | WS_EX_NOACTIVATE | WS_EX_TRANSPARENT,
0U, 0);
return true;
if (!pimpl_) {
pimpl_ = new UIImpl(*this);
if (!pimpl_)
return false;
}
Comment on lines +87 to 90
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new UIImpl(*this) will typically throw std::bad_alloc on failure (or terminate if exceptions are disabled), so the subsequent if (!pimpl_) return false; is ineffective/misleading unless you intentionally use std::nothrow. Consider either switching to new (std::nothrow) UIImpl(*this) and keeping the null-check, or removing the null-check and handling allocation failure via exception policy for this project.

Copilot uses AI. Check for mistakes.

pimpl_ = new UIImpl(*this);
if (!pimpl_)
return false;
if (pimpl_->panel.IsWindow())
return true;
Comment on lines +92 to +93
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create(HWND parent) now returns early when panel.IsWindow() is true, which makes the parent argument effectively ignored on re-entry. If callers can re-enter Create() with a different parent (e.g., focus moved to a different active window), the panel will remain owned by the old window, which can break z-order/lifetime semantics for a popup. Consider checking the current owner (GetParent() for a WS_POPUP owned window) and, when it differs from parent, either update the owner (e.g., via SetWindowLongPtr(GWLP_HWNDPARENT, ...)) or destroy/recreate the panel with the new parent.

Suggested change
if (pimpl_->panel.IsWindow())
return true;
if (pimpl_->panel.IsWindow()) {
HWND current_parent = ::GetParent(pimpl_->panel.m_hWnd);
if (current_parent != parent) {
::SetLastError(0);
LONG_PTR previous_parent = ::SetWindowLongPtr(
pimpl_->panel.m_hWnd, GWLP_HWNDPARENT,
reinterpret_cast<LONG_PTR>(parent));
if (previous_parent == 0 && ::GetLastError() != 0)
return false;
}
return true;
}

Copilot uses AI. Check for mistakes.

pimpl_->panel.Create(
parent, 0, 0, WS_POPUP,
WS_EX_TOOLWINDOW | WS_EX_TOPMOST | WS_EX_NOACTIVATE | WS_EX_TRANSPARENT,
0U, 0);
return true;
return pimpl_->panel.Create(parent, 0, 0, WS_POPUP,
WS_EX_TOOLWINDOW | WS_EX_TOPMOST |
WS_EX_NOACTIVATE | WS_EX_TRANSPARENT,
0U, 0) != nullptr;
}

void UI::Destroy(bool full) {
Expand Down