Created attachment 190386 [details] [review]
compositor: Loop and retry to get compositor selection when replacing

Review of attachment 190386 [details] [review]:

FWIW, works as advertised.

::: src/compositor/compositor.c
@@ +509,1 @@
     }

You're introducing some trailing white space on this while block. I don't know what's the mutter rules on this though.

Attachment 190386 [details] pushed as c18940a - compositor: Loop and retry to get compositor selection when replacing

(In reply to comment #2)
> 
> You're introducing some trailing white space on this while block. I don't know
> what's the mutter rules on this though.

Pushed with trailing space deleted.

What was this?

(In reply to comment #5)
> What was this?

There are 3 exclusive locks in gnome-shell --replace:

1) org.gnome.Shell DBus name
2) WM X selection
3) XCompositeRedirectSubwindows(rootwin)

The old process cooperated on giving up 1, no cooperation is necessary for 2, but for 3, we could easily race to acquire compositing before the old process exited.

And where was this encountered? What problems were people seeing that are now fixed?

(In reply to comment #7)
> And where was this encountered? What problems were people seeing that are now
> fixed?

Running: gnome-shell --replace

If you haven't seen it I'm very surprised.

Note that there is *zero* difference between the exclusive resource of getting structure redirection on the root window and the getting subwindow redirection

In both cases, those aren't locked by the window manager selection, so just force replacing the window manager selection doesn't work - you have to wait for the preceding client to give it up -  See meta_screen_new().

So, since we've waited for the old window manager to give up the selection and destroy the selection window - the actual problem is something different - that we aren't giving up the compositing manager exclusie resource before destroying the selection window - that's the correct solution here.

Now - of course, we can't go back and retroactively make old versions of gnome-shell do things in the right order - so we probably do want something like your patch if the race condition is possible to hit - but we should also fix it right - which means unredirecting windows in meta_compositor_unmanage_screen() - which is currently called at the right time but does nothing.

Created attachment 191057 [details] [review]
Fixes for compositor replacement

* When unmanaging a screen, stop redirecting subwindows explicitly,
  so that we do that before destroying the window manager selection
  window.
* Improve comment in the retry code
* When exiting because the previous compositor couldn't be replaced,
  don't g_error() and drop a core file.

Review of attachment 191057 [details] [review]:

Looks fine to me.  Incidentally in almost every new C project I start, one of the first things I do is create a wrapper for g_printerr () + exit (1).

Maybe put one in main.h called meta_fatal()?

Used meta_fatal() (no need to add.. :-) and pushed.

Attachment 191057 [details] pushed as 66a830f - Fixes for compositor replacement