Fix crash in migrations during secrets deletion

We need to block the migrations returning until the migrations are done.

Fixes #1258
This commit is contained in:
Nicolas Werner 2023-01-07 01:09:36 +01:00
parent 8b6109815d
commit f3b7919a53
No known key found for this signature in database
GPG key ID: C8D75E610773F2D9
3 changed files with 53 additions and 18 deletions

View file

@ -345,9 +345,8 @@ Cache::setup()
{ {
{"pickle_secret", true}, {"pickle_secret", true},
}, },
[this](const std::string &, bool, const std::string &value) { [this](const std::string &, bool, const std::string &value) { this->pickle_secret_ = value; },
this->pickle_secret_ = value; true);
});
} }
static void static void
@ -383,14 +382,21 @@ secretName(std::string name, bool internal)
void void
Cache::loadSecretsFromStore( Cache::loadSecretsFromStore(
std::vector<std::pair<std::string, bool>> toLoad, std::vector<std::pair<std::string, bool>> toLoad,
std::function<void(const std::string &name, bool internal, const std::string &value)> callback) std::function<void(const std::string &name, bool internal, const std::string &value)> callback,
bool databaseReadyOnFinished)
{ {
auto settings = UserSettings::instance()->qsettings(); auto settings = UserSettings::instance()->qsettings();
if (toLoad.empty()) { if (toLoad.empty()) {
this->databaseReady_ = true; this->databaseReady_ = true;
// HACK(Nico): Some migrations would loop infinitely otherwise.
// So we set the database to be ready, but not emit the signal, because that would start the
// migrations again. :D
if (databaseReadyOnFinished) {
emit databaseReady(); emit databaseReady();
nhlog::db()->debug("Database ready"); nhlog::db()->debug("Database ready");
}
return; return;
} }
@ -404,8 +410,10 @@ Cache::loadSecretsFromStore(
callback(name_, internal, value.toStdString()); callback(name_, internal, value.toStdString());
} }
} }
// if we emit the databaseReady signal directly it won't be received // if we emit the DatabaseReady signal directly it won't be received
QTimer::singleShot(0, this, [this, callback] { loadSecretsFromStore({}, callback); }); QTimer::singleShot(0, this, [this, callback, databaseReadyOnFinished] {
loadSecretsFromStore({}, callback, databaseReadyOnFinished);
});
return; return;
} }
@ -421,8 +429,14 @@ Cache::loadSecretsFromStore(
connect(job, connect(job,
&QKeychain::ReadPasswordJob::finished, &QKeychain::ReadPasswordJob::finished,
this, this,
[this, name, toLoad, job, name_ = name_, internal = internal, callback]( [this,
QKeychain::Job *) mutable { name,
toLoad,
job,
name_ = name_,
internal = internal,
callback,
databaseReadyOnFinished](QKeychain::Job *) mutable {
nhlog::db()->debug("Finished reading '{}'", toLoad.begin()->first); nhlog::db()->debug("Finished reading '{}'", toLoad.begin()->first);
const QString secret = job->textData(); const QString secret = job->textData();
if (job->error() && job->error() != QKeychain::Error::EntryNotFound) { if (job->error() && job->error() != QKeychain::Error::EntryNotFound) {
@ -443,8 +457,9 @@ Cache::loadSecretsFromStore(
toLoad.erase(toLoad.begin()); toLoad.erase(toLoad.begin());
// You can't start a job from the finish signal of a job. // You can't start a job from the finish signal of a job.
QTimer::singleShot( QTimer::singleShot(0, this, [this, toLoad, callback, databaseReadyOnFinished] {
0, this, [this, toLoad, callback] { loadSecretsFromStore(toLoad, callback); }); loadSecretsFromStore(toLoad, callback, databaseReadyOnFinished);
});
}); });
nhlog::db()->debug("Reading '{}'", name_); nhlog::db()->debug("Reading '{}'", name_);
job->start(); job->start();
@ -1473,6 +1488,7 @@ Cache::runMigrations()
}}, }},
{"2022.11.06", {"2022.11.06",
[this]() { [this]() {
this->databaseReady_ = false;
loadSecretsFromStore( loadSecretsFromStore(
{ {
{mtx::secret_storage::secrets::cross_signing_master, false}, {mtx::secret_storage::secrets::cross_signing_master, false},
@ -1480,11 +1496,25 @@ Cache::runMigrations()
{mtx::secret_storage::secrets::cross_signing_user_signing, false}, {mtx::secret_storage::secrets::cross_signing_user_signing, false},
{mtx::secret_storage::secrets::megolm_backup_v1, false}, {mtx::secret_storage::secrets::megolm_backup_v1, false},
}, },
[this](const std::string &name, bool internal, const std::string &value) { [this,
count = 1](const std::string &name, bool internal, const std::string &value) mutable {
nhlog::db()->critical("Loaded secret {}", name);
this->storeSecret(name, value, internal); this->storeSecret(name, value, internal);
QTimer::singleShot(
0, this, [this, name, internal] { deleteSecretFromStore(name, internal); }); // HACK(Nico): delay deletion to not crash because of multiple nested deletions.
// Since this is just migration code, this should be *fine*.
QTimer::singleShot(count * 2000, this, [this, name, internal] {
deleteSecretFromStore(name, internal);
}); });
count++;
},
false);
while (!this->databaseReady_) {
QCoreApplication::instance()->processEvents(QEventLoop::AllEvents, 100);
}
return true; return true;
}}, }},
}; };

View file

@ -332,7 +332,8 @@ private:
void loadSecretsFromStore( void loadSecretsFromStore(
std::vector<std::pair<std::string, bool>> toLoad, std::vector<std::pair<std::string, bool>> toLoad,
std::function<void(const std::string &name, bool internal, const std::string &value)> std::function<void(const std::string &name, bool internal, const std::string &value)>
callback); callback,
bool databaseReadyOnFinished = false);
void storeSecretInStore(const std::string name, const std::string secret); void storeSecretInStore(const std::string name, const std::string secret);
void deleteSecretFromStore(const std::string name, bool internal); void deleteSecretFromStore(const std::string name, bool internal);

View file

@ -38,7 +38,11 @@ UserProfile::UserProfile(const QString &roomid,
this, this,
&UserProfile::setGlobalUsername, &UserProfile::setGlobalUsername,
Qt::QueuedConnection); Qt::QueuedConnection);
connect(this, &UserProfile::verificationStatiChanged, &UserProfile::updateVerificationStatus); connect(this,
&UserProfile::verificationStatiChanged,
this,
&UserProfile::updateVerificationStatus,
Qt::QueuedConnection);
if (isGlobalUserProfile()) { if (isGlobalUserProfile()) {
getGlobalProfileData(); getGlobalProfileData();