Date: Fri, 20 Feb 1998 04:15:46 -0500
From: Charles Binko <###@###.###>
To: Philip Helsel <philip.helsel@Central>
Subject: Update on JVM Bug

Philip:
    I think I finally figured out what is happening with our bug. You
won't be happy.

    I have managed to trap the bug in the VC++ debugger (at least
occasionally). Basically, here's the scenario which I think triggers
it:

1) a user presses a button to close a window (Frame).

2) the MOUSE_UP event is pushed onto the Windows EventQueue and will
eventually close the Frame.

3) Additional events (i.e. MOUSE_EXIT as seen in the attached stack
trace) are also pushed onto the Windows EventQueue targetting the same
Component (in this case a subclass of Panel).

4) the MOUSE_UP travels the normal path through WndProc, etc towards the
awt_Component destructor.

5) before it deletes the awt_Component, the MOUSE_EXIT reaches
AwtComponent::WndProc which does a sanity check by making sure that the
HWnd it is given matches the m_hwnd it finds when it looks up the
awt_Component
*that is the last sanity check*

6) As the MOUSE_UP progresses, it succeeds in disposing the frame, which
disposes it's peer and deletes it's awt_Component along with all of the
Components on the Frame. This destructor sets m_hwnd and m_peerObject
to NULL on all of the awt_Components.

7) Since memory is tight (which is when this happens the most) the
memory used by the target awt_Component is reused. Therefore,
awt_Component->m_peerObject and m_hwnd are no longer NULL: they are
garbage (in this I was running with the Debug runtime, so delete set the
memory to 0xddddddd etc. see the screen shot). In normal usage, if the
memory was not reused, it may contain the original values, and might
operate properly causing the later event to fizzle.

8) MOUSE_EXIT has been moving along now, and is in the method
AwtComponent::SendMouseEvent(). It obtains a lock on the value that
GetLock() returns. GetLock() is a normal inline function, essentially a
macro. It looks at this->m_lock (which translates to garbage->garbage =
garbage) and returns garbage (in this case, GetLock().LockSemaphore is
0xdddddddd.. big surprise). Having a useless lock on garbage, but
failing to see a problem, SendMouseEvent() continues.

9) It checks this->m_peerObject to make sure it is not NULL. If the
memory has been reused, however, it is NOT NULL, it is in this case,
0xddddddddd.(normally garbage). So it merrily continues.

10) SendMouseEvent() then calls its first virtual function. In this
case, it is getInsets() which is a virtual inline function (which is
unbelievably stupid coming from Sun! I'll be happy to lecture on just
what virtual inline does, but it's irrelevant :). SendEvents happily
loads the address for this->GetInsets() (which again translates to
garbage->Garbage() = garbage) onto the stack, jumps there and tries to
run it. All hell breaks loose. In the cases where I catch the error, I
get an access violation... this is pure luck (in part thanks to the
Debug runtime). It is perfectly possible for it to jump to areas that do
more damage or less, and often that don't give me that nice error to
catch!

Basically, the solution to this is straightforward: put a sanity check
WITHIN the CriticalSection. Ideally, put a CriticalSection around the
entire call to AwtComponent::WindowProc(). This would probably hurt
performance, but at least it would be correct! I am going to do that
immediately here and see what develops. Your people can probably
improve on that solution by making smaller CriticalSections on only that
code that needs it.

Just to condense all of that mess above, here are the conditions which
lead to the scenario.

1) A component on the Frame triggers it's disposal. I don't know if the
X button or any other closing mechanisms would do it, but the component
makes it easier to generate additional events.
2) Events such as MOUSE_EXIT must follow the triggering event (Usually
MOUSE_UP, I guess) so the mouse should probably be moving
3) The Windows EventQueue must be full enough to put enough lag that the
memory is deleted
4) Memory must be tight enough that the deleted area has been reclaimed
when the secondary events get to the awt_Component.

Just to make this clear, I DO NOT think that SendMouseEvent is the
problem. I see many entry points to the code which can be called AFTER
its deletion. SendKeyEvent, GetFont, and SendFocus event are members
that don't sanity check within the CriticalSection. In addition,
un_awt_windows_WComponentPeer_show, sun_awt_windows_WComponentPeer_hide
etc all DO sanity checks, but OUTSIDE their CriticalSections.

I've included a screen-shot of my MSVC screen, and an annotated Stack
Trace. I hope this is of use to you. Please contact me as soon as you
have any information. I will be out of the office Friday afternoon, but
you may reach me at the numbers below if you have anything urgent
(please do!).

Thanks for all your help!

Bill Binko
Work: (800)872-2992 x3021
Home: (813)934-6724
Cell: (813)503-9990


[Image]


Stack Trace:
AwtComponent::SendMouseEvent(long 505, __int64 887956941012, long 358,
long 480, long 0, long 0, long 0, tagMSG * 0x00000000 {msg=??? wp=???
lp=???}) line 1714 + 12 bytes
    Variables:
        + classEvent 0x013b50c8
        + &insets 0x0258f290 {top=0xb1b15940 bottom=0x000107b2
left=0x000c0015 right=0x01bd3dca}
        + insets {top=0xb1b15940 bottom=0x000107b2 left=0x000c0015
right=0x01bd3dca}
        - this 0x00c9fa80
            + AwtObject {...}
            + m_hwnd 0xdddddddd
             m_myControlID 0xdddddddd
             m_backgroundColorSet 0xdddddddd
             m_dragged 0xdddddddd
             m_colorForeground 0xdddddddd
             m_colorBackground 0xdddddddd
            + m_penForeground 0xdddddddd
            + m_brushBackground 0xdddddddd
            + m_sharedDC 0xdddddddd
             m_DefWindowProc 0xdddddddd
             m_nextControlID 0xdddddddd
            + m_hCursor 0xdddddddd
            + m_lock {...}
            + m_hdwp 0xdddddddd
             m_CodePage 0x000004e4
            + m_childList 0xdddddddd
