[Bug 59655] New: comctl32: Infinite WM_PAINT loop in SysListView32 when parent triggers layout update during NM_CUSTOMDRAW (MPC-BE)
http://bugs.winehq.org/show_bug.cgi?id=59655 Bug ID: 59655 Summary: comctl32: Infinite WM_PAINT loop in SysListView32 when parent triggers layout update during NM_CUSTOMDRAW (MPC-BE) Product: Wine Version: 11.6 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: major Priority: P2 Component: comctl32 Assignee: wine-bugs@list.winehq.org Reporter: lldmakhbfcrmqyluns@nesopf.com Distribution: --- Steps to reproduce: Open MPC-BE. Ensure the Playlist panel is visible (View -> Playlist). Click on the top of the playlist window. The application GUI freezes permanently. Analysis / Root Cause: By adding extensive custom tracing to dlls/comctl32/listview.c, an infinite WM_PAINT loop was identified. Here is what happens during a single iteration of the loop: WM_PAINT is dispatched, calling LISTVIEW_Refresh. LISTVIEW_Refresh sets infoPtr->bIsDrawing = TRUE and starts drawing. During the drawing process, the listview sends NM_CUSTOMDRAW (stage CDDS_PREPAINT / CDDS_ITEMPREPAINT) to the parent window (MPC-BE). The parent's custom draw handler executes logic that indirectly triggers a layout update of the listview (e.g., via SetWindowPos on the header or changing item metrics). This layout update calls LISTVIEW_UpdateSize, which eventually calls LISTVIEW_InvalidateList. LISTVIEW_InvalidateList calls LISTVIEW_InvalidateRect(infoPtr, NULL), marking the entire client area as dirty. LISTVIEW_Refresh finishes and sets infoPtr->bIsDrawing = FALSE. Because the client area was invalidated during step 6, the system immediately generates a new WM_PAINT message. The cycle repeats indefinitely. Native Windows does not freeze, suggesting it ignores invalidation requests while the control is already actively painting itself. The Fix: The LISTVIEW_INFO structure already contains the bIsDrawing flag specifically to prevent reentrancy issues. The fix is to simply return early from LISTVIEW_InvalidateList if bIsDrawing is TRUE. Since LISTVIEW_Refresh is currently repainting the entire visible area anyway, queuing another full invalidation is both redundant and dangerous. Patch / Diff diff --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -1792,6 +1792,7 @@ static inline void LISTVIEW_InvalidateList(const LISTVIEW_INFO *infoPtr) { + if (infoPtr->bIsDrawing) return; LISTVIEW_InvalidateRect(infoPtr, NULL); } -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59655 --- Comment #1 from T_Im <lldmakhbfcrmqyluns@nesopf.com> --- FYI: I'm not a C dev and don't plan to submit a formal patch. I'm just sharing analysis to help fix the bug for everyone. Feel free to use or rewrite the code. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59655 --- Comment #2 from T_Im <lldmakhbfcrmqyluns@nesopf.com> --- Added Stefan Dösinger and Zhiyi Zhang to CC as recent contributors to these files. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59655 T_Im <lldmakhbfcrmqyluns@nesopf.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |stefan@codeweavers.com, | |zzhang@codeweavers.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59655 xmine64 <the.madamin20@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |the.madamin20@gmail.com --- Comment #3 from xmine64 <the.madamin20@gmail.com> --- There's a is_redrawing function, which LISTVIEW_InvalidateRect and LISTVIEW_InvalidateItem call before doing invalidation, so I suggest doing this instead, what do you think? diff --git a/dll/win32/comctl32/listview.c b/dll/win32/comctl32/listview.c index c7d539488bc..9e99efcf9e9 100644 --- a/dll/win32/comctl32/listview.c +++ b/dll/win32/comctl32/listview.c @@ -1743,7 +1743,7 @@ static inline BOOL LISTVIEW_DrawFocusRect(const LISTVIEW_INFO *infoPtr, HDC hdc) static inline BOOL is_redrawing(const LISTVIEW_INFO *infoPtr) { - return infoPtr->redraw; + return infoPtr->redraw || infoPtr->bIsDrawing; } static inline void LISTVIEW_InvalidateRect(const LISTVIEW_INFO *infoPtr, const RECT* rect) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59655 --- Comment #4 from xmine64 <the.madamin20@gmail.com> --- I made a really dumb mistake and I don't know how can I delete my comment. Please ignore it. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59655 KRosUser <kyle.kcsoftwares@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle.kcsoftwares@gmail.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59655 Ken Sharp <imwellcushtymelike@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, source Severity|major |normal -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59655 --- Comment #5 from Stefan Dösinger <stefan@codeweavers.com> --- @xmine64, you can't delete comments yourself. Bugzilla admins can delete them if e.g. there is a link to copyrighted material or a file is attached even though the user attaching it doesn't have permission to redistribute it. This isn't the case in your particular comment. If you think your suggestion is technically incorrect, the better solution is to write another comment explaining why your original thinking is wrong. Then somebody who has a similar idea can learn what you figured out. As for the bug itself: I think this needs some tests how windows behaves. What I suspect is that the layout changes aren't supposed to continue forever. The size changes are probably no-ops. LISTVIEW_UpdateSize or something else should probably check for that and not invalidate the control in this case. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
WineHQ Bugzilla