On Wed Nov 29 17:01:15 2023 +0000, Paul Gofman wrote:
I'm not sure because pretty much nothing is certain in this area, but
if there's any need for additional XSync, it's not because of this MR and it is the case already, and should be added separately. I think now whenever reparent happens (in create_client_window, destroy_whole_window and set_window_visual) there is XSync(gdi_display ) after. After this MR (not exactly the patch introducing detach_client_window, later in the MR when new uses of the function are added) there are paths with XReparentWindow without XSync, so unless I am missing something MR does change things in this regard?
As I don't have evidence that they are required I'm not going to add
them now I'd rather suggest maybe then to keep XSync always after the XReparentWindow (effectively, probably not exactly in detach_client_window or attach_client_window but in the added paths), and then remove those in a separate commit if there is a certainty that it is not needed?
But the current XSync is only there to synchronize between XReparentWindow and XDestroyWindow, which definitely caused crashes (and only because of XDestroyWindow, which still believed client window was a child and destroyed it as well, later causing some GL code using gdi_display to access an invalid client window), not between XReparentWindow and other calls.