GNOME Bugzilla – Bug 726380
Land wayland support
Last modified: 2014-03-20 10:40:24 UTC
This is a placeholder bug to deal with landing jasper's wayland branch.
Created attachment 271955 [details] [review] session: Remove a duplicate definition And move gdm_session_cancel(); somewhere a bit related.
Created attachment 271956 [details] [review] server: Pull out common X server args
Created attachment 271957 [details] [review] server: Force server regeneration off Server regeneration causes Xorg to re-emit the SIGUSR1 signal, which *really* confuses our state machine. Since we also want to control the server lifetime fairly closely, it doesn't really make sense for it to ever trigger, so force it off.
Created attachment 271958 [details] [review] Move gdm_slave_run_script to common code We're going to move the code that runs Init to gdm-server.c and the code that runs PostLogin / PreSession to gdm-session-worker.c
Created attachment 271959 [details] [review] server: Move Init script processing here We still need to have it in the XDMCP chooser slave, since we need to initialize the X server, even if we didn't start it ourselves.
Created attachment 271960 [details] [review] session-worker: Move PostLogin / PreSession / PostSession here Pushing these up to the session worker dramatically simplifies the interface for the slave, which will allow us to kill the slaves as separate binaries. Ideally, in the long-term, these scripts should go away.
Created attachment 271961 [details] [review] Integrate the slaves into the main daemon process For no particular reason, the slave has been kept as a separate process. Integrate this into the main display process by simply making the display keep a handle to a GdmSlave object. To keep the cleanup simple, we won't remove the GdmSlave subtypes yet. A future cleanup should merge the slave functionality into GdmDisplay and its subtypes.
Created attachment 271962 [details] [review] display: Enforce a few invariants The GdmDisplay should only be cleaned up if it's finished or failed.
Created attachment 271963 [details] [review] display: Remove some unnecessary parts from dispose These can't possibly be active and armed if we're disposing.
Created attachment 271964 [details] [review] display: Clean up old signal handlers The signal handlers here are intended to be used if the slave itself tries to stop, for instance if the server or greeter die, and will try to finish the display on its own. Letting the signal handler run again will finish the display *again* after an idle, meaning we have this path: gdm_display_finish gdm_display_unmanage on_server_stopped queue_finish ... idle ... gdm_display_finish gdm_display_unmanage on_server_stopped queue_finish ... This might have been an infinite loop, except for the fact that GDM smartly has code to detect the failure state where the X server starts up and stops sufficiently quickly, and in that case transitions the display to the FAILED state, which causes GdmDisplayStore to unref the last ref on the GdmDisplay. Since the finish is still queued, finish_idle runs, and very quickly discovers that our object is no longer a GdmDisplay, so this very quickly manifests into a: gdm_display_finish: assertion 'GDM_IS_DISPLAY (display)' failed error. Not before poking a hole and corrupting some memory trying to clear the finish_idle_id, however.
Created attachment 271965 [details] [review] manager: Make migrated a special case when starting the session We have three cases for starting a session: 1) We migrate to an existing session. Simply tear down state and switch. 2) We're starting a new session, and it's a traditional Xorg server. Tear down the greeter, and keep the same X server and GdmDisplay, and start a full user session. 3) We're starting a new session, but it's a display server. Keep the greeter around and give it a new seed session, and start a full user session. The last two cases are really the two sides of the same coin, while the migrated case is really a fast-path special case that doesn't involve launching a user session in any case, so use a "goto out;" as a fast return rather than an if/else.
Created attachment 271966 [details] [review] manager: Rename the "user session" or "session" to the "seed session" A greeter environment always has two sessions: the greeter session, and the "user session". The "user session" tracks the login PAM conversation. PAM conversations need to be created without any user, so the name "user session" is a bit of a misnomer. The "user session" is created when the greeter is created, and exists when we're sitting at the username prompt. When the username is entered, we then start a session worker. When the verification conversation is finished, we then log the user in. The same exact GdmSession follows into the user session. Traditionally, the user session is started on the same X server as the greeter, so the GdmDisplay and GdmSlave all go with it. With the new display server management, we need to make the GdmSession follow into the user session created for the login, and then start a *new* user session for the next user that logs in on the same greeter. The name "seed session" tries to capture this. The GdmSession starts out as a little seed, and with a username and password, will one day blossom into a beautiful little user session which will one day live on its own, free from the greeter that gave it life and nourished it. It's the circle of life, and it moves us all.
Created attachment 271967 [details] [review] manager: Don't reuse the same X server when we have a display server Rather than morphing our own X server into the user's session the login screen on the same VT, and start the user session. GNOME will activate its session when ready.
Created attachment 271968 [details] [review] session-worker: Allocate a new VT for sessions with their own display server Ideally, we would simply set XDG_VTNR=auto, but that doesn't work yet, so query the first open VT, and set XDG_VTNR to it when we have a display server.
Created attachment 271969 [details] [review] Add wayland-sessions Add the wayland-sessions directory, as shipped by gdm.
Created attachment 271970 [details] [review] session: Add a simple and naive way to detect Wayland sessions Simply look for "wayland-sessions" as a path element.
Created attachment 271971 [details] [review] session: Wayland sessions bypass Xsession
Created attachment 271972 [details] [review] session: Wayland sessions have their own display server
Created attachment 271973 [details] [review] session: Reset immediately on cancel For some reason, before now, we round-tripped through the GdmManager object when resetting the session instead of doing it directly. The only time we can cancel is when the client calls Cancel();, so just handle that directly.
Comment on attachment 271955 [details] [review] session: Remove a duplicate definition Attachment 271955 [details] pushed as f6f50e7 - session: Remove a duplicate definition
Comment on attachment 271956 [details] [review] server: Pull out common X server args Attachment 271956 [details] pushed as c95e923 - server: Pull out common X server args
Comment on attachment 271957 [details] [review] server: Force server regeneration off Attachment 271957 [details] pushed as db8226e - server: Force server regeneration off
Comment on attachment 271958 [details] [review] Move gdm_slave_run_script to common code Attachment 271958 [details] pushed as fc7d730 - Move gdm_slave_run_script to common code
Created attachment 272202 [details] [review] launch-environment: disconnect session signal handlers when destroyed The launch environment's session can actually live longer than the environment itself. This is because the session creates internals "conversation" objects that take a reference on the session, which sometimes keeps it on life support. This commit makes sure the session signal handlers for the launch environment don't get run after the launch environment is destroyed.
Comment on attachment 272202 [details] [review] launch-environment: disconnect session signal handlers when destroyed Attachment 272202 [details] pushed as 236102c - launch-environment: disconnect session signal handlers when destroyed
Comment on attachment 271961 [details] [review] Integrate the slaves into the main daemon process Attachment 271961 [details] pushed as d7aa724 - Integrate the slaves into the main daemon process
Comment on attachment 271962 [details] [review] display: Enforce a few invariants Attachment 271962 [details] pushed as 9daa892 - display: Enforce a few invariants
Comment on attachment 271963 [details] [review] display: Remove some unnecessary parts from dispose Attachment 271963 [details] pushed as 3d7c84b - display: Remove some unnecessary parts from dispose
Created attachment 272221 [details] [review] manager: update session-id when starting user session In order for session unlocking to work properly, the session-id property of the display object needs to match the session running on the display. The display object's session-id property is bound to the slave's session-id property, so this commit makes sure the slave's session-id is updated to the user's session's id once the session is started.
Comment on attachment 272221 [details] [review] manager: update session-id when starting user session Attachment 272221 [details] pushed as 9d3ef3b - manager: update session-id when starting user session
Comment on attachment 271965 [details] [review] manager: Make migrated a special case when starting the session Attachment 271965 [details] pushed as b28bc6c - manager: Make migrated a special case when starting the session
Comment on attachment 271966 [details] [review] manager: Rename the "user session" or "session" to the "seed session" Attachment 271966 [details] pushed as b04ac2a - manager: Rename the "user session" or "session" to the "seed session"
Comment on attachment 271967 [details] [review] manager: Don't reuse the same X server when we have a display server Attachment 271967 [details] pushed as 72ccb27 - manager: Don't reuse the same X server when we have a display server
Review of attachment 271973 [details] [review]: i don't think this is right, if you have a session worker managing a sub-session for reauthentication, then we need to know when the user clicks cancel at the unlock screen, right? Well, at a minimum if the cancelled signal is going away the code to connect to it in gdm-session-worker.c needs to get removed.
(holding off on the last few for a bit after some discussion with Jasper)
Created attachment 272232 [details] [review] session: Introduce the concept of the "session display mode" The session display mode describes exactly how the worker environment will set up VTs, and whether the greeter X server will be reused for the user session.
Created attachment 272233 [details] [review] manager: Don't reuse the same X server for alternate display modes GNOME is going to need a mode of operation in GDM where the session manages its own display server. In this mode of operation, we won't morph the login screen into a user session by reusing the same X server. Instead, we reset the login screen to prepare it for a future login, and then let GNOME or the session worker activate its own session. This commit adds the prerequisite work needed to gdm-manager.c, but only as dead code, since the way to "turn it on" is stubbed out to always reuse the existing server.
Created attachment 272234 [details] [review] session-worker: Implement support for the different display server modes
Created attachment 272235 [details] [review] Add wayland-sessions Add the wayland-sessions directory, as shipped by gdm.
Created attachment 272236 [details] [review] session: Add a simple and naive way to detect Wayland sessions Simply look for "wayland-sessions" as a path element.
Created attachment 272237 [details] [review] session: Wayland sessions bypass Xsession
Created attachment 272238 [details] [review] session: Wayland sessions are allocated on a new VT ... at least for now, until we land logind integration in mutter and turn on the Xorg logind-aware codepath in gdm.
Created attachment 272239 [details] [review] session: Reset immediately on cancel For some reason, before now, we round-tripped through the GdmManager object when resetting the session instead of doing it directly. The only time we can cancel is when the client calls Cancel();, so just handle that directly.
Review of attachment 272239 [details] [review]: not intended for landing, pushed to branch by mistake
Comment on attachment 272232 [details] [review] session: Introduce the concept of the "session display mode" Attachment 272232 [details] pushed as 4313a01 - session: Introduce the concept of the "session display mode"
Comment on attachment 272233 [details] [review] manager: Don't reuse the same X server for alternate display modes Attachment 272233 [details] pushed as 33225d4 - manager: Don't reuse the same X server for alternate display modes
Comment on attachment 272234 [details] [review] session-worker: Implement support for the different display server modes Attachment 272234 [details] pushed as 505b817 - session-worker: Implement support for the different display server modes
Comment on attachment 272235 [details] [review] Add wayland-sessions Attachment 272235 [details] pushed as 01f402e - Add wayland-sessions
Comment on attachment 272236 [details] [review] session: Add a simple and naive way to detect Wayland sessions Attachment 272236 [details] pushed as 83ba197 - session: Add a simple and naive way to detect Wayland sessions
Comment on attachment 272237 [details] [review] session: Wayland sessions bypass Xsession Attachment 272237 [details] pushed as c1cc21a - session: Wayland sessions bypass Xsession
i'm going to go ahead and close this bug out, any follow up work we can do in a new bug. Attachment 272238 [details] pushed as 9808164 - session: Wayland sessions are allocated on a new VT
Reopened for the FreeBSD build failures related to the unconditional inclusion of (non-standard) <sys/vt.h>.
Created attachment 272429 [details] [review] wayland: make optional for FreeBSD FreeBSD build broke, so this commit makes wayland a configure time argument
Review of attachment 272429 [details] [review]: ::: configure.ac @@ +330,3 @@ AC_CHECK_FUNCS(_NSGetEnviron) +AC_CHECK_HEADERS(sys/vt.h) ignore this line ^ it's from an aborted attempt to fix this in a different way.
Attachment 272429 [details] pushed as 80f6ebe - wayland: make optional for FreeBSD
Created attachment 272442 [details] [review] session worker: fix one more _MODE_NEW_VT case The previous patch missed one case of GDM_SESSION_DISPLAY_MODE_NEW_VT, resulting in an undefined reference to jump_to_vt().