After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 144107 - window manager should be killed last when logging out
window manager should be killed last when logging out
Status: RESOLVED OBSOLETE
Product: gnome-session
Classification: Core
Component: general
git master
Other All
: High enhancement
: ---
Assigned To: Session Maintainers
Session Maintainers
: 529036 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2004-06-10 14:34 UTC by Mariano Suárez-Alvarez
Modified: 2021-06-14 18:21 UTC
See Also:
GNOME target: ---
GNOME version: 2.23/2.24


Attachments
Proposed patch (678 bytes, patch)
2006-12-21 17:39 UTC, Christian Neumair
rejected Details | Review
sort list before sending die message (854 bytes, patch)
2007-01-06 05:00 UTC, Tom Tromey
reviewed Details | Review
patch updated to reflect comments (553 bytes, patch)
2007-01-06 18:26 UTC, Tom Tromey
committed Details | Review

Description Mariano Suárez-Alvarez 2004-06-10 14:34:07 UTC
when one logs out of gnome, metacity is killed before other processes, so that
one gets to see all currently opened windows on all workspaces collapse on top
of each other without their decorations and such. That looks pretty like
something broke.

I guess gnome-session does know which process is the window manager, so it can
wait for other process to die before killing it.
Comment 1 Tom Tromey 2006-10-09 23:16:11 UTC
I agree, we should be smarter about this.
We should also be smarter about startup ordering.
Comment 2 Christian Neumair 2006-12-21 17:39:22 UTC
Created attachment 78746 [details] [review]
Proposed patch

This patch ensures that the clients are removed from the session in the opposite order they were added to it (like a stack). I hope the quick hack is readable enough, we could also explicitly iterate over the list, but then the REMOVE statement would look a bit fishy.
Comment 3 Mariano Suárez-Alvarez 2006-12-21 19:57:27 UTC
(You could simply reverse the list instead... (Useless O(n) vs. O(n^2) optimization ;-) )

I can't test the patch, but: doesn't that simply remove the items from the list of clients in the session (as opposed to actually killing them)?

I don't know where the client list is gotten from, but one has to keep in mind that the window manager may have changed since gnome-session started it, so one may need to see who owns the WM_S<screen-numeber> selections (for each screen?) to see what processes are window managers. I think.
Comment 4 Dan Winship 2006-12-21 20:06:58 UTC
(In reply to comment #3)
> I don't know where the client list is gotten from, but one has to keep in mind
> that the window manager may have changed since gnome-session started it, so one
> may need to see who owns the WM_S<screen-numeber> selections (for each screen?)
> to see what processes are window managers. I think.

Ew. Don't do that. Just sort the list in reverse order of _GSM_Priority
property values.
Comment 5 Christian Neumair 2006-12-23 18:35:28 UTC
I'm really not sure how this can be handled properly.

We seem to have three possibilities:

* Remove the clients in the reverse order they were added to the list

* Remove the clients in the reverse priority order, i.e. remove those first that have a higher priority score, which means they are supposed to be started later

* Use/introduce some mechanism that makes us aware about session-launched and user-launched clients, and may also provide info about the purpose of the apps to determine the order and remove the clients that were launched by the user first


The first possibility has the problem that a WM that was launched during the session and appended to the list won't be removed last.

I think it would be a good idea to use the second option as suggested by Dan, and tell WM authors to set the GsmPriority to the smallest possible value.


Marking patch as "rejected".
Comment 6 Tom Tromey 2007-01-06 05:00:08 UTC
Created attachment 79512 [details] [review]
sort list before sending die message

Please try this patch.  It reverse sorts the list of clients before
sending the 'die' messages.
Comment 7 Christian Neumair 2007-01-06 13:58:42 UTC
Tom: I'd prefer

save_finished_list = g_slist_reverse (g_slist_sort (save_finished_list, compare_priority));

It doesn't add so much clutter. Thanks for looking into it, though :).
Comment 8 Tom Tromey 2007-01-06 18:26:00 UTC
Created attachment 79546 [details] [review]
patch updated to reflect comments

How about this one?
Comment 9 Christian Neumair 2007-01-06 19:20:41 UTC
Great! I've mailed the desktop development mailing list. Now we just need a maintainer to approve it :). Setting version/priority/severity information.
Comment 10 Vincent Untz 2007-01-07 02:30:07 UTC
Tom: if it works as expected, just go ahead and commit :-)
Comment 11 Christian Kirbach 2007-01-08 23:10:01 UTC
This sounds really cool. I've tested the patch, but it does not always work, still leaving some windows "naked" (i.e. without decoration) on logout.

