Skip to content

fix: Wrap updateConversationTags transaction in a background task - #2659

Merged
SystemKeeper merged 2 commits into
mainfrom
fix/noid/crash-updating-tags
Aug 21, 2026
Merged

fix: Wrap updateConversationTags transaction in a background task#2659
SystemKeeper merged 2 commits into
mainfrom
fix/noid/crash-updating-tags

Conversation

@Ivansss

@Ivansss Ivansss commented Aug 19, 2026

Copy link
Copy Markdown
Member

Every realm write should run inside a background assertion, so iOS does not
suspend us while the realm write lock is held. The realm lives in the shared
app group container, so being suspended holding its lock is a kill. That was
open-coded at every call site, and about half the write sites were missing it.

RLMRealm.writeTransaction now does it in one place, releasing the assertion
with a defer, and NCDatabaseManager.updateTalkAccount(forAccountId:) covers
the common "change one field on the account" case.

NCRoomsManager.updateRooms is left as it is: it loops over every room on the
account, which can be hundreds, each with nested deletes. It is the only write
that could realistically run out of background time and need to bail out mid
transaction, so it keeps its own background task and cancelWriteTransaction().
Everywhere else the block is a few statements, updateConversationTags
included — which is why its per item expiration checks are gone.

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Signed-off-by: Ivan Sein <ivan@nextcloud.com>
@Ivansss
Ivansss requested a review from SystemKeeper August 19, 2026 10:07
Comment on lines +893 to +896
if bgTask.isExpired {
realm.cancelWriteTransaction()
return
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As discussed, should probably be moved to outside of transaction

Realm writes were open-coded everywhere: get the default realm, begin,
mutate, commit, and sometimes take a background assertion so iOS does not
suspend us mid write. About half the write sites were missing that
assertion.

Add RLMRealm.writeTransaction, which does all of that in one place and
releases the assertion with a defer, plus
NCDatabaseManager.updateTalkAccount(forAccountId:) for the common "change
one field on the account" case. Migrate every write site except
NCRoomsManager.updateRooms, which needs to bail out mid transaction and
keeps its own background task.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ivan Sein <ivan@nextcloud.com>
@SystemKeeper
SystemKeeper merged commit 2ed0221 into main Aug 21, 2026
15 of 17 checks passed
@SystemKeeper
SystemKeeper deleted the fix/noid/crash-updating-tags branch August 21, 2026 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants