Re: [lmi] lmi + wine-3.0 (Debian 3.0-1) = purple fungus

[Vadim Zeitlin]

On Wed, 18 Jul 2018 21:12:37 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2018-03-06 22:53, Greg Chicares wrote:
GC> > On 2018-03-06 14:54, Vadim Zeitlin wrote:
GC> >> On Fri, 2 Mar 2018 18:32:16 +0000 Greg Chicares <address@hidden> wrote:
GC> [...]
GC> >> GC> For now, I'll just copy and paste the console messages:
GC> >> GC> 
GC> >> GC> $wine ./lmi_wx_shared.exe --ash_nazg --data_path=/opt/lmi/data
GC> >> GC> In file ../src/msw/mdi.cpp at line 1491: 
'SendMessage(WM_MDISETMENU)' failed with error 0x00000578 (Invalid window 
handle.).
[...]
GC> > So that MDI-578 thing is a buster regression.
GC> 
GC> It can't be merely a wine or gcc regression, because that message
GC> has reappeared since I upgraded wx for PDF pagination:

 But nothing has changed in wxWidgets neither, so there must be some other,
hidden, variable which affects this.

GC> /opt/lmi/bin[0]$wine ./lmi_wx_shared --ash_nazg --data_path=/opt/lmi/data 
--pyx=only_new_pdf
GC> In file /cache_for_lmi/vcs/wxWidgets/src/msw/mdi.cpp at line 1488: 
'SendMessage(WM_MDISETMENU)'\
GC>  failed with error 0x00000578 (Invalid window handle.).
GC> 
GC> As before:
GC>   it always appears once per invocation of lmi;
GC>   it appears even if I quit lmi immediately after loading;

 This is to be expected, as it's shown from wxFrame::SetMenuBar() called
by Skeleton::InitMenuBar() on application startup.

GC>   it's shown only on the console, so no end user would see it; and
GC>   it doesn't seem to be associated with any discernible ill effect;
GC> so it doesn't seem to call for urgent investigation, especially if
GC> it's observed only with 'wine'.

 Yes, I'm quite sure it is harmless. You could confirm it by setting
WINEDEBUG=mdi and verifying that you get 2 trace messages from
SetMenuBar(), the first one of the form

        "%p, frame menu %p, window menu %p\n"

and the second one, only given if the parameter validity checks succeed, of
the form

        "old frame menu %p, old window menu %p\n"


 So it is really not that important and the only annoying thing is that I
still have no idea why does it happen and, especially, why does it only
happen sometimes and not all the time, consistently. But I think that the
only way to find it out is to debug Wine, which is something I wanted to do
since quite some time, to deal with other problems in it, but I'm not sure
if it's worth doing it just for this.


