Hook rewrite / regressions

[Aleksey Bragin]

        Hello,
we have a serious regression in trunk which started 23rd of October
with revision 49231 ("hook rewrite"). Considerable amount of time
passed, automatic testing is not possible thus I'm afraid we are
accumulating bugs.
I think there is a need to revert 49231 + its dependencies in trunk
to allow automated testing to work again and also fix regressions
caused by this rewrite, and move this rewrite to a branch if possible.
In future, it would be a good idea to:
a. Discuss such serious rewrite merges beforehand (like Timo did with
Yarotows).
b. Try not to merge all rewrites simulteneously.
E.g. I have to wait with heap rewrite merge (every rewrite contains a
certain amount of bugs, by definition) even though it's fully ready.
James - could you please share your opinion? The decision is yours,
as you are the author of that commit.
WBR,
Aleksey Bragin.

[Olaf Siejka]

This commit did cause a range of regressions, with apps like KStudio, CPU-z
and CrystalCPUId. Jim was informed about it and since then is working hard
to get them back in order. For me and Johannes, at the present moment,
KSStudio is of top priority, as its utterly vital for any advanced sound
support works.
Regards
2010/10/30 Aleksey Bragin <aleksey(a)reactos.org>
>          Hello,
>  we have a serious regression in trunk which started 23rd of October with
>  revision 49231 ("hook rewrite"). Considerable amount of time passed,
>  automatic testing is not possible thus I'm afraid we are accumulating bugs.
>  I think there is a need to revert 49231 + its dependencies in trunk to
>  allow automated testing to work again and also fix regressions caused by
>  this rewrite, and move this rewrite to a branch if possible.
>  In future, it would be a good idea to:
>  a. Discuss such serious rewrite merges beforehand (like Timo did with
>  Yarotows).
>  b. Try not to merge all rewrites simulteneously.
>  E.g. I have to wait with heap rewrite merge (every rewrite contains a
>  certain amount of bugs, by definition) even though it's fully ready.
>  James - could you please share your opinion? The decision is yours, as you
>  are the author of that commit.
>  WBR,
>   Aleksey Bragin.

[James Tabor]

