fix: key uploads only running once

There were several issues here. Key uploads in general failed, because
the await caused a Future<dynamic> to be returned, which failed type
checking.

But also we never unset our future, which was used for the online key
backup uploads, which meant we would very quickly stop uploading any
keys at all.
This commit is contained in:
Nicolas Werner 2023-11-24 17:37:39 +01:00
parent 9019dfd566
commit 25f4353ab0
No known key found for this signature in database
GPG Key ID: B38119FF80087618
3 changed files with 89 additions and 70 deletions

View File

@ -232,9 +232,12 @@ class Encryption {
// the entry should always exist. In the case it doesn't, the following
// line *could* throw an error. As that is a future, though, and we call
// it un-awaited here, nothing happens, which is exactly the result we want
client.database
// ignore: discarded_futures
client.database?.updateInboundGroupSessionIndexes(
json.encode(inboundGroupSession.indexes), roomId, sessionId);
?.updateInboundGroupSessionIndexes(
json.encode(inboundGroupSession.indexes), roomId, sessionId)
// ignore: discarded_futures
.onError((e, _) => Logs().e('Ignoring error for updating indexes'));
}
decryptedPayload = json.decode(decryptResult.plaintext);
} catch (exception) {

View File

@ -795,13 +795,18 @@ class KeyManager {
// Make sure to not run in parallel
if (_uploadingFuture != null) {
if (skipIfInProgress) return;
await _uploadingFuture;
}
final completer = Completer<void>();
_uploadingFuture = completer.future;
await client.userDeviceKeysLoading;
try {
await _uploadingFuture;
} finally {
// shouldn't be necessary, since it will be unset already by the other process that started it, but just to be safe, also unset the future here
_uploadingFuture = null;
}
}
Future<void> uploadInternal() async {
try {
await client.userDeviceKeysLoading;
if (!(await isCached())) {
return; // we can't backup anyways
}
@ -821,6 +826,7 @@ class KeyManager {
if (info.algorithm !=
BackupAlgorithm.mMegolmBackupV1Curve25519AesSha2 ||
info.authData['public_key'] != backupPubKey) {
decryption.free();
return;
}
final args = GenerateUploadKeysArgs(
@ -861,8 +867,14 @@ class KeyManager {
}
} catch (e, s) {
Logs().e('[Key Manager] Error uploading room keys', e, s);
}
}
_uploadingFuture = uploadInternal();
try {
await _uploadingFuture;
} finally {
completer.complete();
_uploadingFuture = null;
}
}
@ -1187,12 +1199,12 @@ RoomKeys generateUploadKeysImplementation(GenerateUploadKeysArgs args) {
},
);
}
enc.free();
return roomKeys;
} catch (e, s) {
Logs().e('[Key Manager] Error generating payload', e, s);
rethrow;
} finally {
enc.free();
rethrow;
}
}

View File

@ -52,7 +52,7 @@ abstract class NativeImplementations {
/// this implementation will catch any non-implemented method
@override
dynamic noSuchMethod(Invocation invocation) async {
dynamic noSuchMethod(Invocation invocation) {
final dynamic argument = invocation.positionalArguments.single;
final memberName = invocation.memberName.toString().split('"')[1];
@ -62,11 +62,15 @@ abstract class NativeImplementations {
'Fallback from NativeImplementations.dummy used.',
);
switch (memberName) {
// we need to pass the futures right through or we will run into type errors later!
case 'generateUploadKeys':
// ignore: discarded_futures
return dummy.generateUploadKeys(argument);
case 'keyFromPassphrase':
// ignore: discarded_futures
return dummy.keyFromPassphrase(argument);
case 'decryptFile':
// ignore: discarded_futures
return dummy.decryptFile(argument);
case 'shrinkImage':
return dummy.shrinkImage(argument);