On Wed Nov 29 17:07:41 2023 +0000, Rémi Bernon wrote:
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.
Maybe, but regardless of the motivation, it is de-facto there after each XReparentWindow now. We may have other desync problems besides fatal crashes with XBadWindow. If you ask me now for exact bugs without XSync on the added paths it would be hard to figure out, there were some in Proton version but that was without the referenced earlier upstream changes fixing the sync elsewhere, it might be that those are resolved this way. Usually if we have reasonable doubts if synchronization is needed somewhere we tend to do it and then think on whether / how we can relax it, or if extra sync is especially harmful for performance we want to be sure if it is needed, do we not? Any reason this should be different approach here: in case of doubts assume that it is not needed and then add once the evidence proves otherwise? We are not talking here about any X server operation, only X window tree changes, which are expensive anyway and also potentially sensitive to desync.