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 726380 - Land wayland support
Land wayland support
Status: RESOLVED FIXED
Product: gdm
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GDM maintainers
GDM maintainers
Depends on:
Blocks:
 
 
Reported: 2014-03-14 22:42 UTC by Ray Strode [halfline]
Modified: 2014-03-20 10:40 UTC
See Also:
GNOME target: 3.12
GNOME version: ---


Attachments
session: Remove a duplicate definition (4.66 KB, patch)
2014-03-14 22:46 UTC, Ray Strode [halfline]
committed Details | Review
server: Pull out common X server args (3.54 KB, patch)
2014-03-14 22:46 UTC, Ray Strode [halfline]
committed Details | Review
server: Force server regeneration off (2.18 KB, patch)
2014-03-14 22:46 UTC, Ray Strode [halfline]
committed Details | Review
Move gdm_slave_run_script to common code (37.69 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
committed Details | Review
server: Move Init script processing here (5.98 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
committed Details | Review
session-worker: Move PostLogin / PreSession / PostSession here (13.59 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
committed Details | Review
Integrate the slaves into the main daemon process (275.98 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
committed Details | Review
display: Enforce a few invariants (1.96 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
committed Details | Review
display: Remove some unnecessary parts from dispose (2.78 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
committed Details | Review
display: Clean up old signal handlers (3.29 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
committed Details | Review
manager: Make migrated a special case when starting the session (3.80 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
committed Details | Review
manager: Rename the "user session" or "session" to the "seed session" (13.02 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
committed Details | Review
manager: Don't reuse the same X server when we have a display server (8.50 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
committed Details | Review
session-worker: Allocate a new VT for sessions with their own display server (16.60 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
none Details | Review
Add wayland-sessions (3.07 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
none Details | Review
session: Add a simple and naive way to detect Wayland sessions (5.82 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
none Details | Review
session: Wayland sessions bypass Xsession (1.97 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
none Details | Review
session: Wayland sessions have their own display server (1.70 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
none Details | Review
session: Reset immediately on cancel (18.94 KB, patch)
2014-03-14 22:47 UTC, Ray Strode [halfline]
needs-work Details | Review
launch-environment: disconnect session signal handlers when destroyed (6.59 KB, patch)
2014-03-17 20:11 UTC, Ray Strode [halfline]
committed Details | Review
manager: update session-id when starting user session (5.03 KB, patch)
2014-03-18 01:36 UTC, Ray Strode [halfline]
committed Details | Review
session: Introduce the concept of the "session display mode" (8.05 KB, patch)
2014-03-18 04:28 UTC, Ray Strode [halfline]
committed Details | Review
manager: Don't reuse the same X server for alternate display modes (3.77 KB, patch)
2014-03-18 04:29 UTC, Ray Strode [halfline]
committed Details | Review
session-worker: Implement support for the different display server modes (29.41 KB, patch)
2014-03-18 04:29 UTC, Ray Strode [halfline]
committed Details | Review
Add wayland-sessions (3.07 KB, patch)
2014-03-18 04:29 UTC, Ray Strode [halfline]
committed Details | Review
session: Add a simple and naive way to detect Wayland sessions (5.82 KB, patch)
2014-03-18 04:29 UTC, Ray Strode [halfline]
committed Details | Review
session: Wayland sessions bypass Xsession (1.97 KB, patch)
2014-03-18 04:29 UTC, Ray Strode [halfline]
committed Details | Review
session: Wayland sessions are allocated on a new VT (2.10 KB, patch)
2014-03-18 04:29 UTC, Ray Strode [halfline]
committed Details | Review
session: Reset immediately on cancel (18.91 KB, patch)
2014-03-18 04:29 UTC, Ray Strode [halfline]
rejected Details | Review
wayland: make optional for FreeBSD (22.44 KB, patch)
2014-03-19 19:44 UTC, Ray Strode [halfline]
committed Details | Review
session worker: fix one more _MODE_NEW_VT case (1.23 KB, patch)
2014-03-19 22:30 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Ray Strode [halfline] 2014-03-14 22:42:42 UTC
This is a placeholder bug to deal with landing jasper's wayland branch.
Comment 1 Ray Strode [halfline] 2014-03-14 22:46:50 UTC
Created attachment 271955 [details] [review]
session: Remove a duplicate definition

And move gdm_session_cancel(); somewhere a bit related.
Comment 2 Ray Strode [halfline] 2014-03-14 22:46:53 UTC
Created attachment 271956 [details] [review]
server: Pull out common X server args
Comment 3 Ray Strode [halfline] 2014-03-14 22:46:57 UTC
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.
Comment 4 Ray Strode [halfline] 2014-03-14 22:47:01 UTC
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
Comment 5 Ray Strode [halfline] 2014-03-14 22:47:04 UTC
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.
Comment 6 Ray Strode [halfline] 2014-03-14 22:47:07 UTC
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.
Comment 7 Ray Strode [halfline] 2014-03-14 22:47:11 UTC
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.
Comment 8 Ray Strode [halfline] 2014-03-14 22:47:15 UTC
Created attachment 271962 [details] [review]
display: Enforce a few invariants

The GdmDisplay should only be cleaned up if it's finished
or failed.
Comment 9 Ray Strode [halfline] 2014-03-14 22:47:18 UTC
Created attachment 271963 [details] [review]
display: Remove some unnecessary parts from dispose

These can't possibly be active and armed if we're disposing.
Comment 10 Ray Strode [halfline] 2014-03-14 22:47:22 UTC
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.
Comment 11 Ray Strode [halfline] 2014-03-14 22:47:25 UTC
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.
Comment 12 Ray Strode [halfline] 2014-03-14 22:47:29 UTC
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.
Comment 13 Ray Strode [halfline] 2014-03-14 22:47:32 UTC
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.
Comment 14 Ray Strode [halfline] 2014-03-14 22:47:36 UTC
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.
Comment 15 Ray Strode [halfline] 2014-03-14 22:47:40 UTC
Created attachment 271969 [details] [review]
Add wayland-sessions

Add the wayland-sessions directory, as shipped by gdm.
Comment 16 Ray Strode [halfline] 2014-03-14 22:47:43 UTC
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.
Comment 17 Ray Strode [halfline] 2014-03-14 22:47:48 UTC
Created attachment 271971 [details] [review]
session: Wayland sessions bypass Xsession
Comment 18 Ray Strode [halfline] 2014-03-14 22:47:54 UTC
Created attachment 271972 [details] [review]
session: Wayland sessions have their own display server
Comment 19 Ray Strode [halfline] 2014-03-14 22:47:58 UTC
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 20 Ray Strode [halfline] 2014-03-14 22:52:20 UTC
Comment on attachment 271955 [details] [review]
session: Remove a duplicate definition

Attachment 271955 [details] pushed as f6f50e7 - session: Remove a duplicate definition
Comment 21 Ray Strode [halfline] 2014-03-14 22:58:39 UTC
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 22 Ray Strode [halfline] 2014-03-14 23:04:24 UTC
Comment on attachment 271957 [details] [review]
server: Force server regeneration off

Attachment 271957 [details] pushed as db8226e - server: Force server regeneration off
Comment 23 Ray Strode [halfline] 2014-03-15 02:33:51 UTC
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
Comment 24 Ray Strode [halfline] 2014-03-17 20:11:37 UTC
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 25 Ray Strode [halfline] 2014-03-17 20:12:02 UTC
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 26 Ray Strode [halfline] 2014-03-17 20:39:06 UTC
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 27 Ray Strode [halfline] 2014-03-17 20:48:02 UTC
Comment on attachment 271962 [details] [review]
display: Enforce a few invariants

Attachment 271962 [details] pushed as 9daa892 - display: Enforce a few invariants
Comment 28 Ray Strode [halfline] 2014-03-17 20:56:57 UTC
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
Comment 29 Ray Strode [halfline] 2014-03-18 01:36:12 UTC
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 30 Ray Strode [halfline] 2014-03-18 01:37:10 UTC
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 31 Ray Strode [halfline] 2014-03-18 01:45:49 UTC
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 32 Ray Strode [halfline] 2014-03-18 01:49:58 UTC
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 33 Ray Strode [halfline] 2014-03-18 02:00:50 UTC
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
Comment 34 Ray Strode [halfline] 2014-03-18 02:21:11 UTC
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.
Comment 35 Ray Strode [halfline] 2014-03-18 02:22:57 UTC
(holding off on the last few for a bit after some discussion with Jasper)
Comment 36 Ray Strode [halfline] 2014-03-18 04:28:57 UTC
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.
Comment 37 Ray Strode [halfline] 2014-03-18 04:29:02 UTC
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.
Comment 38 Ray Strode [halfline] 2014-03-18 04:29:08 UTC
Created attachment 272234 [details] [review]
session-worker: Implement support for the different display server modes
Comment 39 Ray Strode [halfline] 2014-03-18 04:29:12 UTC
Created attachment 272235 [details] [review]
Add wayland-sessions

Add the wayland-sessions directory, as shipped by gdm.
Comment 40 Ray Strode [halfline] 2014-03-18 04:29:17 UTC
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.
Comment 41 Ray Strode [halfline] 2014-03-18 04:29:23 UTC
Created attachment 272237 [details] [review]
session: Wayland sessions bypass Xsession
Comment 42 Ray Strode [halfline] 2014-03-18 04:29:32 UTC
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.
Comment 43 Ray Strode [halfline] 2014-03-18 04:29:38 UTC
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.
Comment 44 Jasper St. Pierre (not reading bugmail) 2014-03-18 04:32:43 UTC
Review of attachment 272239 [details] [review]:

not intended for landing, pushed to branch by mistake
Comment 45 Ray Strode [halfline] 2014-03-18 04:36:40 UTC
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 46 Ray Strode [halfline] 2014-03-18 04:40:14 UTC
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 47 Ray Strode [halfline] 2014-03-18 05:00:46 UTC
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 48 Ray Strode [halfline] 2014-03-18 20:20:00 UTC
Comment on attachment 272235 [details] [review]
Add wayland-sessions

Attachment 272235 [details] pushed as 01f402e - Add wayland-sessions
Comment 49 Ray Strode [halfline] 2014-03-18 20:26:03 UTC
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 50 Ray Strode [halfline] 2014-03-18 20:27:23 UTC
Comment on attachment 272237 [details] [review]
session: Wayland sessions bypass Xsession

Attachment 272237 [details] pushed as c1cc21a - session: Wayland sessions bypass Xsession
Comment 51 Ray Strode [halfline] 2014-03-19 03:08:35 UTC
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
Comment 52 Allison Karlitskaya (desrt) 2014-03-19 16:02:09 UTC
Reopened for the FreeBSD build failures related to the unconditional inclusion of (non-standard) <sys/vt.h>.
Comment 53 Ray Strode [halfline] 2014-03-19 19:44:25 UTC
Created attachment 272429 [details] [review]
wayland: make optional for FreeBSD

FreeBSD build broke, so this commit makes wayland a configure time
argument
Comment 54 Ray Strode [halfline] 2014-03-19 19:47:20 UTC
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.
Comment 55 Ray Strode [halfline] 2014-03-19 19:52:58 UTC
Attachment 272429 [details] pushed as 80f6ebe - wayland: make optional for FreeBSD
Comment 56 Allison Karlitskaya (desrt) 2014-03-19 22:30:42 UTC
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().