Skip to content

Commit 467eafe

Browse files
Truncate the metadata realm in-place rather than deleting and recreating it (#7526)
This solves some locking problems in multi-process scenarios. The flow of checking if the key is valid, deleting the file if not, and then recreating the file if needed needs to be done with an exclusive lock on the file held. This can't be done with DB::call_with_lock() because there isn't a good way to initialize the new file from within the call_with_lock() callback, and instead needs to be pushed into the file initialization guarded by the control mutex. Co-authored-by: Finn Schiermer Andersen <[email protected]>
1 parent 6005e69 commit 467eafe

File tree

14 files changed

+341
-202
lines changed

14 files changed

+341
-202
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
### Internals
2020
* Update libuv used in object store tests from v1.35.0 to v1.48.0 ([PR #7508](https://github.com/realm/realm-core/pull/7508)).
2121
* Made `set_default_logger` nullable in the bindgen spec.yml (PR [#7515](https://github.com/realm/realm-core/pull/7515)).
22+
* Recreating the sync metadata Realm when the encryption key changes is now done in a multi-process safe manner ([PR #7526](https://github.com/realm/realm-core/pull/7526)).
2223
* Added `App::default_base_url()` static accessor for SDKs to retrieve the default base URL from Core. ([PR #7534](https://github.com/realm/realm-core/pull/7534))
2324
* Realm2JSON tool will now correctly upgrade file to current fileformat.
2425

src/realm/alloc_slab.cpp

Lines changed: 78 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -813,17 +813,39 @@ ref_type SlabAlloc::attach_file(const std::string& path, Config& cfg, util::Writ
813813
// the call below to set_encryption_key.
814814
m_file.set_encryption_key(cfg.encryption_key);
815815

816+
note_reader_start(this);
817+
util::ScopeExit reader_end_guard([this]() noexcept {
818+
note_reader_end(this);
819+
});
816820
size_t size = 0;
817821
// The size of a database file must not exceed what can be encoded in
818822
// size_t.
819823
if (REALM_UNLIKELY(int_cast_with_overflow_detect(m_file.get_size(), size)))
820824
throw InvalidDatabase("Realm file too large", path);
821-
if (cfg.encryption_key && size == 0 && physical_file_size != 0) {
825+
if (cfg.clear_file_on_error && cfg.session_initiator) {
826+
if (size == 0 && physical_file_size != 0) {
827+
cfg.clear_file = true;
828+
}
829+
else if (size > 0) {
830+
try {
831+
read_and_validate_header(m_file, path, size, cfg.session_initiator, m_write_observer);
832+
}
833+
catch (const InvalidDatabase&) {
834+
cfg.clear_file = true;
835+
}
836+
}
837+
}
838+
if (cfg.clear_file) {
839+
m_file.resize(0);
840+
size = 0;
841+
physical_file_size = 0;
842+
}
843+
else if (cfg.encryption_key && !cfg.clear_file && size == 0 && physical_file_size != 0) {
822844
// The opened file holds data, but is so small it cannot have
823845
// been created with encryption
824846
throw InvalidDatabase("Attempt to open unencrypted file with encryption key", path);
825847
}
826-
if (size == 0 || cfg.clear_file) {
848+
if (size == 0) {
827849
if (REALM_UNLIKELY(cfg.read_only))
828850
throw InvalidDatabase("Read-only access to empty Realm file", path);
829851

@@ -833,7 +855,7 @@ ref_type SlabAlloc::attach_file(const std::string& path, Config& cfg, util::Writ
833855
// mappings_for_file in the encryption layer. So the prealloc() is required before
834856
// interacting with the encryption layer in File::write().
835857
// Pre-alloc initial space
836-
m_file.prealloc(initial_size); // Throws
858+
m_file.prealloc(initial_size); // Throws
837859
// seek() back to the start of the file in preparation for writing the header
838860
// This sequence of File operations is protected from races by
839861
// DB::m_controlmutex, so we know we are the only ones operating on the file
@@ -847,65 +869,9 @@ ref_type SlabAlloc::attach_file(const std::string& path, Config& cfg, util::Writ
847869

848870
size = initial_size;
849871
}
850-
ref_type top_ref;
851-
note_reader_start(this);
852-
util::ScopeExit reader_end_guard([this]() noexcept {
853-
note_reader_end(this);
854-
});
855-
856-
try {
857-
// we'll read header and (potentially) footer
858-
File::Map<char> map_header(m_file, File::access_ReadOnly, sizeof(Header), 0, m_write_observer);
859-
realm::util::encryption_read_barrier(map_header, 0, sizeof(Header));
860-
auto header = reinterpret_cast<const Header*>(map_header.get_addr());
861872

862-
File::Map<char> map_footer;
863-
const StreamingFooter* footer = nullptr;
864-
if (is_file_on_streaming_form(*header) && size >= sizeof(StreamingFooter) + sizeof(Header)) {
865-
size_t footer_ref = size - sizeof(StreamingFooter);
866-
size_t footer_page_base = footer_ref & ~(page_size() - 1);
867-
size_t footer_offset = footer_ref - footer_page_base;
868-
map_footer = File::Map<char>(m_file, footer_page_base, File::access_ReadOnly,
869-
sizeof(StreamingFooter) + footer_offset, 0, m_write_observer);
870-
realm::util::encryption_read_barrier(map_footer, footer_offset, sizeof(StreamingFooter));
871-
footer = reinterpret_cast<const StreamingFooter*>(map_footer.get_addr() + footer_offset);
872-
}
873-
874-
top_ref = validate_header(header, footer, size, path, cfg.encryption_key != nullptr); // Throws
875-
m_attach_mode = cfg.is_shared ? attach_SharedFile : attach_UnsharedFile;
876-
m_data = map_header.get_addr(); // <-- needed below
877-
878-
if (cfg.session_initiator && is_file_on_streaming_form(*header)) {
879-
// Don't compare file format version fields as they are allowed to differ.
880-
// Also don't compare reserved fields.
881-
REALM_ASSERT_EX(header->m_flags == 0, header->m_flags, get_file_path_for_assertions());
882-
REALM_ASSERT_EX(header->m_mnemonic[0] == uint8_t('T'), header->m_mnemonic[0],
883-
get_file_path_for_assertions());
884-
REALM_ASSERT_EX(header->m_mnemonic[1] == uint8_t('-'), header->m_mnemonic[1],
885-
get_file_path_for_assertions());
886-
REALM_ASSERT_EX(header->m_mnemonic[2] == uint8_t('D'), header->m_mnemonic[2],
887-
get_file_path_for_assertions());
888-
REALM_ASSERT_EX(header->m_mnemonic[3] == uint8_t('B'), header->m_mnemonic[3],
889-
get_file_path_for_assertions());
890-
REALM_ASSERT_EX(header->m_top_ref[0] == 0xFFFFFFFFFFFFFFFFULL, header->m_top_ref[0],
891-
get_file_path_for_assertions());
892-
REALM_ASSERT_EX(header->m_top_ref[1] == 0, header->m_top_ref[1], get_file_path_for_assertions());
893-
REALM_ASSERT_EX(footer->m_magic_cookie == footer_magic_cookie, footer->m_magic_cookie,
894-
get_file_path_for_assertions());
895-
}
896-
}
897-
catch (const InvalidDatabase&) {
898-
throw;
899-
}
900-
catch (const DecryptionFailed& e) {
901-
throw InvalidDatabase(util::format("Realm file decryption failed (%1)", e.what()), path);
902-
}
903-
catch (const std::exception& e) {
904-
throw InvalidDatabase(e.what(), path);
905-
}
906-
catch (...) {
907-
throw InvalidDatabase("unknown error", path);
908-
}
873+
ref_type top_ref = read_and_validate_header(m_file, path, size, cfg.session_initiator, m_write_observer);
874+
m_attach_mode = cfg.is_shared ? attach_SharedFile : attach_UnsharedFile;
909875
// m_data not valid at this point!
910876
m_baseline = 0;
911877
// make sure that any call to begin_read cause any slab to be placed in free
@@ -1040,6 +1006,57 @@ void SlabAlloc::attach_empty()
10401006
m_ref_translation_ptr = new RefTranslation[1];
10411007
}
10421008

1009+
ref_type SlabAlloc::read_and_validate_header(util::File& file, const std::string& path, size_t size,
1010+
bool session_initiator, util::WriteObserver* write_observer)
1011+
{
1012+
try {
1013+
// we'll read header and (potentially) footer
1014+
File::Map<char> map_header(file, File::access_ReadOnly, sizeof(Header), 0, write_observer);
1015+
realm::util::encryption_read_barrier(map_header, 0, sizeof(Header));
1016+
auto header = reinterpret_cast<const Header*>(map_header.get_addr());
1017+
1018+
File::Map<char> map_footer;
1019+
const StreamingFooter* footer = nullptr;
1020+
if (is_file_on_streaming_form(*header) && size >= sizeof(StreamingFooter) + sizeof(Header)) {
1021+
size_t footer_ref = size - sizeof(StreamingFooter);
1022+
size_t footer_page_base = footer_ref & ~(page_size() - 1);
1023+
size_t footer_offset = footer_ref - footer_page_base;
1024+
map_footer = File::Map<char>(file, footer_page_base, File::access_ReadOnly,
1025+
sizeof(StreamingFooter) + footer_offset, 0, write_observer);
1026+
realm::util::encryption_read_barrier(map_footer, footer_offset, sizeof(StreamingFooter));
1027+
footer = reinterpret_cast<const StreamingFooter*>(map_footer.get_addr() + footer_offset);
1028+
}
1029+
1030+
auto top_ref = validate_header(header, footer, size, path, file.get_encryption_key() != nullptr); // Throws
1031+
1032+
if (session_initiator && is_file_on_streaming_form(*header)) {
1033+
// Don't compare file format version fields as they are allowed to differ.
1034+
// Also don't compare reserved fields.
1035+
REALM_ASSERT_EX(header->m_flags == 0, header->m_flags, path);
1036+
REALM_ASSERT_EX(header->m_mnemonic[0] == uint8_t('T'), header->m_mnemonic[0], path);
1037+
REALM_ASSERT_EX(header->m_mnemonic[1] == uint8_t('-'), header->m_mnemonic[1], path);
1038+
REALM_ASSERT_EX(header->m_mnemonic[2] == uint8_t('D'), header->m_mnemonic[2], path);
1039+
REALM_ASSERT_EX(header->m_mnemonic[3] == uint8_t('B'), header->m_mnemonic[3], path);
1040+
REALM_ASSERT_EX(header->m_top_ref[0] == 0xFFFFFFFFFFFFFFFFULL, header->m_top_ref[0], path);
1041+
REALM_ASSERT_EX(header->m_top_ref[1] == 0, header->m_top_ref[1], path);
1042+
REALM_ASSERT_EX(footer->m_magic_cookie == footer_magic_cookie, footer->m_magic_cookie, path);
1043+
}
1044+
return top_ref;
1045+
}
1046+
catch (const InvalidDatabase&) {
1047+
throw;
1048+
}
1049+
catch (const DecryptionFailed& e) {
1050+
throw InvalidDatabase(util::format("Realm file decryption failed (%1)", e.what()), path);
1051+
}
1052+
catch (const std::exception& e) {
1053+
throw InvalidDatabase(e.what(), path);
1054+
}
1055+
catch (...) {
1056+
throw InvalidDatabase("unknown error", path);
1057+
}
1058+
}
1059+
10431060
void SlabAlloc::throw_header_exception(std::string msg, const Header& header, const std::string& path)
10441061
{
10451062
char buf[256];

src/realm/alloc_slab.hpp

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,20 @@ class SlabAlloc : public Allocator {
108108
/// Always initialize the file as if it was a newly
109109
/// created file and ignore any pre-existing contents. Requires that
110110
/// Config::session_initiator be true as well.
111+
///
112+
/// \var Config::clear_file_on_error
113+
/// If the file being opened is not a valid Realm file (possibly due to a
114+
/// decryption failure), reinitialize it as if clear_file was set.
111115
struct Config {
116+
const char* encryption_key = nullptr;
112117
bool is_shared = false;
113118
bool read_only = false;
114119
bool no_create = false;
115120
bool skip_validate = false;
116121
bool session_initiator = false;
117122
bool clear_file = false;
123+
bool clear_file_on_error = false;
118124
bool disable_sync = false;
119-
const char* encryption_key = nullptr;
120125
};
121126

122127
struct Retry {};
@@ -363,6 +368,11 @@ class SlabAlloc : public Allocator {
363368
void note_reader_start(const void* reader_id);
364369
void note_reader_end(const void* reader_id) noexcept;
365370

371+
/// Read the header (and possibly footer) from the file, returning the top ref if it's valid and throwing
372+
/// InvalidDatabase otherwise.
373+
static ref_type read_and_validate_header(util::File& file, const std::string& path, size_t size,
374+
bool session_initiator, util::WriteObserver* write_observer);
375+
366376
void verify() const override;
367377
#ifdef REALM_DEBUG
368378
void enable_debug(bool enable)
@@ -703,10 +713,10 @@ class SlabAlloc : public Allocator {
703713
/// corrupted, or if the specified encryption key is incorrect. This
704714
/// function will not detect all forms of corruption, though.
705715
/// Returns the top_ref for the latest commit.
706-
ref_type validate_header(const char* data, size_t len, const std::string& path);
707-
ref_type validate_header(const Header* header, const StreamingFooter* footer, size_t size,
708-
const std::string& path, bool is_encrypted = false);
709-
void throw_header_exception(std::string msg, const Header& header, const std::string& path);
716+
static ref_type validate_header(const char* data, size_t len, const std::string& path);
717+
static ref_type validate_header(const Header* header, const StreamingFooter* footer, size_t size,
718+
const std::string& path, bool is_encrypted = false);
719+
static void throw_header_exception(std::string msg, const Header& header, const std::string& path);
710720

711721
static bool is_file_on_streaming_form(const Header& header);
712722
/// Read the top_ref from the given buffer and set m_file_on_streaming_form

src/realm/db.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,7 @@ std::string DBOptions::sys_tmp_dir = getenv("TMPDIR") ? getenv("TMPDIR") : "";
898898
// initializing process crashes and leaves the shared memory in an
899899
// undefined state.
900900

901-
void DB::open(const std::string& path, bool no_create_file, const DBOptions& options)
901+
void DB::open(const std::string& path, const DBOptions& options)
902902
{
903903
// Exception safety: Since do_open() is called from constructors, if it
904904
// throws, it must leave the file closed.
@@ -1144,10 +1144,11 @@ void DB::open(const std::string& path, bool no_create_file, const DBOptions& opt
11441144
cfg.read_only = false;
11451145
cfg.skip_validate = !begin_new_session;
11461146
cfg.disable_sync = options.durability == Durability::MemOnly || options.durability == Durability::Unsafe;
1147+
cfg.clear_file_on_error = options.clear_on_invalid_file;
11471148

11481149
// only the session initiator is allowed to create the database, all other
11491150
// must assume that it already exists.
1150-
cfg.no_create = (begin_new_session ? no_create_file : true);
1151+
cfg.no_create = (begin_new_session ? options.no_create : true);
11511152

11521153
// if we're opening a MemOnly file that isn't already opened by
11531154
// someone else then it's a file which should have been deleted on
@@ -1499,8 +1500,7 @@ void DB::open(Replication& repl, const std::string& file, const DBOptions& optio
14991500

15001501
set_replication(&repl);
15011502

1502-
bool no_create = false;
1503-
open(file, no_create, options); // Throws
1503+
open(file, options); // Throws
15041504
}
15051505

15061506
class DBLogger : public Logger {
@@ -1532,7 +1532,7 @@ void DB::set_logger(const std::shared_ptr<util::Logger>& logger) noexcept
15321532
m_logger = std::make_shared<DBLogger>(logger, m_log_id);
15331533
}
15341534

1535-
void DB::open(Replication& repl, const DBOptions options)
1535+
void DB::open(Replication& repl, const DBOptions& options)
15361536
{
15371537
REALM_ASSERT(!is_attached());
15381538
repl.initialize(*this); // Throws
@@ -2808,10 +2808,10 @@ inline DB::DB(Private, const DBOptions& options)
28082808
}
28092809
}
28102810

2811-
DBRef DB::create(const std::string& file, bool no_create, const DBOptions& options) NO_THREAD_SAFETY_ANALYSIS
2811+
DBRef DB::create(const std::string& file, const DBOptions& options) NO_THREAD_SAFETY_ANALYSIS
28122812
{
28132813
DBRef retval = std::make_shared<DB>(Private(), options);
2814-
retval->open(file, no_create, options);
2814+
retval->open(file, options);
28152815
return retval;
28162816
}
28172817

src/realm/db.hpp

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ class DB : public std::enable_shared_from_this<DB> {
128128
// calling DB::close(), but after that no new association can be established. To reopen the
129129
// file (or another file), a new DB object is needed. The specified Replication instance, if
130130
// any, must remain in existence for as long as the DB.
131-
static DBRef create(const std::string& file, bool no_create = false, const DBOptions& options = DBOptions());
131+
static DBRef create(const std::string& file, const DBOptions& options = DBOptions());
132132
static DBRef create(Replication& repl, const std::string& file, const DBOptions& options = DBOptions());
133133
static DBRef create(std::unique_ptr<Replication> repl, const std::string& file,
134134
const DBOptions& options = DBOptions());
@@ -534,10 +534,6 @@ class DB : public std::enable_shared_from_this<DB> {
534534
///
535535
/// \param file Filesystem path to a Realm database file.
536536
///
537-
/// \param no_create If the database file does not already exist, it will be
538-
/// created (unless this is set to true.) When multiple threads are involved,
539-
/// it is safe to let the first thread, that gets to it, create the file.
540-
///
541537
/// \param options See DBOptions for details of each option.
542538
/// Sensible defaults are provided if this parameter is left out.
543539
///
@@ -553,13 +549,12 @@ class DB : public std::enable_shared_from_this<DB> {
553549
/// \throw UnsupportedFileFormatVersion if the file format version or
554550
/// history schema version is one which this version of Realm does not know
555551
/// how to migrate from.
556-
void open(const std::string& file, bool no_create = false, const DBOptions& options = DBOptions())
557-
REQUIRES(!m_mutex);
552+
void open(const std::string& file, const DBOptions& options = DBOptions()) REQUIRES(!m_mutex);
558553
void open(BinaryData, bool take_ownership = true) REQUIRES(!m_mutex);
559554
void open(Replication&, const std::string& file, const DBOptions& options = DBOptions()) REQUIRES(!m_mutex);
560-
void open(Replication& repl, const DBOptions options = DBOptions()) REQUIRES(!m_mutex);
555+
void open(Replication& repl, const DBOptions& options = DBOptions()) REQUIRES(!m_mutex);
561556

562-
void do_open(const std::string& file, bool no_create, const DBOptions& options);
557+
void do_open(const std::string& file, const DBOptions& options);
563558

564559
Replication* const* get_repl() const noexcept
565560
{

src/realm/db_options.hpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ struct DBOptions {
8888
/// Disable automatic backup at file format upgrade by setting to false
8989
bool backup_at_file_format_change = true;
9090

91+
/// Disable creating new files if the DB to open does not already exist.
92+
bool no_create = false;
93+
9194
/// List of versions we can upgrade from
9295
BackupHandler::VersionList accepted_versions = BackupHandler::accepted_versions_;
9396

@@ -99,6 +102,10 @@ struct DBOptions {
99102
/// a performance impact.
100103
bool enable_async_writes = false;
101104

105+
/// If set, opening a file which is not a Realm file or cannot be decrypted
106+
/// will clear and reinitialize the file.
107+
bool clear_on_invalid_file = false;
108+
102109
/// sys_tmp_dir will be used if the temp_dir is empty when creating DBOptions.
103110
/// It must be writable and allowed to create pipe/fifo file on it.
104111
/// set_sys_tmp_dir is not a thread-safe call and it is only supposed to be called once

src/realm/object-store/impl/realm_coordinator.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,7 @@ bool RealmCoordinator::open_db()
479479
}
480480
options.encryption_key = m_config.encryption_key.data();
481481
options.allow_file_format_upgrade = !m_config.disable_format_upgrade && !schema_mode_reset_file;
482+
options.clear_on_invalid_file = m_config.clear_on_invalid_file;
482483
if (history) {
483484
options.backup_at_file_format_change = m_config.backup_at_file_format_change;
484485
#ifdef __EMSCRIPTEN__
@@ -496,7 +497,8 @@ bool RealmCoordinator::open_db()
496497
#endif
497498
}
498499
else {
499-
m_db = DB::create(m_config.path, true, options);
500+
options.no_create = true;
501+
m_db = DB::create(m_config.path, options);
500502
}
501503
}
502504
catch (realm::FileFormatUpgradeRequired const&) {

src/realm/object-store/shared_realm.hpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,13 @@ struct RealmConfig {
184184
// everything can be done deterministically on one thread, and
185185
// speeds up tests that don't need notifications.
186186
bool automatic_change_notifications = true;
187+
188+
// For internal use and should not be exposed by SDKs.
189+
//
190+
// If the file is invalid or can't be decrypted with the given encryption
191+
// key, clear it and reinitialize it as a new file. This is used for the
192+
// sync metadata realm which is automatically deleted if it can't be used.
193+
bool clear_on_invalid_file = false;
187194
};
188195

189196
class Realm : public std::enable_shared_from_this<Realm> {

0 commit comments

Comments
 (0)