Skip to content

Commit

Permalink
Simplify the SessionWrapper lifecycle a bit
Browse files Browse the repository at this point in the history
Initializing a sync::Session was a multi-step process of creating the Session
with a config, configuring some addition things via mutation functions, and
then calling bind(). We can simplify this a bit by pushing everything into the
config struct and binding inside the wrapper's constructor, eliminating the
constructed-but-not-initialized state and letting us make more of the members
const.
  • Loading branch information
tgoyne committed May 29, 2024
1 parent fd05f55 commit 8a68a7f
Show file tree
Hide file tree
Showing 16 changed files with 747 additions and 1,171 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

### Internals
* Work around a bug in VC++ that resulted in runtime errors when running the tests in a debug build (#[7741](https://github.com/realm/realm-core/issues/7741)).
* Refactor `sync::Session` to eliminate the bind() step of session creation ([#7609](https://github.com/realm/realm-core/pull/7609)).
* Add ScopeExitFail which only calls the handler if exiting the scope via an uncaught exception ([#7609](https://github.com/realm/realm-core/pull/7609)).

----------------------------------------------

Expand Down
5 changes: 2 additions & 3 deletions src/realm/db.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1189,10 +1189,9 @@ void DB::open(const std::string& path, const DBOptions& options)
SlabAlloc::DetachGuard alloc_detach_guard(alloc);
alloc.note_reader_start(this);
// must come after the alloc detach guard
auto handler = [this, &alloc]() noexcept {
auto reader_end_guard = make_scope_exit([this, &alloc]() noexcept {
alloc.note_reader_end(this);
};
auto reader_end_guard = make_scope_exit(handler);
});

// Check validity of top array (to give more meaningful errors
// early)
Expand Down
2 changes: 1 addition & 1 deletion src/realm/object-store/sync/impl/sync_client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ struct SyncClient {
std::unique_ptr<sync::Session> make_session(std::shared_ptr<DB> db,
std::shared_ptr<sync::SubscriptionStore> flx_sub_store,
std::shared_ptr<sync::MigrationStore> migration_store,
sync::Session::Config config)
sync::Session::Config&& config)
{
return std::make_unique<sync::Session>(m_client, std::move(db), std::move(flx_sub_store),
std::move(migration_store), std::move(config));
Expand Down
74 changes: 34 additions & 40 deletions src/realm/object-store/sync/sync_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,7 @@ void SyncSession::become_active()
}

// when entering from the Dying state the session will still be bound
if (!m_session) {
create_sync_session();
m_session->bind();
}
create_sync_session();

// Register all the pending wait-for-completion blocks. This can
// potentially add a redundant callback if we're coming from the Dying
Expand Down Expand Up @@ -896,6 +893,8 @@ void SyncSession::create_sync_session()
SyncConfig& sync_config = *m_config.sync_config;
REALM_ASSERT(sync_config.user);

std::weak_ptr<SyncSession> weak_self = weak_from_this();

sync::Session::Config session_config;
session_config.signed_user_token = sync_config.user->access_token();
session_config.user_id = sync_config.user->user_id();
Expand All @@ -912,8 +911,8 @@ void SyncSession::create_sync_session()

if (sync_config.on_sync_client_event_hook) {
session_config.on_sync_client_event_hook = [hook = sync_config.on_sync_client_event_hook,
anchor = weak_from_this()](const SyncClientHookData& data) {
return hook(anchor, data);
weak_self](const SyncClientHookData& data) {
return hook(weak_self, data);
};
}

Expand Down Expand Up @@ -954,46 +953,41 @@ void SyncSession::create_sync_session()
m_server_requests_action = sync::ProtocolErrorInfo::Action::NoAction;
}

m_session = m_client.make_session(m_db, m_flx_subscription_store, m_migration_store, std::move(session_config));

std::weak_ptr<SyncSession> weak_self = weak_from_this();

// Set up the wrapped progress handler callback
m_session->set_progress_handler([weak_self](uint_fast64_t downloaded, uint_fast64_t downloadable,
uint_fast64_t uploaded, uint_fast64_t uploadable,
uint_fast64_t snapshot_version, double download_estimate,
double upload_estimate, int64_t query_version) {
session_config.progress_handler = [weak_self](uint_fast64_t downloaded, uint_fast64_t downloadable,
uint_fast64_t uploaded, uint_fast64_t uploadable,
uint_fast64_t snapshot_version, double download_estimate,
double upload_estimate, int64_t query_version) {
if (auto self = weak_self.lock()) {
self->handle_progress_update(downloaded, downloadable, uploaded, uploadable, snapshot_version,
download_estimate, upload_estimate, query_version);
}
});
};

// Sets up the connection state listener. This callback is used for both reporting errors as well as changes to
// the connection state.
m_session->set_connection_state_change_listener(
[weak_self](sync::ConnectionState state, std::optional<sync::SessionErrorInfo> error) {
using cs = sync::ConnectionState;
ConnectionState new_state = [&] {
switch (state) {
case cs::disconnected:
return ConnectionState::Disconnected;
case cs::connecting:
return ConnectionState::Connecting;
case cs::connected:
return ConnectionState::Connected;
}
REALM_UNREACHABLE();
}();
// If the OS SyncSession object is destroyed, we ignore any events from the underlying Session as there is
// nothing useful we can do with them.
if (auto self = weak_self.lock()) {
self->update_connection_state(new_state);
if (error) {
self->handle_error(std::move(*error));
}
session_config.connection_state_change_listener = [weak_self](sync::ConnectionState state,
std::optional<sync::SessionErrorInfo> error) {
using cs = sync::ConnectionState;
ConnectionState new_state = [&] {
switch (state) {
case cs::disconnected:
return ConnectionState::Disconnected;
case cs::connecting:
return ConnectionState::Connecting;
case cs::connected:
return ConnectionState::Connected;
}
});
REALM_UNREACHABLE();
}();
// If the OS SyncSession object is destroyed, we ignore any events from the underlying Session as there is
// nothing useful we can do with them.
if (auto self = weak_self.lock()) {
self->update_connection_state(new_state);
if (error) {
self->handle_error(std::move(*error));
}
}
};

m_session = m_client.make_session(m_db, m_flx_subscription_store, m_migration_store, std::move(session_config));
}

void SyncSession::update_connection_state(ConnectionState new_state)
Expand Down
Loading

0 comments on commit 8a68a7f

Please sign in to comment.