/dev/urandom missing? How would that happen?

This assert should be in.. if there is no /dev/urandom or the getrandom() system call is not available.. the system is broken...

I don't see why /dev/urandom would be missing, but I can understand that using assert() for checking file-system paths is wrong. We also don't do things like assert(!access("/run", F_OK)).

@dvdhrm I'd be ok to change from assert to a normal failure, but this patch returns 0 and tries to keep going.

Just to mention this: if the code in question is "library" code then we should always be as permissive and defensive as possible, and not place assert()s. (which is why we have softer logic like assert_return() for library cases). However, in main program code assert()s are fine for error conditions that cannot realistically ever happen, and there's no way to continue.

In this occasion this is main program code (i.e. located in src/core, not in src/shared or so), and /dev/urandom missing cannot really happen and without it we cannot continue, hence an assert() is fine.

That all said maybe I am missing something on @lnykryn's reasoning for the change, please elaborate!

Even then, if it's something the sysadmin can fix (as opposed to an internal error/bug), a "fatal error" message pointing at the broken system would still look better than an assert() crash pointing at the (not broken) code.

This is from our bugzilla and it was "synthetic" bug. Basically the reporter was complaining that if he deletes /dev/urandow, then systemd freeze itself.

For reference: https://bugzilla.redhat.com/show_bug.cgi?id=1232195

Well, there are tons of other ways you can crash the system. It's pointless to play these games.

In https://launchpad.net/bugs/1519499 I got reported a real instance of this bug, with https://launchpadlibrarian.net/228117270/journal.txt being the complete debug log. This happens in a container, apparently /dev/urandom goes away because dev-urandom.mount gets stopped during shutdown. This is a bind mount from the host system.

Running into the same issues with lxc, +1 to reopen this bug please. @poettering

Looks like the change that was commited as 874d340 did not actually fix the issue of not umounting those file systems.
I still see them umounted in a v230 container on shutdown

So, I tried 874d340 on my debian/jessie container with systemd 215-17+deb8u4 and that did indeed not work.

Then I also added this patch, after which things started working like expected:

--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -353,8 +353,16 @@ static int mount_add_quota_links(Mount *
 static bool should_umount(Mount *m) {
         MountParameters *p;

+        /* We do not auto-unmount / and /usr, since they are
+         * guaranteed to stay mounted the whole time, since
+         * our system is on it. Also, don't bother with anything
+         * mounted below virtual file systems, it's also going to be
+         * virtual, and hence not worth the effort. */
         if (path_equal(m->where, "/") ||
-            path_equal(m->where, "/usr"))
+            path_equal(m->where, "/usr") ||
+            path_startswith(m->where, "/proc") ||
+            path_startswith(m->where, "/sys") ||
+            path_startswith(m->where, "/dev"))
                 return false;

         p = get_mount_parameters(m);