Access violation in RenderWindow::GetEvent

[PJägare]

Hello.

We are developing an application that under some circumstances uses several windows. We have encountered a problem where closing one window causes an access violation in the GetEvent method of a different window.
The problem is timing based or otherwise semirandom in that it appears only occasionally when running on my computer, but often or always on my partner's computer.
The application has several threads, but all window creation, destruction and event handling is on the main thread.

Any idea what could be causing this?

[Laurent]

Can you please provide a complete and minimal source code that reproduces the problem?

[PJägare]

Unfortunately no. The error does not occur in a clearly defined part of the code that could be isolated. In fact, the event that causes the window to close and the call to GetEvent that crashes are not even in the same iteration of the main loop.

What I was hoping for was some insight into what GetEvent does that could access invalid memory. For example, does it access anything created outside of SFML that might have been incorrectly deallocated? Or do windows use some shared resource that must be specially managed? Or anything else that comes to mind?

[Laurent]

Nop, there's no obvious explanation. I'm afraid you'll have to investigate further and manage to reduce your code to something minimal. If simple codes don't reproduce the problem, chances are that its cause is not trivial.

Never debug directly into complex source code, you'll waste time. Simplify it first.

[Mikademus]

Hello Laurent, I am the second half of the "we" mentioned in the OP

I thought I'd add a little extra information about the situation leading to this problem, so as to brainstorm together with you about possible causes, whether in SFML or outside.

In the current Win32/SFML1.6 set-up, the application always has a main window and can create additional ones. The problem with GetEvent only emerges when we have at least three windows. Then, after one of them are closed, GetEvent apparently references deallocated memory.

This makes us suspect there is a resource shared between RenderWindow (or perhaps Window?) instances, and this shared resource has been incorrectly deallocated, perhaps because of incorrect window count or something like it.

Are there any such data used by f.i. RenderWindow, Input, or any other part of SFML you might suspect have any bearing on the situation?

Further, might upgrading to SFML2 help solving this?

[Laurent]

Are there any such data used by f.i. RenderWindow, Input, or any other part of SFML you might suspect have any bearing on the situation?

Like I already said... no.

Further, might upgrading to SFML2 help solving this?

Since I have no idea of the problem, I really don't know. It can be worth trying.

[PJägare]

The problem remains in SFML2, but using static linking we've been able to gather more information.

The access violation occurs in Window/Win32/WindowImplWin32.cpp, on line 827 (debugger screenshot).

The immediate cause of the problem appears to be that the call GetWindowLongPtr(handle, GWLP_USERDATA) on line 819 returns a pointer to an object that has been deallocated.
On reviewing the code, the WindowImplWin32 destructor seems odd.

Code: [Select]

WindowImplWin32::~WindowImplWin32()
{
    // Destroy the custom icon, if any
    if (myIcon)
        DestroyIcon(myIcon);

    if (!myCallback)
    {
        // Destroy the window
        if (myHandle)
            DestroyWindow(myHandle);

        // Decrement the window count
        WindowCount--;

        // Unregister window class if we were the last window
        if (WindowCount == 0)
        {
            if (HasUnicodeSupport())
            {
                UnregisterClassW(ClassNameW, GetModuleHandle(NULL));
            }
            else
            {
                UnregisterClassA(ClassNameA, GetModuleHandle(NULL));
            }
        }
    }
    else
    {
        // The window is external : remove the hook on its message callback
        SetWindowLongPtr(myHandle, GWLP_WNDPROC, myCallback);
    }
}

Admittedly we're not fully up to speed on the details of window creation and desctruction, but shouldn't the part in the if (!myCallback)-statement always happen, so that DestroyWindow is called?

Alternately, if that is correct, should perhaps SetWindowLongPtr(myHandle, GWLP_USERDATA, 0) be used to deregister the object being destroyed?

[Laurent]

Admittedly we're not fully up to speed on the details of window creation and desctruction, but shouldn't the part in the if (!myCallback)-statement always happen, so that DestroyWindow is called?

When myCallback is not null, it means that the window was created externally (not by SFML -- see the Window::Create(WindowHandle) function), therefore it mustn't be destroyed.

Alternately, if that is correct, should perhaps SetWindowLongPtr(myHandle, GWLP_USERDATA, 0) be used to deregister the object being destroyed?

SetWindowLongPtr assigns custom data to a window. So there's no point assigning something to a window that is going to be destroyed.

I still have no idea about the source of the problem, and without a minimal code to test I'm afraid I won't be able to help you more.

[PJägare]

All right, I understand why DestroyWindow and so forth should not always be called. However, I think you missed my point regarding SetWindowLongPtr.

SetWindowLongPtr assigns custom data to a window. So there's no point assigning something to a window that is going to be destroyed.

In particular, on line 83 of WindowImplWin32.cpp, the WindowImplWin32 being created is assigned as custom data to the provided window handle:

Code: [Select]

WindowImplWin32::WindowImplWin32(WindowHandle handle) :
    //...
{
    if (myHandle)
    {
        //...
        SetWindowLongPtr(myHandle, GWLP_USERDATA, reinterpret_cast<LONG_PTR>(this)); // line 83
        myCallback = SetWindowLongPtr(myHandle, GWLP_WNDPROC, reinterpret_cast<LONG_PTR>(&WindowImplWin32::GlobalOnEvent));
    }
}

or, in the case of non-external windows (which is what we have), on line 815:

Code: [Select]

   // Associate handle and Window instance when the creation message is received
    if (message == WM_CREATE)
    {
        // Get WindowImplWin32 instance (it was passed as the last argument of CreateWindow)
        LONG_PTR window = (LONG_PTR)reinterpret_cast<CREATESTRUCT*>(lParam)->lpCreateParams;

        // Set as the "user data" parameter of the window
        SetWindowLongPtr(handle, GWLP_USERDATA, window); // line 815
    }
But in the destructor this association between the window handle and the WindowImplWin32 object is not broken. The window may be about to be destroyed, but it has not been destroyed yet, and if the window is sent a message before it is destroyed, it will fetch that pointer to the now deallocated WindowImplWin32, and access it. And this does in fact happen.

By adding something like

Code: [Select]

if (myHandle)
    SetWindowLongPtr(myHandle, GWLP_USERDATA, NULL);
to the destructor, this would be protected against. In fact, you have an if-guard on line 822 for that very purpose.

[Laurent]

You can try and see if it solves your problem