[PATCH 2/4] winex11: move XDestroyWindow() after free_gl_drawable() in set_win_format()

[Alexandre Julliard]

Miklós Máté <mtmkls at gmail.com> writes:

> On 04/04/17 16:06, Alexandre Julliard wrote:
>> Miklós Máté <mtmkls at gmail.com> writes:
>>
>>> On 04/04/17 12:11, Alexandre Julliard wrote:
>>>> Miklós Máté <mtmkls at gmail.com> writes:
>>>>
>>>>> @@ -1334,21 +1334,21 @@ static void free_gl_drawable( struct gl_drawable *gl )
>>>>>    /***********************************************************************
>>>>>     *              create_gl_drawable
>>>>>     */
>>>>> -static BOOL create_gl_drawable( HWND hwnd, struct gl_drawable *gl )
>>>>> +static BOOL create_gl_drawable( HWND hwnd, struct gl_drawable *gl, struct x11drv_win_data *data )
>>>>>    {
>>>>>        gl->drawable = 0;
>>>>>          if (GetAncestor( hwnd, GA_PARENT ) == GetDesktopWindow())
>>>>> /* top-level window */
>>>>>        {
>>>>> -        struct x11drv_win_data *data = get_win_data( hwnd );
>>>>> -
>>>>>            if (data)
>>>>>            {
>>>>>                gl->type = DC_GL_WINDOW;
>>>>>                gl->window = create_client_window( data, gl->visual );
>>>>>                if (gl->window)
>>>>> +            {
>>>>>                    gl->drawable = pglXCreateWindow( gdi_display, gl->format->fbconfig, gl->window, NULL );
>>>>> -            release_win_data( data );
>>>>> +                XSync( gdi_display, False );
>>>> Do you really need all the extra XSync calls?  We try to avoid server
>>>> round-trips as much as possible.
>>>>
>>> Yes. We must ensure that glXCreateWindow() and glXDestroyWindow()
>>> refer to a valid X window, so the command queue of data->display and
>>> gdi_display have to be in sync around those calls.
>> If the goal is that the X window is valid, I'd have expected an XSync on
>> the thread display. Why is the XSync on gdi_display needed here? What is
>> the scenario that fails?
>>
>> I'm not questioning that some XSyncs are necessary (and we already have
>> a few), but I want to make sure we don't add more than needed.
>>
> This one prevents crash in the following scenario:
>
> 1. create_gl_drawable()
>
> 2. X11DRV_ThreadDetach() calls XCloseDisplay( data->display );
>
> 3. the Xserver processes the glXCreateWindow in the command queue of
> gdi_display -> X window is not valid
>
> This happens sometimes during the ddraw and d3d9 tests.

Shouldn't this be happening even without your other changes then?

-- 
Alexandre Julliard
julliard at winehq.org