3523
|
|
|
Sam Spilsbury |
11 years ago
|
|
|
3522
|
|
|
Sam Spilsbury |
11 years ago
|
|
|
3521
|
|
Ensure that we never get a BadWindow due to the sibling in a ConfigureWindow.
Issuing a ConfigureWindow request with a sibling parameter that refers to a sibling that's actually destroyed server side results in a BadWindow error, and causes what we might think is a correct ConfigureWindow request to fail. This would cause even more problems because we had marked the request as succeeding, and adjusted serverWindows appropriately, which would cause further restacks to rely on the invalid internal state.
Unfortunately, this is effectively a race condition between the client and the server. We receive our events and are racing the server to not destroy the window internally before we get a chance to issue a ConfigureWindow request relative to the destroyed window.
Because of that, we need to hold a server grab during the period in which we check if the sibling window is still valid server side and when we issue a ConfigureWindow request. If it is valid, then we can guaruntee that the server will process the request relative to the sibling window, and if not, we can select another sibling as appropriate.
Adjusted the API such that any function which looks for an appropriate sibling or any function which calls XConfigureWindow with the sibling parameter set requires a server grab to be active (by virtue of the fact that they accept a const ServerLock &).
The only implicit contract between clients now is that the client must obtain the sibling window either by using one of the designated functions to do so which requires a ServerLock, or that the client holds a server lock and while it holds the ServerLock, it calls XGetWindowAttributes or XGetGeometry to check if the sibling window is valid.
configureXWindow was split into configureXWindow and restackAndConfigureXWindow. The latter requires a server lock, the former will warn about this potential race condition if you attempt to restack a window using it.
Passing tests:
[ RUN ] CompizXorgSystemStackingTest.TestCreateRelativeToDestroyedWindowFindsAnotherAppropriatePosition [ OK ] CompizXorgSystemStackingTest.TestCreateRelativeToDestroyedWindowFindsAnotherAppropriatePosition (55 ms)
(LP: #1088399)
|
Sam Spilsbury |
11 years ago
|
|
|
3520
|
|
|
Sam Spilsbury |
11 years ago
|
|
|
3519
|
|
|
Sam Spilsbury |
11 years ago
|
|
|
3518
|
|
|
Sam Spilsbury |
11 years ago
|
|
|
3517
|
|
|
Daniel van Vugt |
11 years ago
|
|
|
3516
|
|
|
Daniel van Vugt |
11 years ago
|
|
|
3515
|
|
|
Didier Roche |
11 years ago
|
|
|
3514
|
|
|
Sam Spilsbury |
11 years ago
|
|
|
3513
|
|
|
Daniel van Vugt |
11 years ago
|
|
|
3512
|
|
|
Sam Spilsbury |
11 years ago
|
|
|
3511
|
|
|
Daniel van Vugt |
11 years ago
|
|
|
3510
|
|
|
Matija Skala |
11 years ago
|
|
|
3509
|
|
|
Jussi Pakkanen |
11 years ago
|
|
|
3508
|
|
|
Daniel van Vugt |
11 years ago
|
|
|
3507
|
|
|
Automatic PS uploade... |
11 years ago
|
|
|
3506
|
|
|
Sam Spilsbury |
11 years ago
|
|
|
3505
|
|
|
Jussi Pakkanen |
11 years ago
|
|
|
3504
|
|
Fix race condition causing --replace to fail occasionally.
Clear XErrors right before taking SubstructureRedirectMask.
This changes the startup order a fair bit. The new order is now:
1. Do non-critical initialization that does not require a server grab 2. Do the ICCCM check to shut down the other window manager 3. Take server grab 4. Create edge windows 5. Clear error buffer 6. Attempt to take SubstructureRedirectMask 7. Clear error buffer -> if fail, exit 8. Create handles based on XQueryTree 9. Release server grab
(LP: #1085591)
System test added to demonstrate that compiz should exit when another client has SubstructureRedirectMask and attempting to take it results in an X error. It wasn't doing that before, and instead checked for X errors much earlier on (in error, because an X error up there wouldn't indicate that another WM was running at all).
In order to make this work properly, some changes had to be made to compiz. This adds a new runtime switch --send-startup-message which simply posts a message to all clients saying that the startup procedure has finished (and either succeeded or failed). Its needed because the location of when we take the server grab was changed to a much smaller critical section of the code, and that doesn't include the property change on the selection window. This is a far more accurate way of knowing that we've started, because we send it /after/ the startup procedure is done (rather than relying on the server to do the right thing.
Also fixed race condition caused by destroying the WMSnSelectionWindow before shutdown.
Because we haven't changed our active event mask to remove SubstructureRedirectMask, other ICCCM compliant window managers may receive a DestroyNotify (eg, because we're blocked on XSync) before we get a chance to close our display connection and remove our SubstructureRedirectMask. That will cause them to fail to start.
The selection window is destroyed anyways when we close our connection, and that is a very accurate indicator to other WM's that we are well and truly gone because the protocol requires the implementation to remove all client event masks before destroying windows.
Added acceptance-tests for testing that behaviour. They are not added to the default test target for obvious reasons. Fixes: https://bugs.launchpad.net/bugs/1085591.
Approved by PS Jenkins bot, Daniel van Vugt.
|
Sam Spilsbury |
11 years ago
|
|
|