Hi!
Yes that is right, I'm working on it!
CrystalCPUId has source available and which I'm now working with w2003
sdk mfc to see how mfc uses the SetWindowsHookEx WH_CBT and
WH_MSGFILTER as hooks with callbacks.
MFC:
./appgray.cpp:37:   pThreadState->m_hHookOldCbtFilter =
::SetWindowsHookEx(WH_CBT,
./thrdcore.cpp:262: pThreadState->m_hHookOldMsgFilter =
::SetWindowsHookEx(WH_MSGFILTER,
./wincore.cpp:637:  pThreadState->m_hHookOldCbtFilter =
::SetWindowsHookEx(WH_CBT,
On Sat, Oct 30, 2010 at 12:23 PM, Olaf Siejka <caemyr(a)gmail.com> wrote:
>   This commit did cause a range of regressions, with
> apps like KStudio, CPU-z
>  and CrystalCPUId. Jim was informed about it and since then is working hard
>  to get them back in order. For me and Johannes, at the present moment,
>  KSStudio is of top priority, as its utterly vital for any advanced sound
>  support works.
>  Regards 
KSStudio uses MFC and the crash is related to one of two results of
the first passes (page full) of HCBT_CREATEWND call outs.
>   2010/10/30 Aleksey Bragin <aleksey(a)reactos.org>
> >         Hello,
> >  we have a serious regression in trunk which started 23rd of October with
> >  revision 49231 ("hook rewrite"). Considerable amount of time passed,
> >  automatic testing is not possible thus I'm afraid we are accumulating bugs.
> >  I think there is a need to revert 49231 + its dependencies in trunk to
> >  allow automated testing to work again and also fix regressions caused by
> >  this rewrite, and move this rewrite to a branch if possible.
> >  In future, it would be a good idea to:
> >  a. Discuss such serious rewrite merges beforehand (like Timo did with
> >  Yarotows).
> >  b. Try not to merge all rewrites simulteneously.
> >  E.g. I have to wait with heap rewrite merge (every rewrite contains a
> >  certain amount of bugs, by definition) even though it's fully ready.
> >  James - could you please share your opinion? The decision is yours, as you
> >  are the author of that commit.
> >  WBR,
> >   Aleksey Bragin. 
>  
There is a bug report 5670 related to the issue of DosBox, I posted
patches and test results for weeks! Which resulted in bug 5705. All
the tests I had access to passed! AbiWord, FF and OOo uses hooks. I
was even able to install on real hardware w/o any issues with boot
hangs as it persist with Qemu or the build test system! The boot hang
is due to a kernel issue with newer Qemu ( I noticed the degrade in
time when changes had been made to the kernel ) and does not have any
problems with 0.9.0. After adding the patch from bug 5681/5409, Qemu
0.9.0 works w/o issues. From this end, my tests passed and certify the
hook code as valid. I still have two more commits to do with
HCBT_CREATEWND in window.c ( of passing the original Name/Class
strings and MFC research ) and DosBox hook.c that allows WH_MOUSE_LL
to work as it should ( this one is ready ), now DosBox does not lag
when moving the mouse over its window or hangs in transition from
dosbox mouse to explorer mouse.
Before! Hooks never worked! It would not have made it through
callback.c! Even the set next call hook was wrong. So every developer
just turn them off or just moved them around in the code to hid it.
Event hooks ( which or new hook code is based on ) have minor issues
with message sequencing with hook calls, this is pointed out with
user32 wine msg tests, hook checks enabled and the rest commented out.
Like 3xx passed and 11 failures.
Patients,
James
Research:
WH_MOUSE_LL:
I figured out that this needed a desktop to check and monitor global
hooks and a pti to hold the minimal data ( also no TEB ) to help send
messages to other threads even as it had system message queue to work
from. I now marked these threads as "System Threads". Originally this
never worked (find first hook then next hook hid the call and the next
call had a null proc because it was a dead hook), now mouse_ll
messages are sent and the results are seen in hook_tests and debug
monitors prints.

[Aleksey Bragin]

That's very cool, however two things concern me most of all:
1. Our testbot's "qemu" version (kvm to be precise) did not change.
2. The hang is clearly indicated by the hook rewrite commit, which
triggers it.
And it still hangs...
WBR,
Aleksey.
On Oct 31, 2010, at 4:51 AM, James Tabor wrote:
>   Hi!
>  Yes that is right, I'm working on it!
>  CrystalCPUId has source available and which I'm now working with w2003
>  sdk mfc to see how mfc uses the SetWindowsHookEx WH_CBT and
>  WH_MSGFILTER as hooks with callbacks.
>  MFC:
>  ./appgray.cpp:37:   pThreadState->m_hHookOldCbtFilter =
>  ::SetWindowsHookEx(WH_CBT,
>  ./thrdcore.cpp:262: pThreadState->m_hHookOldMsgFilter =
>  ::SetWindowsHookEx(WH_MSGFILTER,
>  ./wincore.cpp:637:  pThreadState->m_hHookOldCbtFilter =
>  ::SetWindowsHookEx(WH_CBT,
>  On Sat, Oct 30, 2010 at 12:23 PM, Olaf Siejka <caemyr(a)gmail.com>
>  wrote:
> >   This commit did cause a range of regressions,
> > with apps like
> >  KStudio, CPU-z
> >  and CrystalCPUId. Jim was informed about it and since then is
> >  working hard
> >  to get them back in order. For me and Johannes, at the present
> >  moment,
> >  KSStudio is of top priority, as its utterly vital for any advanced
> >  sound
> >  support works.
> >  Regards 
>  KSStudio uses MFC and the crash is related to one of two results of
>  the first passes (page full) of HCBT_CREATEWND call outs.
> >   2010/10/30 Aleksey Bragin
> > <aleksey(a)reactos.org>
> > >         Hello,
> > >  we have a serious regression in trunk which started 23rd of
> > >  October with
> > >  revision 49231 ("hook rewrite"). Considerable amount of time passed,
> > >  automatic testing is not possible thus I'm afraid we are
> > >  accumulating bugs.
> > >  I think there is a need to revert 49231 + its dependencies in
> > >  trunk to
> > >  allow automated testing to work again and also fix regressions
> > >  caused by
> > >  this rewrite, and move this rewrite to a branch if possible.
> > >  In future, it would be a good idea to:
> > >  a. Discuss such serious rewrite merges beforehand (like Timo did
> > >  with
> > >  Yarotows).
> > >  b. Try not to merge all rewrites simulteneously.
> > >  E.g. I have to wait with heap rewrite merge (every rewrite
> > >  contains a
> > >  certain amount of bugs, by definition) even though it's fully ready.
> > >  James - could you please share your opinion? The decision is
> > >  yours, as you
> > >  are the author of that commit.
> > >  WBR,
> > >   Aleksey Bragin. 
> >  
>  There is a bug report 5670 related to the issue of DosBox, I posted
>  patches and test results for weeks! Which resulted in bug 5705. All
>  the tests I had access to passed! AbiWord, FF and OOo uses hooks. I
>  was even able to install on real hardware w/o any issues with boot
>  hangs as it persist with Qemu or the build test system! The boot hang
>  is due to a kernel issue with newer Qemu ( I noticed the degrade in
>  time when changes had been made to the kernel ) and does not have any
>  problems with 0.9.0. After adding the patch from bug 5681/5409, Qemu
>  0.9.0 works w/o issues. From this end, my tests passed and certify the
>  hook code as valid. I still have two more commits to do with
>  HCBT_CREATEWND in window.c ( of passing the original Name/Class
>  strings and MFC research ) and DosBox hook.c that allows WH_MOUSE_LL
>  to work as it should ( this one is ready ), now DosBox does not lag
>  when moving the mouse over its window or hangs in transition from
>  dosbox mouse to explorer mouse.
>  Before! Hooks never worked! It would not have made it through
>  callback.c! Even the set next call hook was wrong. So every developer
>  just turn them off or just moved them around in the code to hid it.
>  Event hooks ( which or new hook code is based on ) have minor issues
>  with message sequencing with hook calls, this is pointed out with
>  user32 wine msg tests, hook checks enabled and the rest commented out.
>  Like 3xx passed and 11 failures.
>  Patients,
>  James
>  Research:
>  WH_MOUSE_LL:
>  I figured out that this needed a desktop to check and monitor global
>  hooks and a pti to hold the minimal data ( also no TEB ) to help send
>  messages to other threads even as it had system message queue to work
>  from. I now marked these threads as "System Threads". Originally this
>  never worked (find first hook then next hook hid the call and the next
>  call had a null proc because it was a dead hook), now mouse_ll
>  messages are sent and the results are seen in hook_tests and debug
>  monitors prints.
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev

[James Tabor]

Hi,
I've noticed this:
// log
Using shell hooks for notification of shell events.
(subsystems/win32/win32k/ntuser/hook.c:1410) Installing: HookId 3 Global TRUE
and, I do not see any of theses,
// log
err:(dll/win32/user32/windows/hook.c:486) HCBT_CREATEWND: wParam
131388 Name 0x0 Class 0x10f24b8
anywhere at anytime when a window is created.
On Sun, Oct 31, 2010 at 4:01 AM, Aleksey Bragin <aleksey(a)reactos.org> wrote:
>   That's very cool, however two things concern me
> most of all:
>  1. Our testbot's "qemu" version (kvm to be precise) did not change.
>  2. The hang is clearly indicated by the hook rewrite commit, which triggers
>  it.
>  And it still hangs...
>  WBR,
>  Aleksey.
>

[James Tabor]

Okay!
On Sun, Oct 31, 2010 at 4:01 AM, Aleksey Bragin <aleksey(a)reactos.org> wrote:
>   That's very cool, however two things concern me
> most of all:
>  1. Our testbot's "qemu" version (kvm to be precise) did not change.
>  2. The hang is clearly indicated by the hook rewrite commit, which triggers
>  it.
>  And it still hangs...
>  WBR,
>  Aleksey.
>  
I have some concerns as well!
I just finished working with "makex bootcdregtest". I need to know why
after the network data is displayed and dumped to the debug, the cmd
window closes. Also I enabled all the hook prints and saw nothing. I
then reopen cmd and run rosautotest then the test started running. How
does the auto run file relate to hooks and callbacks? Does anyone know
how rosautotest works? What are the signals or calls needed? Is it
started by a batch file? Without this information it will make it
difficult for me to troubleshot this.
Thanks,
James

[James Tabor]

Hi all!
Hooks to date:
This is the start of regtest,cmd, a message box pop up warning of a
failure in finding the next file is issued.
ReactOS IP Configuration
Ethernet Adapter Network connection:
(subsystems/win32/win32k/ntuser/message.c:961) message 26
(subsystems/win32/win32k/ntuser/message.c:961) message 26
(subsystems/win32/win32k/ntuser/message.c:961) message 15
        Connection-specific DNS Suffix. . :
        IP Address. . . . . . . . . . . . : 10.0.2.15
        Subnet Mask . . . . . . . . . . . : 255.255.255.0
        Default Gateway . . . . . . . . . : 10.0.2.2
(subsystems/win32/win32k/ntuser/message.c:961) message 275
fixme:(dll/win32/user32/windows/cursoricon.c:1008) Copying from
resource isn't implemented yet
fixme:(dll/win32/user32/windows/cursoricon.c:1008) Copying from
resource isn't implemented yet
fixme:(dll/win32/user32/windows/cursoricon.c:1008) Copying from
resource isn't implemented yet
fixme:(dll/win32/user32/windows/cursoricon.c:1008) Copying from
resource isn't implemented yet
WARNING:  MmLockPageableDataSection at ARM³::DRVMGMT:62 is UNIMPLEMENTED!
WARNING:  MmUnlockPageableImageSection at ARM³::DRVMGMT:39 is UNIMPLEMENTED!
After the message box button is selected,,,,
(subsystems/win32/win32k/ntuser/message.c:961) message 15
(subsystems/win32/win32k/ntuser/message.c:961) message 8
(subsystems/win32/win32k/ntuser/message.c:961) message 15
(subsystems/win32/win32k/ntuser/message.c:961) message 15
(subsystems/win32/win32k/ntuser/message.c:961) message 15
(subsystems/win32/win32k/ntuser/message.c:961) message 275
(subsystems/win32/win32k/ntuser/message.c:961) message 15
(subsystems/win32/win32k/ntuser/message.c:961) message 15
(subsystems/win32/win32k/ntuser/message.c:961) message 512
(subsystems/win32/win32k/ntuser/message.c:961) message 275
(subsystems/win32/win32k/ntuser/message.c:961) message 513
(subsystems/win32/win32k/ntuser/message.c:961) message 15
(subsystems/win32/win32k/ntuser/message.c:961) message 514
(subsystems/win32/win32k/ntuser/message.c:961) message 275
(subsystems/win32/win32k/ntuser/message.c:961) message 784
(subsystems/win32/win32k/ntuser/message.c:961) message 784
(subsystems/win32/win32k/ntuser/message.c:961) message 7
For some reason this goes off without any messages being posted and
kills the reg test system.
err:(dll/win32/user32/windows/hook.c:563) pMsg -11908415  pcMsg -11908415
err:(dll/win32/user32/windows/hook.c:563) pMsg -11891039  pcMsg -11891039
err:(dll/win32/user32/windows/hook.c:563) pMsg 1263029847  pcMsg 1263029847
err:(dll/win32/user32/windows/hook.c:563) pMsg -6070240  pcMsg -6070240
err:(dll/win32/user32/windows/hook.c:563) pMsg 1313296973  pcMsg 1313296973
err:(dll/win32/user32/windows/hook.c:563) pMsg 0  pcMsg 0
err:(dll/win32/user32/windows/hook.c:563) pMsg -6070240  pcMsg -6070240
err:(dll/win32/user32/windows/hook.c:563) pMsg 1313296973  pcMsg 1313296973
err:(dll/win32/user32/windows/hook.c:563) pMsg 0  pcMsg 0
err:(dll/win32/user32/windows/hook.c:563) pMsg -6070240  pcMsg -6070240
err:(dll/win32/user32/windows/hook.c:563) pMsg 1313296973  pcMsg 1313296973
err:(dll/win32/user32/windows/hook.c:563) pMsg 7  pcMsg 7
err:(dll/win32/user32/windows/hook.c:563) pMsg 7  pcMsg 7
err:(dll/win32/user32/windows/hook.c:563) pMsg 1263029847  pcMsg 1263029847
err:(dll/win32/user32/windows/hook.c:563) pMsg 7  pcMsg 7
err:(dll/win32/user32/windows/hook.c:563) pMsg 1263029847  pcMsg 1263029847
err:(dll/win32/user32/windows/hook.c:563) pMsg 7  pcMsg 7
err:(dll/win32/user32/windows/hook.c:563) pMsg 1313296973  pcMsg 1313296973
err:(dll/win32/user32/windows/hook.c:563) pMsg 7  pcMsg 7
err:(dll/win32/user32/windows/hook.c:563) pMsg 1313296973  pcMsg 1313296973
err:(dll/win32/user32/windows/hook.c:563) pMsg 7  pcMsg 7
err:(dll/win32/user32/windows/hook.c:563) pMsg 1263029847  pcMsg 1263029847
err:(dll/win32/user32/windows/hook.c:563) pMsg 7  pcMsg 7
err:(dll/win32/user32/windows/hook.c:563) pMsg 1383288934  pcMsg 1383288934
err:(dll/win32/user32/windows/hook.c:563) pMsg 7  pcMsg 7
err:(dll/win32/user32/windows/hook.c:563) pMsg 1383288934  pcMsg 1383288934
err:(dll/win32/user32/windows/hook.c:563) pMsg 7  pcMsg 7
err:(dll/win32/user32/windows/hook.c:563) pMsg 784  pcMsg 784
err:(dll/win32/user32/windows/hook.c:563) pMsg 784  pcMsg 784
err:(dll/win32/user32/windows/hook.c:563) pMsg 7  pcMsg 7
Now as this goes on, it starts to sync up with peek/get message to sent hooks.
(subsystems/win32/win32k/ntuser/message.c:961) message 1536
(subsystems/win32/win32k/ntuser/message.c:961) message 783
err:(dll/win32/user32/windows/hook.c:563) pMsg 1536  pcMsg 1536
err:(dll/win32/user32/windows/hook.c:563) pMsg 783  pcMsg 783
(subsystems/win32/win32k/ntuser/message.c:961) message 1536
(subsystems/win32/win32k/ntuser/message.c:961) message 784
err:(dll/win32/user32/windows/hook.c:563) pMsg 1536  pcMsg 1536
err:(dll/win32/user32/windows/hook.c:563) pMsg 784  pcMsg 784
(subsystems/win32/win32k/ntuser/message.c:961) message 1536
(subsystems/win32/win32k/ntuser/message.c:961) message 784
err:(dll/win32/user32/windows/hook.c:563) pMsg 1536  pcMsg 1536
err:(dll/win32/user32/windows/hook.c:563) pMsg 784  pcMsg 784
(subsystems/win32/win32k/ntuser/message.c:961) message 26
(subsystems/win32/win32k/ntuser/message.c:961) message 783
err:(dll/win32/user32/windows/hook.c:563) pMsg 26  pcMsg 26
err:(dll/win32/user32/windows/hook.c:563) pMsg 783  pcMsg 783
(subsystems/win32/win32k/ntuser/message.c:961) message 26
(subsystems/win32/win32k/ntuser/message.c:961) message 784
err:(dll/win32/user32/windows/hook.c:563) pMsg 26  pcMsg 26
err:(dll/win32/user32/windows/hook.c:563) pMsg 784  pcMsg 784
(subsystems/win32/win32k/ntuser/message.c:961) message 26
(subsystems/win32/win32k/ntuser/message.c:961) message 784
err:(dll/win32/user32/windows/hook.c:563) pMsg 26  pcMsg 26
err:(dll/win32/user32/windows/hook.c:563) pMsg 784  pcMsg 784
(subsystems/win32/win32k/ntuser/message.c:961) message 26
(subsystems/win32/win32k/ntuser/message.c:961) message 512
err:(dll/win32/user32/windows/hook.c:563) pMsg 26  pcMsg 26
err:(dll/win32/user32/windows/hook.c:563) pMsg 512  pcMsg 512
(subsystems/win32/win32k/ntuser/message.c:961) message 26
(subsystems/win32/win32k/ntuser/message.c:961) message 275
err:(dll/win32/user32/windows/hook.c:563) pMsg 26  pcMsg 26
err:(dll/win32/user32/windows/hook.c:563) pMsg 275  pcMsg 275
(subsystems/win32/win32k/ntuser/message.c:961) message 26
(subsystems/win32/win32k/ntuser/message.c:961) message 15
err:(dll/win32/user32/windows/hook.c:563) pMsg 26  pcMsg 26
err:(dll/win32/user32/windows/hook.c:563) pMsg 15  pcMsg 15
(subsystems/win32/win32k/ntuser/message.c:961) message 26
(subsystems/win32/win32k/ntuser/message.c:961) message 18
err:(dll/win32/user32/windows/hook.c:563) pMsg 26  pcMsg 26
err:(dll/win32/user32/windows/hook.c:563) pMsg 18  pcMsg 18
(subsystems/win32/win32k/ntuser/message.c:961) message 26
err:(dll/win32/user32/windows/hook.c:563) pMsg 26  pcMsg 26
(subsystems/win32/win32k/ntuser/message.c:961) message 49215
err:(dll/win32/user32/windows/hook.c:563) pMsg 49215  pcMsg 49215
(subsystems/win32/win32k/ntuser/message.c:961) message 49215
err:(dll/win32/user32/windows/hook.c:563) pMsg 49215  pcMsg 49215
(subsystems/win32/win32k/ntuser/message.c:961) message 49215
err:(dll/win32/user32/windows/hook.c:563) pMsg 49215  pcMsg 49215
I'm working on this as fast as I can,
James

[James Tabor]

Hi!
I guess I just answered my own question. What is happening, is basic
stack corruption. WH_GETMESSAGE as an example, when the call to
co_HOOK_CallHooks is made with the Msg pointer in the data stack, we
send the global message hook with this pointer. So it crashes or sends
the wrong corrupted garbage data to the hook proc. This leaves us with
one way to fix this,,, using a allocated packet the same way we send
hooks to the user proc....
So we expand HOOKPACK, co_CallHook and IntCallLowLevelHook to handle this..
Thanks,
James

[James Tabor]

Hi all,
Revision 49472 fixes the issue and now the Regression Test is
restored. I need more time to investigate the other hook issues.
Global hooks, are they global or only called when in the thread except
keyboard or mouse low level hooks?
Reinventing the Wheel,
James

[Johannes Anderwald]

>   Hi all, 
Hi,
>   Revision 49472 fixes the issue and now the Regression
> Test is
>  restored. I need more time to investigate the other hook issues. 
KSStudio is now broken since weeks! Why cant you just revert it and develop & test it
in a branch?
PS : Quoting you from rev 30807:
[blabla bullshit] If you dont know what your are doing? Do not play in here.
regards
Johannes

[James Tabor]

Hi,
On Fri, Nov 5, 2010 at 3:13 AM, Johannes Anderwald
<johannes.anderwald(a)reactos.org> wrote:
> >   Hi all, 
>  Hi,
> >   Revision 49472 fixes the issue and now the
> > Regression Test is
> >  restored. I need more time to investigate the other hook issues. 
>  KSStudio is now broken since weeks! Why cant you just revert it and develop & test it
> in a branch?
>  
I'm still here and working on it right now. I would not have committed
it unless I knew it would break everything. I tested OOo with FF and
it worked! I can't help if developers do not read bug reports.
>   PS : Quoting you from rev 30807:
>  [blabla bullshit] If you dont know what your are doing? Do not play in here.
>  
I know, I tested everything I had and it was not good enough.
>   regards
>  Johannes 
I'm not here to pick a fight,,, developers need to start notating the
source with information to let newbies in the future understand what
ReactOS is about.
James