Need to trace this further back to see why an item would be nullptr. Also need to fix a couple things here. bool locked; and if (item)

Codecov Report

Merging #7850 (4b6093e) into develop (5916a8f) will decrease coverage by 0.01%.
The diff coverage is 75.00%.

@@             Coverage Diff             @@
##           develop    #7850      +/-   ##
===========================================
- Coverage    64.31%   64.29%   -0.01%     
===========================================
  Files          339      339              
  Lines        43404    43431      +27     
===========================================
+ Hits         27911    27922      +11     
- Misses       15493    15509      +16     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5916a8f...4b6093e. Read the comment docs.

Need to trace this further back to see why an item would be nullptr. Also need to fix a couple things here. bool locked; and if (item)

Agreed. @Aetf

@bionade24 , I would remove the "Fixes: #7731", I'm not convinced this is the same crash as the OP in #7731. Actually, the OP sounds more like what was fixed in 2.7.1.

Need to trace this further back to see why an item would be nullptr. Also need to fix a couple things here. bool locked; and if (item)

Agreed. @Aetf

@bionade24 , I would remove the "Fixes: #7731", I'm not convinced this is the same crash as the OP in #7731. Actually, the OP sounds more like what was fixed in 2.7.1.

Fine, as he didn't provide a backtrace we can't know for sure.

Agreed the referenced issue definitely sounds like the problem fixed by 2.7.1. Going to close that one as duplicate.

Any info about how to reproduce this? (best with a dbus-monitor trace dbus-monitor "destination=org.freedesktop.secrets" "sender=org.freedesktop.secrets")
The original repro in #7731 is for the already fixed crash.

items pointer may be nullptr if Collection::searchItems returns nullptr. And all pointers produced by
Collection::searchItems are obtained by m_entryToItem.value(entry). So if for some reason the map doesn't contain
the entry, a default nullptr will be returned.

So if after an entry is added, and before Collection::onEntryAdded gets called, a searchItems call comes in, it may
trigger this.

The correct fix should be to make sure Collection::searchItems doesn't return nullptr.

Agree searchItems should never add nullptr to the array, that is begging for crashes.

However it is still unclear to me why this happens in the first place. I can't come up with a case where an entry is added but onEntryAdded is not called before anything else. That's why there is no checks in searchItems, because I supposed the invariant is always maintained.

The root cause is found to be entries in the recycle bin returned during search, and they don't have corresponding items, thus nullptr.

I'll prepare a new PR to fix that, and close this one.

@Aetf have you opened a PR/Issue for it yet?

#8021