Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RCORE-2159 Make async_open_realm() return StatusWith<std::shared_ptr<Realm>> #7808

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented Jun 12, 2024

This simplifies a lot of test code and seems to fix a case where we were opening a Realm on the wrong thread on Linux, although I'm not entirely sure how that was happening.

Fixes #7793.

This simplifies a lot of test code and eliminates some cases where the Realm
was being opened on a background thread, which is unsupported on linux.
@tgoyne tgoyne self-assigned this Jun 12, 2024
@cla-bot cla-bot bot added the cla: yes label Jun 12, 2024
Copy link

coveralls-official bot commented Jun 12, 2024

Pull Request Test Coverage Report for Build thomas.goyne_417

Details

  • 93 of 93 (100.0%) changed or added relevant lines in 4 files are covered.
  • 58 unchanged lines in 12 files lost coverage.
  • Overall coverage increased (+0.01%) to 90.964%

Files with Coverage Reduction New Missed Lines %
src/realm/array_string.cpp 1 87.23%
src/realm/object-store/sync/async_open_task.cpp 1 88.36%
src/realm/util/serializer.cpp 1 90.43%
test/fuzz_group.cpp 2 51.99%
src/realm/collection_parent.cpp 3 93.08%
src/realm/alloc_slab.cpp 4 90.56%
src/realm/sync/noinst/client_impl_base.cpp 5 82.42%
test/object-store/util/sync/baas_admin_api.cpp 5 84.93%
src/realm/bplustree.cpp 7 71.41%
test/sync_fixtures.hpp 7 75.56%
Totals Coverage Status
Change from base Build 2415: 0.01%
Covered Lines: 214660
Relevant Lines: 235983

💛 - Coveralls

});
task->cancel(); // don't run the above notifier again on this session
finish(std::move(tsr), err);
auto sw = std::move(pf.future).get_no_throw();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i think this could be

return std::move(pf.future).then([&](ThreadSafeReference&& ref) {
    return Realm::get_shared_realm(std::move(ref));
}).get_no_throw();

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that'd open the Realm on the wrong thread. Resolving the TSR needs to happen after the get() so that it happens on the main thread.

Future's API works well for passing around thread-unsafe objects but less so for thread-confined objects.

@tgoyne tgoyne merged commit e768965 into master Jun 18, 2024
46 checks passed
@tgoyne tgoyne deleted the tg/refactor-test-helper branch June 18, 2024 16:04
@github-actions github-actions bot mentioned this pull request Jun 28, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate failure in "flx: client reset::Adding a local property matching a server addition is allowed"
3 participants