Skip null resources when restoring repository entries - #5799
Merged
Merged
Conversation
When a local or outdated resource is removed, prepareData restores its repository entry so the region shows it as downloadable. The lookup uses QHash::value, which returns a null pointer when the id is not in the repository, and that null was stored in the region cache. Readers of the cache dereference every entry, so the crash surfaced later, in collectSubregionItemsFromRegularRegion rather than here. Insert the entry only when the repository actually has it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
prepareDatarebuilds the region cache incrementally. When a resource that was local or outdated is gone from the fresh scan, theelsebranch restores its repository entry so the region shows it as downloadable again:QHash::valuereturns a default-constructedshared_ptrwhen the key is absent, so an id that is no longer in the repository stores a null pointer inallResources. Every reader of the region cache dereferences its entries unconditionally —collectSubregionItemsFromRegularRegiondoesresource_->typeon each one — so the null is dereferenced on a later pass, not at the insertion site.Two ways to reach it: a catalog refresh drops an id that is still installed locally, and
rescanUnmanagedStoragePathsputs imported.obffiles into the local resources, which by definition have no repository entry.Insert the entry only when the repository actually has it.
Scope
Deliberately minimal for the release branch: two call sites, no behaviour change when the value is non-null, no locking, no perf impact.
The same area has a separate synchronization problem — the region cache is static but its lock is a per-instance ivar, and four external
+prepareDatacallers bypass it, one of them on a background queue. That belongs in master on top of #5765, which makes a lock acrossprepareDatacheap; it is not in this PR.Testing
Not built or run — build verification is left to the maintainer. The change is a null check on the value returned by
QHash::valuebefore storing it.