Anyways, the behaviour is better than the one we currently have.

From a brief look at the code: 

 SmsDie (client->connection)

is being run on all apps in the list. Is that a blocking call?
If not, we might end up that metacity shuts down before the last application and its window(s) vanishes.
Comment 12 Tom Tromey 2007-01-09 02:25:31 UTC
I checked in my patch.

SmsDie is blocking on the server side, but of course clients
may take different amounts of time to respond to a message.

The only way to make shutdown really synchronous would be to 
send SmsDie and then wait for the corresponding channel to be
closed.

I suppose this could be done if there is a timeout.
But I'm concerned that this would make the overall shutdown
slower.  Maybe a small timeout would work.

This is related to the PR about startup ordering.
Comment 13 Jakub 'Livio' Rusinek 2008-04-04 16:02:47 UTC
Change the version to 2.22.0.
Comment 14 Dan Winship 2008-04-04 16:38:55 UTC
Even if we blocked on SmsDie calls, which would, as Tom mentioned, be really horribly slow, there's still going to be a problem with non-session-managed apps, which won't be told to Die at all, before or after the window manager.

At this point, I think the right fix is that metacity should just ignore the "Die" request from the SM, and keep running until it's killed by losing its X connection. That ensures it will not exit while any other app is visible.

Right?
Comment 15 Dan Winship 2008-04-20 14:03:15 UTC
*** Bug 529036 has been marked as a duplicate of this bug. ***
Comment 16 Dan Winship 2008-04-20 14:16:02 UTC
(In reply to comment #14)
> At this point, I think the right fix is that metacity should just ignore the
> "Die" request from the SM, and keep running until it's killed by losing its X
> connection. That ensures it will not exit while any other app is visible.
> 
> Right?

Wrong. I changed my mind on this one. If the SM tells an XSMP client to Die, the client should die. Also, we want the right thing to happen if the user is using a different window manager as well.

One fix would be for gnome-session to just not kill apps in the Initialization and WindowManager (and Panel?) phases, and let them be killed by X exiting instead. But then, that would probably interfere with bug 522017 (making g-s-d exit cleanly). But we could use the suggestion from there of having a d-bus signal...

So in that case, logout would look like this:

    - User selects "Logout", clicks "OK", whatever
    - gnome-session tells all post-WM-phase apps to Die
    - XSMP client apps exit. Non-XSMP apps are still running, but so are
      metacity and gnome-settings-daemon, so everything still looks ok.
    - gnome-session emits a "logging-out" d-bus signal
    - gnome-settings-daemon gets the d-bus signal, and tells some of its
      child processes (like esd) to exit, but *does not exit itself*
    - gnome-session exits
    - gdm bounces the X connection, causing metacity, gnome-settings-daemon,
      and all the non-XSMP apps to die simultaneously
Comment 17 Jakub 'Livio' Rusinek 2008-04-20 17:43:28 UTC
everywhere you said "metacity" insert "window manager" instead, and everywhere you said "esd" put "pulseaudio", to be modern :> .

PS: please change the summary appropriately and change the version of gnome-session and gnome itself.
Comment 18 Dan Winship 2008-04-20 18:07:21 UTC
(In reply to comment #17)
> everywhere you said "metacity" insert "window manager" instead, and everywhere
> you said "esd" put "pulseaudio", to be modern :> .

actually, pulseaudio doesn't have the problem, only esd does :)
Comment 19 Jakub 'Livio' Rusinek 2008-04-20 20:21:57 UTC
pulseaudio is killed until end of sound is reached.
Comment 20 André Klapper 2021-06-14 18:21:07 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version of gnome-session, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new ticket at
  https://gitlab.gnome.org/GNOME/gnome-session/-/issues/

Thank you for your understanding and your help.