AwtComponent::WmMouseExit(unsigned int 0, int 358, int 480) line 1081
    Variables:
     mrConsume 2
    + this 0x00c9fa80
        (ALL 0xddd's)
     x 358
     y 480
AwtComponent::WindowProc(unsigned int 1129, unsigned int 0, long
31457638) line 658 + 38 bytes
    Variables:
     lParam 31457638
     mr mrDoDefault
    + this 0x00c9fa80
        (ALL 0xddd's)
     wParam 0
AwtComponent::WndProc(void * 0x002a086c, unsigned int 1129, unsigned int
0, long 31457638) line 179 + 23 bytes
    Variables:
    + hWnd 0x002a086c
        (Can't expand)
     lParam 31457638
     message 1129
    + self 0x00c9fa80
        (ALL ddd's)
     wParam 0
SendMessageWorker@20 + 154 bytes
SendMessageA@16 + 71 bytes
AwtComponent::SendMessageA(unsigned int 1129, unsigned int 0, long
31457638) line 200 + 27 bytes
    Variables:
     lParam 31457638
     msg 1129
    + this 0x00c9fa80
        (ALL ddd's)
     wParam 0
AwtToolkit::PreProcessMouseMsg(AwtComponent * 0x00c93f70, tagMSG &
{msg=0x00000200 wp=0x00000000 lp=0x01e00166}) line 526

TOO MANY VARIABLES: CALL ME FOR VALUES I WILL LEAVE DEBUGGER IN THIS
STATE

AwtToolkit::PreProcessMsg(tagMSG & {msg=0x00000200 wp=0x00000000
lp=0x01e00166}) line 479 + 16 bytes
AwtToolkit::GetMessageFilter(int 0, unsigned int 1, long 39384276) line
357 + 16 bytes
DispatchHookA@16 + 42 bytes
CallHookWithSEH@16 + 62 bytes
__fnHkINLPMSG@4 + 36 bytes
KiUserCallbackDispatcher@12 + 19 bytes
_PeekMessage@24 + 34 bytes
USER32! 77e71bfe()
AwtToolkit::PumpWaitingMessages() line 439 + 18 bytes
AwtToolkit::MessageLoop() line 424
sun_awt_windows_WToolkit_eventLoop(Hsun_awt_windows_WToolkit *
0x013ac548) line 637
Java_sun_awt_windows_WToolkit_eventLoop_stub(stack_item * 0x021c101c,
execenv * 0x0258ff00) line 970 + 11 bytes
invokeNativeMethod(Hjava_lang_Object * 0x013ac548, methodblock *
0x021bc928, int 1, execenv * 0x0258ff00) line 230 + 14 bytes
invokeLazyNativeMethod(Hjava_lang_Object * 0x013ac548, methodblock *
0x021bc928, int 1, execenv * 0x0258ff00) line 531 + 22 bytes
ExecuteJava(unsigned char * 0x0258fe50, execenv * 0x0258ff00) line 1445
+ 22 bytes
do_execute_java_method_vararg(execenv * 0x0258ff00, void * 0x013ac6b8,
char * 0x1007f264, char * 0x1007f260, methodblock * 0x00000000, int 0,
char * 0x0258feec, long * 0x00000000, int 0) line 604 + 13 bytes
execute_java_dynamic_method(execenv * 0x0258ff00, Hjava_lang_Object *
0x013ac6b8, char * 0x1007f264, char * 0x1007f260) line 283 + 33 bytes
ThreadRT0(Hjava_lang_Thread * 0x013ac6b8) line 61 + 23 bytes
_start(sys_thread * 0x0088cd00) line 244 + 13 bytes
MSVCRT! 78009b75()
BaseThreadStart@8 + 81 bytes



Date: Sat, 21 Feb 1998 18:30:26 -0500
From: Charles Binko <###@###.###>
To: Philip Helsel <philip.helsel@Central>

Subject: Re: Java Race Condition Problem

Philip,

    Anyway, I have put in place the CriticalSection around the upcall from WndProc.
It has so far solved the problem. We have someone beating on it all weekend. I
really think that what I sent you Friday morning was the problem.

Here are the diffs.

The file is awt_Component.cpp. Basically, all that is required is to put
the sanity check withing the CriticalSection. Please make sure that they check other entry points to the object (show() and hide() are candidates) to make sure that they are thread safe as well.

diffs ......

//Line 182 of original awt_Component.cpp
//
// Single window proc for all the components. Delegates real work to the component's
// WindowProc() member function.
//
LRESULT CALLBACK AwtComponent::WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
{
    ASSERT(::IsWindow(hWnd));

    AwtComponent* self = GetComponent(hWnd);

    if (!self || self->GetHWnd() != hWnd) {
        return ::DefWindowProc(hWnd, message, wParam, lParam);
    }
                                // Binko's Sanity Check
                                // Note it is WITHIN Critical Section
    CriticalSection::Lock l(self->GetLock());
    if (self->GetHWnd() != hWnd)
      {
        return ::DefWindowProc(hWnd, message, wParam, lParam);
      }
    return self->WindowProc(message, wParam, lParam);
}




Custoemr contact info:
Bill Binko 800-872-2992 x3021