GC> [Speed-read through this wild-goose chase to the git question below.]
GC> The source line referred to is marked below:
GC> 
GC>   void MDISetMenu(wxWindow *win, HMENU hmenuFrame, HMENU hmenuWindow)
GC> ...
GC>         ::SetLastError(ERROR_SUCCESS);
GC> 
GC>         if ( !::SendMessage(GetWinHwnd(win),
GC>                             WM_MDISETMENU,
GC>                             (WPARAM)hmenuFrame,
GC>                             (LPARAM)hmenuWindow) )
GC>         {
GC>             const DWORD err = ::GetLastError();
GC>             if ( err != ERROR_SUCCESS )
GC>             {
GC>                 wxLogApiError(wxT("SendMessage(WM_MDISETMENU)"), err); 
<--THIS IS LINE 1488
GC>             }

 I've decided to check if this code was useful at all by passing an
intentionally invalid menu handle to this function. With "genuine" MSW, at
least, it does show the expected error:

'SendMessage(WM_MDISETMENU)' failed with error 0x00000579 (Invalid menu 
handle.).

so this check is not completely useless and I'd rather keep it. AFAICS from
reading Wine sources, it's never going to be executed in its case, however,
as it doesn't seem to ever call SetLastError() from its MDISetMenu()
function. I could be missing something, however, as I also thought that the
native MSW version didn't do it neither, yet the experiment above showed
that it does. In my defense, user32.dll disassembly is even more difficult
to read than Wine code, however...


GC> However, AFAICT, the last time that function was modified was
GC>   commit 9650c5b55bb2cf2a6a57a120b56cb9c8206b2aff
GC>   2014-10-24T13:45:41+00:00
GC> and we've certainly updated wx more than once in the last four years,
GC> so whatever made this message reappear now must be upstream of that
GC> function. At least that's the way it appears if I do
GC>   git log --patch src/msw/mdi.cpp
GC> and search for "MDISetMenu".

 FWIW a potentially faster way to see this is to use "git blame". If you
don't want to scroll all the output from it, you could also do this:

---------------------------------- >8 --------------------------------------
% git blame -L:MDISetMenu -L:MDISetMenu src/msw/mdi.cpp
838e6ed7076 (Vadim Zeitlin    2009-01-28 09:01:44 +0000   87) void 
MDISetMenu(wxWindow *win, HMENU hmenuFrame, HMENU hmenuWindow);
42e69d6b435 (Vadim Zeitlin    1999-05-19 00:53:27 +0000   88)
42e69d6b435 (Vadim Zeitlin    1999-05-19 00:53:27 +0000   89) // insert the 
window menu (subMenu) into menu just before "Help" submenu or at
42e69d6b435 (Vadim Zeitlin    1999-05-19 00:53:27 +0000   90) // the very end 
if not found
838e6ed7076 (Vadim Zeitlin    2009-01-28 09:01:44 +0000 1471) void 
MDISetMenu(wxWindow *win, HMENU hmenuFrame, HMENU hmenuWindow)
42e69d6b435 (Vadim Zeitlin    1999-05-19 00:53:27 +0000 1472) {
e45a6885492 (Vadim Zeitlin    2006-01-07 01:14:45 +0000 1473)     if ( 
hmenuFrame || hmenuWindow )
e45a6885492 (Vadim Zeitlin    2006-01-07 01:14:45 +0000 1474)     {
9650c5b55bb (Vadim Zeitlin    2014-10-24 13:45:41 +0000 1475)         // Under 
XP, the last error seems to be not reset by this function, so
9650c5b55bb (Vadim Zeitlin    2014-10-24 13:45:41 +0000 1476)         // ensure 
we don't report spurious errors below when setting the menu
9650c5b55bb (Vadim Zeitlin    2014-10-24 13:45:41 +0000 1477)         // 
initially.
9650c5b55bb (Vadim Zeitlin    2014-10-24 13:45:41 +0000 1478)         
::SetLastError(ERROR_SUCCESS);
9650c5b55bb (Vadim Zeitlin    2014-10-24 13:45:41 +0000 1479)
e45a6885492 (Vadim Zeitlin    2006-01-07 01:14:45 +0000 1480)         if ( 
!::SendMessage(GetWinHwnd(win),
e45a6885492 (Vadim Zeitlin    2006-01-07 01:14:45 +0000 1481)                   
          WM_MDISETMENU,
e45a6885492 (Vadim Zeitlin    2006-01-07 01:14:45 +0000 1482)                   
          (WPARAM)hmenuFrame,
e45a6885492 (Vadim Zeitlin    2006-01-07 01:14:45 +0000 1483)                   
          (LPARAM)hmenuWindow) )
e45a6885492 (Vadim Zeitlin    2006-01-07 01:14:45 +0000 1484)         {
9650c5b55bb (Vadim Zeitlin    2014-10-24 13:45:41 +0000 1485)             const 
DWORD err = ::GetLastError();
9650c5b55bb (Vadim Zeitlin    2014-10-24 13:45:41 +0000 1486)             if ( 
err != ERROR_SUCCESS )
43b2d5e7c3b (Vadim Zeitlin    2009-07-20 16:47:54 +0000 1487)             {
9a83f860948 (Vadim Zeitlin    2009-07-23 20:30:22 +0000 1488)                 
wxLogApiError(wxT("SendMessage(WM_MDISETMENU)"), err);
43b2d5e7c3b (Vadim Zeitlin    2009-07-20 16:47:54 +0000 1489)             }
e45a6885492 (Vadim Zeitlin    2006-01-07 01:14:45 +0000 1490)         }
e45a6885492 (Vadim Zeitlin    2006-01-07 01:14:45 +0000 1491)     }
42e69d6b435 (Vadim Zeitlin    1999-05-19 00:53:27 +0000 1492)
42e69d6b435 (Vadim Zeitlin    1999-05-19 00:53:27 +0000 1493)     // update 
menu bar of the parent window
42e69d6b435 (Vadim Zeitlin    1999-05-19 00:53:27 +0000 1494)     wxWindow 
*parent = win->GetParent();
223d09f6b52 (Karsten Ballüder 1999-10-08 14:35:56 +0000 1495)     wxCHECK_RET( 
parent, wxT("MDI client without parent frame? weird...") );
42e69d6b435 (Vadim Zeitlin    1999-05-19 00:53:27 +0000 1496)
a967ef9dcef (Vadim Zeitlin    2001-02-26 10:24:24 +0000 1497)     
::SendMessage(GetWinHwnd(win), WM_MDIREFRESHMENU, 0, 0L);
788722ac5ca (Julian Smart     2001-05-14 16:13:12 +0000 1498)
42e69d6b435 (Vadim Zeitlin    1999-05-19 00:53:27 +0000 1499)     
::DrawMenuBar(GetWinHwnd(parent));
42e69d6b435 (Vadim Zeitlin    1999-05-19 00:53:27 +0000 1500) }
42e69d6b435 (Vadim Zeitlin    1999-05-19 00:53:27 +0000 1501)
---------------------------------- >8 --------------------------------------

which immediately shows that 9650c5b55bb was indeed the latest commit to
modify this function (you need to use "-L" twice because the first one
matches the function declaration and we want the definition).

GC> I thought I'd be able to see that file
GC> as of the wx SHA1 we're using in production thus:
GC> 
GC> /opt/lmi/vcs/wxWidgets[0]$git show 
2a5aafb27419efb36999e24dbcc091eacea56286:src/msw/mdi.cpp
GC> fatal: Path 'src/msw/mdi.cpp' exists on disk, but not in 
'2a5aafb27419efb36999e24dbcc091eacea56286'.
GC> 
GC> but that SHA1 doesn't even seem to exist:
GC> 
GC> /opt/lmi/vcs/wxWidgets[128]$git show 
2a5aafb27419efb36999e24dbcc091eacea56286                
GC> fatal: bad object 2a5aafb27419efb36999e24dbcc091eacea56286
GC> 
GC> How is that so?

 The only explanation I have is that you somehow hadn't fetched it yet, but
if you had run the updated install_wx.sh from the PR 86, you should have
it.

GC> I thought 'git show SHA1' should always work. My HEAD is detatched
GC> three commits into PR 86, but that shouldn't matter, right?

 No, it definitely shouldn't. The install_wx.sh script does

        git rev-parse --quiet --verify "$wx_commit_sha^{commit}"

and if this fails (does it for you now, if you run it interactively, with
wx_commit_sha=2a5aafb27419efb36999e24dbcc091eacea56286?), it does "git
fetch" which should have got it. I'm sorry, but I have no idea about what's
going on here...

 Regards,
VZ