SystemServer: Make decision on whether to enable a service more readable

This change ensures that code in the Service class doesn't try to check
the g_system_mode variable, but instead is asked on whether it supports
a given system mode string value.

Also, don't assume that we should create sockets for any new Service
instance, but instead do that only if the Service should run in the
current system mode.

Userland/Services/SystemServer/Service.cpp

@@ -359,16 +359,13 @@ Service::Service(Core::ConfigFile const& config, StringView name)
 
 ErrorOr<NonnullRefPtr<Service>> Service::try_create(Core::ConfigFile const& config, StringView name)
 {
-    auto service = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Service(config, name)));
+    return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Service(config, name)));
-    if (service->is_enabled())
-        TRY(service->setup_sockets());
-    return service;
 }
 
-bool Service::is_enabled() const
+bool Service::is_enabled_for_system_mode(StringView mode) const
 {
     extern DeprecatedString g_system_mode;
-    return m_system_modes.contains_slow(g_system_mode);
+    return m_system_modes.contains_slow(mode);
 }
 
 ErrorOr<void> Service::determine_account(int fd)

Userland/Services/SystemServer/Service.h

@@ -20,11 +20,13 @@ public:
     static ErrorOr<NonnullRefPtr<Service>> try_create(Core::ConfigFile const& config, StringView name);
     ~Service();
 
-    bool is_enabled() const;
+    bool is_enabled_for_system_mode(StringView) const;
     ErrorOr<void> activate();
     // Note: This is a `status` as in POSIX's wait syscall, not an exit-code.
     ErrorOr<void> did_exit(int status);
 
+    ErrorOr<void> setup_sockets();
+
     static Service* find_by_pid(pid_t);
 
 private:
@@ -90,7 +92,6 @@ private:
     int m_restart_attempts { 0 };
 
     ErrorOr<void> setup_socket(SocketDescriptor&);
-    ErrorOr<void> setup_sockets();
     void setup_notifier();
     ErrorOr<void> handle_socket_connection();
 };

Userland/Services/SystemServer/main.cpp

@@ -30,7 +30,10 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
-DeprecatedString g_system_mode = "graphical";
+static constexpr StringView text_system_mode = "text"sv;
+static constexpr StringView selftest_system_mode = "self-test"sv;
+static constexpr StringView graphical_system_mode = "graphical"sv;
+DeprecatedString g_system_mode = graphical_system_mode;
 Vector<NonnullRefPtr<Service>> g_services;
 
 // NOTE: This handler ensures that the destructor of g_services is called.
@@ -68,8 +71,8 @@ static ErrorOr<void> determine_system_mode()
 {
     ArmedScopeGuard declare_text_mode_on_failure([&] {
         // Note: Only if the mode is not set to self-test, degrade it to text mode.
-        if (g_system_mode != "self-test")
+        if (g_system_mode != selftest_system_mode)
-            g_system_mode = "text";
+            g_system_mode = text_system_mode;
     });
 
     auto file_or_error = Core::File::open("/sys/kernel/system_mode"sv, Core::File::OpenMode::Read);
@@ -482,6 +485,26 @@ static ErrorOr<void> create_tmp_semaphore_directory()
     return {};
 }
 
+static ErrorOr<void> activate_services(Core::ConfigFile const& config)
+{
+    for (auto const& name : config.groups()) {
+        auto service = TRY(Service::try_create(config, name));
+        if (service->is_enabled_for_system_mode(g_system_mode)) {
+            TRY(service->setup_sockets());
+            g_services.append(move(service));
+        }
+    }
+    // After we've set them all up, activate them!
+    dbgln("Activating {} services...", g_services.size());
+    for (auto& service : g_services) {
+        dbgln_if(SYSTEMSERVER_DEBUG, "Activating {}", service->name());
+        if (auto result = service->activate(); result.is_error())
+            dbgln("{}: {}", service->name(), result.release_error());
+    }
+
+    return {};
+}
+
 static ErrorOr<void> reopen_base_file_descriptors()
 {
     // Note: We open the /dev/null device and set file descriptors 0, 1, 2 to it
@@ -498,6 +521,42 @@ static ErrorOr<void> reopen_base_file_descriptors()
     return {};
 }
 
+static ErrorOr<void> activate_base_services_based_on_system_mode()
+{
+    if (g_system_mode == graphical_system_mode) {
+        bool found_gpu_device = false;
+        for (int attempt = 0; attempt < 10; attempt++) {
+            struct stat file_state;
+            int rc = lstat("/dev/gpu/connector0", &file_state);
+            if (rc == 0) {
+                found_gpu_device = true;
+                break;
+            }
+            sleep(1);
+        }
+        if (!found_gpu_device) {
+            dbgln("WARNING: No device nodes at /dev/gpu/ directory after 10 seconds. This is probably a sign of disabled graphics functionality.");
+            dbgln("To cope with this, graphical mode will not be enabled.");
+            g_system_mode = text_system_mode;
+        }
+    }
+
+    // Read our config and instantiate services.
+    // This takes care of setting up sockets.
+    auto config = TRY(Core::ConfigFile::open_for_system("SystemServer"));
+    TRY(activate_services(*config));
+    return {};
+}
+
+static ErrorOr<void> activate_user_services_based_on_system_mode()
+{
+    // Read our config and instantiate services.
+    // This takes care of setting up sockets.
+    auto config = TRY(Core::ConfigFile::open_for_app("SystemServer"));
+    TRY(activate_services(*config));
+    return {};
+}
+
 ErrorOr<int> serenity_main(Main::Arguments arguments)
 {
     bool user = false;
@@ -525,22 +584,10 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     event_loop.register_signal(SIGCHLD, sigchld_handler);
     event_loop.register_signal(SIGTERM, sigterm_handler);
 
-    // Read our config and instantiate services.
+    if (!user) {
-    // This takes care of setting up sockets.
+        TRY(activate_base_services_based_on_system_mode());
-    auto config = (user)
+    } else {
-        ? TRY(Core::ConfigFile::open_for_app("SystemServer"))
+        TRY(activate_user_services_based_on_system_mode());
-        : TRY(Core::ConfigFile::open_for_system("SystemServer"));
-    for (auto const& name : config->groups()) {
-        auto service = TRY(Service::try_create(*config, name));
-        if (service->is_enabled())
-            g_services.append(move(service));
-    }
-
-    // After we've set them all up, activate them!
-    dbgln("Activating {} services...", g_services.size());
-    for (auto& service : g_services) {
-        if (auto result = service->activate(); result.is_error())
-            dbgln("{}: {}", service->name(), result.release_error());
     }
 
     return event_loop.exec();