GNOME Bugzilla – Bug 563640
In all phases before APPLICATION, consider process termination as completion
Last modified: 2009-01-06 02:14:07 UTC
Most clients register using the session protocol. But it's perfectly fine if a client simply exits upon completion, or forks and exits in the parent when ready. This is indeed how we were treating phase INITIALIZATION. There is no reason to not treat other non-APPLICATION phases that way. The default clients for those phases (gnome-panel, metacity, nautilus) register to the session, but for example, libcanberra installed a script for phase DESKTOP to play login sound, and since it doesn't connect to the session, gnome-session was timeout'ing for that phase.
Created attachment 124315 [details] [review] The patch Oops, seems like I forgot to attach.
Seems reasonable. Is the change to app_registered actually needed though? It seems like it should be a no-op. (I'm guessing you added that before moving the "exited" signal handler registration inside the "if phase < ..." ?) (Disclaimer: IANAMaintainer)
It's needed, in case the app that terminated was from a previous phase. Say, a previous phase timed out, or the app had registered by other means.
The change to app_registered doesn't look right to me. What you actually want to check is whether the registered app belongs to current manager phase or not. If so, check if there are no other pending apps so that you move on to the next phase.
(In reply to comment #4) > The change to app_registered doesn't look right to me. What you actually want > to check is whether the registered app belongs to current manager phase or not. > If so, check if there are no other pending apps so that you move on to the next > phase. That's exactly what my code does. Note that the pending list only has list of pending apps for current phase. So, if it wasn't empty before, but is empty after removing the registered app, this means it was the last pending app of the current phase.
(In reply to comment #5) > (In reply to comment #4) > > The change to app_registered doesn't look right to me. What you actually want > > to check is whether the registered app belongs to current manager phase or not. > > If so, check if there are no other pending apps so that you move on to the next > > phase. > > That's exactly what my code does. Note that the pending list only has list of > pending apps for current phase. So, if it wasn't empty before, but is empty > after removing the registered app, this means it was the last pending app of > the current phase. A more explicit/correct way to do that would be to match manager->priv->phase with GsmApp's "phase" property.
Thinking about it again, Dan was right. The check is not needed indeed.
Yeah, Dan is right. All handlers for exited/registered are disconnected on phase timeout. No need to check signals coming from apps from a previous phase. Commited in trunk without the changes in app_registered. Thanks!