RCORE-1982 Opening realm with cached user while offline results in fatal error and session does not retry connection

CHANGELOG.md

@@ -10,6 +10,7 @@
 * Fix a spurious crash related to opening a Realm on background thread while the process was in the middle of exiting ([#7420](https://github.com/realm/realm-core/issues/7420jj))
 * Fix a data race in change notification delivery when running at debug log level ([PR #7402](https://github.com/realm/realm-core/pull/7402), since v14.0.0).
 * Fix a 10-15% performance regression when reading data from the Realm resulting from Obj being made a non-trivial type ([PR #7402](https://github.com/realm/realm-core/pull/7402), since v14.0.0).
+* Fix Opening realm with cached user while offline results in fatal error and session does not retry connection. ([#7349](https://github.com/realm/realm-core/issues/7349), since v13.26.0)
 
 ### Breaking changes
 * Remove `realm_scheduler_set_default_factory()` and `realm_scheduler_has_default_factory()`, and change the `Scheduler` factory function to a bare function pointer rather than a `UniqueFunction` so that it does not have a non-trivial destructor.

src/realm.h

@@ -88,6 +88,9 @@ typedef bool (*realm_migration_func_t)(realm_userdata_t userdata, realm_t* old_r
 typedef bool (*realm_data_initialization_func_t)(realm_userdata_t userdata, realm_t* realm);
 typedef bool (*realm_should_compact_on_launch_func_t)(realm_userdata_t userdata, uint64_t total_bytes,
                                                       uint64_t used_bytes);
+
+RLM_API const char* RLM_DEFAULT_BASE_URL;
+
 typedef enum realm_schema_mode {
     RLM_SCHEMA_MODE_AUTOMATIC,
     RLM_SCHEMA_MODE_IMMUTABLE,

src/realm/object-store/c_api/app.cpp

@@ -125,6 +125,8 @@ static inline bson::BsonArray parse_ejson_array(const char* serialized)
     }
 }
 
+const char* RLM_DEFAULT_BASE_URL = App::default_base_url.data();
+
 RLM_API realm_app_credentials_t* realm_app_credentials_new_anonymous(bool reuse_credentials) noexcept
 {
     return new realm_app_credentials_t(AppCredentials::anonymous(reuse_credentials));

src/realm/object-store/sync/app.cpp

@@ -167,7 +167,6 @@ UniqueFunction<void(const Response&)> handle_default_response(UniqueFunction<voi
     };
 }
 
-constexpr static std::string_view s_default_base_url = "https://realm.mongodb.com";
 constexpr static std::string_view s_base_path = "/api/client/v2.0";
 constexpr static std::string_view s_app_path = "/app";
 constexpr static std::string_view s_auth_path = "/auth";
@@ -184,6 +183,8 @@ std::mutex s_apps_mutex;
 namespace realm {
 namespace app {
 
+std::string_view App::default_base_url = "https://realm.mongodb.com";
+
 App::Config::DeviceInfo::DeviceInfo()
     : platform(util::get_library_platform())
     , cpu_arch(util::get_library_cpu_arch())
@@ -215,7 +216,7 @@ SharedApp App::get_app(CacheMode mode, const Config& config,
 {
     if (mode == CacheMode::Enabled) {
         std::lock_guard<std::mutex> lock(s_apps_mutex);
-        auto& app = s_apps_cache[config.app_id][config.base_url.value_or(std::string(s_default_base_url))];
+        auto& app = s_apps_cache[config.app_id][config.base_url.value_or(std::string(default_base_url))];
         if (!app) {
             app = std::make_shared<App>(Private(), config);
             app->configure(sync_client_config);
@@ -261,7 +262,7 @@ void App::close_all_sync_sessions()
 
 App::App(Private, const Config& config)
     : m_config(config)
-    , m_base_url(m_config.base_url.value_or(std::string(s_default_base_url)))
+    , m_base_url(m_config.base_url.value_or(std::string(default_base_url)))
     , m_location_updated(false)
     , m_request_timeout_ms(m_config.default_request_timeout_ms.value_or(s_default_timeout_ms))
 {
@@ -301,16 +302,21 @@ App::~App() {}
 
 void App::configure(const SyncClientConfig& sync_client_config)
 {
+    std::string ws_route;
     {
         util::CheckedLockGuard guard(m_route_mutex);
         // Make sure to request the location when the app is configured
         m_location_updated = false;
+        // Create a tentative sync route using the generated ws_host_url
+        REALM_ASSERT(!m_ws_host_url.empty());
+        ws_route = make_sync_route();
     }
 
-    // Start with an empty sync route in the sync manager. It will ensure the
-    // location has been updated at least once when the first sync session is
-    // started by requesting a new access token.
-    m_sync_manager = SyncManager::create(shared_from_this(), {}, sync_client_config, config().app_id);
+    // When App starts, the ws_host_url will be populated with the generated value based on
+    // the provided host_url value and the sync route will be created using this. If this is
+    // is incorrect, the websocket connection will fail and the SyncSession will request a
+    // new access token, which will update the location if it has not already.
+    m_sync_manager = SyncManager::create(shared_from_this(), ws_route, sync_client_config, config().app_id);
 }
 
 bool App::init_logger()
@@ -384,23 +390,54 @@ void App::configure_route(const std::string& host_url, const std::optional<std::
     if (ws_host_url && ws_host_url->length() > 0) {
         m_ws_host_url = *ws_host_url;
     }
-    // Otherwise, convert the host url to a websocket host url (http[s]:// -> ws[s]://)
+    // Otherwise, convert the host url to a websocket host url
     else {
-        m_ws_host_url = m_host_url;
-        if (m_ws_host_url.find("http") == 0) {
-            m_ws_host_url.replace(0, 4, "ws");
-        }
+        m_ws_host_url = App::create_ws_host_url(m_host_url);
     }
 
     // host_url is the url to the server: e.g., https://realm.mongodb.com or https://localhost:9090
-    // base_route is the baseline client api path: e.g. https://realm.mongodb.com/api/client/v2.0
+    // base_route is the baseline client api path: e.g. <host_url>/api/client/v2.0
     m_base_route = util::format("%1%2", m_host_url, s_base_path);
-    // app_route is the cloud app URL: https://realm.mongodb.com/api/client/v2.0/app/<app_id>
+    // app_route is the cloud app URL: <host_url>/api/client/v2.0/app/<app_id>
     m_app_route = util::format("%1%2/%3", m_base_route, s_app_path, m_config.app_id);
-    // auth_route is cloud app auth URL: https://realm.mongodb.com/api/client/v2.0/app/<app_id>/auth
+    // auth_route is cloud app auth URL: <host_url>/api/client/v2.0/app/<app_id>/auth
     m_auth_route = util::format("%1%2", m_app_route, s_auth_path);
 }
 
+// Create a temporary websocket URL domain using the given host URL
+// This updates the URL based on the following assumptions:
+// If the URL doesn't start with 'http' => <host-url>
+// http[s]://[region-prefix]realm.mongodb.com => ws[s]://ws.[region-prefix]realm.mongodb.com
+// http[s]://[region-prefix]services.cloud.mongodb.com => ws[s]://[region-prefix].ws.services.cloud.mongodb.com
+// All others => http[s]://<host-url> => ws[s]://<host-url>
+std::string App::create_ws_host_url(const std::string_view& host_url)
+{
+    constexpr static std::string_view s_orig_base_domain = "realm.mongodb.com";
+    constexpr static std::string_view s_new_base_domain = "services.cloud.mongodb.com";
+
+    // Doesn't start with http, just return provided string
+    if (host_url.substr(0, 4) != "http") {
+        return std::string(host_url);
+    }
+    // If it starts with 'https' then ws url will start with wss
+    bool https = host_url[4] == 's';
+    size_t prefix_len = std::char_traits<char>::length("http://") + (https ? 1 : 0);
+    std::string_view prefix = https ? "wss://" : "ws://";
+
+    // http[s]://[region-prefix]realm.mongodb.com => ws[s]://ws.[region-prefix]realm.mongodb.com
+    if (host_url.find(s_orig_base_domain) != std::string_view::npos) {
+        return util::format("%1ws.%2", prefix, host_url.substr(prefix_len));
+    }
+    // http[s]://[region-prefix]services.cloud.mongodb.com => ws[s]://[region-prefix].ws.services.cloud.mongodb.com
+    if (auto start = host_url.find(s_new_base_domain); start != std::string_view::npos) {
+        return util::format("%1%2ws.%3", prefix, host_url.substr(prefix_len, start - prefix_len),
+                            host_url.substr(start));
+    }
+
+    // All others => http[s]://<host-url> => ws[s]://<host-url>
+    return util::format("ws%1", host_url.substr(4));
+}
+
 void App::update_hostname(const std::string& host_url, const std::optional<std::string>& ws_host_url,
                           const std::optional<std::string>& new_base_url)
 {
@@ -612,11 +649,11 @@ std::string App::get_base_url() const
 
 void App::update_base_url(std::optional<std::string> base_url, UniqueFunction<void(Optional<AppError>)>&& completion)
 {
-    std::string new_base_url = base_url.value_or(std::string(s_default_base_url));
+    std::string new_base_url = base_url.value_or(std::string(default_base_url));
 
     if (new_base_url.empty()) {
         // Treat an empty string the same as requesting the default base url
-        new_base_url = s_default_base_url;
+        new_base_url = default_base_url;
         log_debug("App::update_base_url: empty => %1", new_base_url);
     }
     else {
@@ -1023,8 +1060,6 @@ std::optional<AppError> App::update_location(const Response& response, const std
         return error;
     }
 
-    REALM_ASSERT(m_sync_manager); // Need a valid sync manager
-
     // Update the location info with the data from the response
     try {
         auto json = parse<BsonDocument>(response.body);
@@ -1038,8 +1073,10 @@ std::optional<AppError> App::update_location(const Response& response, const std
             // Update the local hostname and path information
             update_hostname(hostname, ws_hostname, base_url);
             m_location_updated = true;
-            // Provide the Device Sync websocket route to the SyncManager
-            m_sync_manager->set_sync_route(make_sync_route());
+            if (m_sync_manager) {
+                // Provide the Device Sync websocket route to the SyncManager
+                m_sync_manager->set_sync_route(make_sync_route());
+            }
         }
     }
     catch (const AppError& ex) {

src/realm/object-store/sync/app.hpp

@@ -90,6 +90,8 @@ class App : public std::enable_shared_from_this<App>,
         DeviceInfo device_info;
     };
 
+    static std::string_view default_base_url;
+
     // `enable_shared_from_this` is unsafe with public constructors;
     // use `App::get_app()` instead
     explicit App(Private, const Config& config);
@@ -432,6 +434,8 @@ class App : public std::enable_shared_from_this<App>,
     // Return the base url path used for Sync Session Websocket requests
     std::string get_ws_host_url() REQUIRES(!m_route_mutex);
 
+    static std::string create_ws_host_url(const std::string_view& host_url);
+
 private:
     const Config m_config;
 

src/realm/object-store/sync/sync_manager.cpp

@@ -43,13 +43,13 @@ SyncClientTimeouts::SyncClientTimeouts()
 {
 }
 
-std::shared_ptr<SyncManager> SyncManager::create(std::shared_ptr<app::App> app, std::optional<std::string> sync_route,
+std::shared_ptr<SyncManager> SyncManager::create(std::shared_ptr<app::App> app, std::string sync_route,
                                                  const SyncClientConfig& config, const std::string& app_id)
 {
-    return std::make_shared<SyncManager>(Private(), std::move(app), std::move(sync_route), config, app_id);
+    return std::make_shared<SyncManager>(Private(), std::move(app), sync_route, config, app_id);
 }
 
-SyncManager::SyncManager(Private, std::shared_ptr<app::App> app, std::optional<std::string> sync_route,
+SyncManager::SyncManager(Private, std::shared_ptr<app::App> app, std::string sync_route,
                          const SyncClientConfig& config, const std::string& app_id)
     : m_config(config)
     , m_file_manager(std::make_unique<SyncFileManager>(m_config.base_file_path, app_id))
@@ -150,7 +150,6 @@ void SyncManager::tear_down_for_testing()
         // Destroy the client now that we have no remaining sessions.
         m_sync_client = nullptr;
         m_logger_ptr.reset();
-        m_sync_route.reset();
     }
 
     {
@@ -666,6 +665,36 @@ void SyncManager::close_all_sessions()
     get_sync_client().wait_for_session_terminations();
 }
 
+void SyncManager::set_sync_route(std::string sync_route, bool verified, RestartSessions restart_sessions)
+{
+    REALM_ASSERT(!sync_route.empty()); // Cannot be set to empty string
+    {
+        util::CheckedLockGuard lock(m_mutex);
+        m_sync_route = sync_route;
+        m_sync_route_verified = verified;
+    }
+
+    // Bail out early if not restarting the existing sessions
+    if (!restart_sessions) {
+        return;
+    }
+
+    std::vector<std::shared_ptr<SyncSession>> sessions;
+    {
+        util::CheckedLockGuard lk(m_session_mutex);
+        // Make a copy of the session ptrs
+        sessions.reserve(m_sessions.size());
+        for (auto& session : m_sessions) {
+            sessions.push_back(session.second);
+        }
+    }
+
+    // Restart the sessions that are currently active
+    for (auto& session : sessions) {
+        session->restart_session();
+    }
+}
+
 void SyncManager::OnlyForTesting::voluntary_disconnect_all_connections(SyncManager& mgr)
 {
     mgr.get_sync_client().voluntary_disconnect_all_connections();

src/realm/object-store/sync/sync_manager.hpp

@@ -222,23 +222,23 @@ class SyncManager : public std::enable_shared_from_this<SyncManager> {
     // Immediately closes any open sync sessions for this sync manager
     void close_all_sessions() REQUIRES(!m_mutex, !m_session_mutex);
 
+    struct RestartSessionsTag {};
+
+    using RestartSessions = util::TaggedBool<RestartSessionsTag>;
     // Used by App to update the sync route any time the location info has been refreshed.
-    // m_sync_route starts out as unset when the SyncManager is created or configured.
-    // It will be updated to a valid value upon the first App AppServices HTTP request or
-    // the access token will be refreshed (forcing a location update) when a SyncSession
-    // is activated and it is still unset. This value is not allowed to be reset to
-    // nullopt once it has a valid value.
-    void set_sync_route(std::string sync_route) REQUIRES(!m_mutex)
-    {
-        REALM_ASSERT(!sync_route.empty());
-        util::CheckedLockGuard lock(m_mutex);
-        m_sync_route = std::move(sync_route);
-    }
+    // m_sync_route starts out as a generated value based on the configured base_url when
+    // the SyncManager is created by App. If this is incorrect, the websocket connection
+    // will fail, resulting in an update to the access token (and the location, if it hasn't
+    // been updated yet). If the sync route is being manually set without being validated,
+    // then set validated to false.
+    void set_sync_route(std::string sync_route, bool verified = true,
+                        RestartSessions restart_sessions = RestartSessions{true})
+        REQUIRES(!m_mutex, !m_session_mutex);
 
-    const std::optional<std::string> sync_route() const REQUIRES(!m_mutex)
+    std::pair<const std::string, bool> sync_route() REQUIRES(!m_mutex)
     {
         util::CheckedLockGuard lock(m_mutex);
-        return m_sync_route;
+        return std::make_pair(m_sync_route, m_sync_route_verified);
     }
 
     std::weak_ptr<app::App> app() const REQUIRES(!m_mutex)
@@ -267,10 +267,10 @@ class SyncManager : public std::enable_shared_from_this<SyncManager> {
         static void voluntary_disconnect_all_connections(SyncManager&);
     };
 
-    static std::shared_ptr<SyncManager> create(std::shared_ptr<app::App> app, std::optional<std::string> sync_route,
+    static std::shared_ptr<SyncManager> create(std::shared_ptr<app::App> app, std::string sync_route,
                                                const SyncClientConfig& config, const std::string& app_id);
-    SyncManager(Private, std::shared_ptr<app::App> app, std::optional<std::string> sync_route,
-                const SyncClientConfig& config, const std::string& app_id);
+    SyncManager(Private, std::shared_ptr<app::App> app, std::string sync_route, const SyncClientConfig& config,
+                const std::string& app_id);
 
 private:
     friend class app::App;
@@ -328,9 +328,11 @@ class SyncManager : public std::enable_shared_from_this<SyncManager> {
     // Callers of this method should hold the `m_session_mutex` themselves.
     bool do_has_existing_sessions() REQUIRES(m_session_mutex);
 
-    // The sync route URL to connect to the server. This can be initially empty, but should not
-    // be cleared once it has been set to a value, except by `tear_down_for_testing()`.
-    std::optional<std::string> m_sync_route GUARDED_BY(m_mutex);
+    // The sync route URL for the sync connection to the server.
+    std::string m_sync_route GUARDED_BY(m_mutex);
+    // If true, then the sync route has been verified by querying the location info or successfully
+    // connecting to the server.
+    bool m_sync_route_verified GUARDED_BY(m_mutex) = false;
 
     std::weak_ptr<app::App> m_app GUARDED_BY(m_mutex);
     const std::string m_app_id;