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

Miklós Máté mtmkls at gmail.com
Sat Apr 8 16:30:40 CDT 2017


On 05/04/17 19:52, Alexandre Julliard wrote:
> 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?
>
Well, according to my notes from half a year ago the ddraw test of 
wine-csmt definitely crashes without this, but now I can't reproduce it. 
I don't know how likely the above scenario is in the real world.

MM

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20170408/53334477/attachment.html>


More information about the wine-devel mailing list