Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash and incorrect implementation of seen chat notifications removal #5879

Merged
merged 2 commits into from
Jul 18, 2023

Conversation

allexzander
Copy link
Contributor

@allexzander allexzander commented Jul 10, 2023

Fix crash in _finalList removal loop. Fix incorrect implementation of seen chat notifications removal. Improved notifications fetch code to avoid multiple calls from different threads.

@allexzander allexzander changed the title Fix crash in _finalList removal loop. Fix incorrect implementation of… Fix incorrect implementation of seen chat notofications removal Jul 10, 2023
@allexzander allexzander changed the title Fix incorrect implementation of seen chat notofications removal Fix crash and incorrect implementation of seen chat notofications removal Jul 10, 2023
@allexzander
Copy link
Contributor Author

/backport to stable-3.9

Copy link
Collaborator

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

I tested with concurrent access detection tool from valgrind and failed to get any concurrent unprotected access
@allexzander why did you added mutexes ?

src/gui/tray/usermodel.h Outdated Show resolved Hide resolved
src/gui/tray/activitylistmodel.h Outdated Show resolved Hide resolved
@allexzander allexzander force-pushed the bugfix/fix-incorrect-seen-chat-notifications-removal branch from 1a9f4a7 to 50dd564 Compare July 17, 2023 08:04
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #5879 (5991e78) into master (ed3c952) will decrease coverage by 0.03%.
The diff coverage is 69.38%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5879      +/-   ##
==========================================
- Coverage   60.09%   60.06%   -0.03%     
==========================================
  Files         145      145              
  Lines       18746    18746              
==========================================
- Hits        11265    11260       -5     
- Misses       7481     7486       +5     
Impacted Files Coverage Δ
src/libsync/abstractnetworkjob.cpp 72.24% <ø> (ø)
src/libsync/abstractnetworkjob.h 53.84% <ø> (ø)
src/libsync/networkjobs.h 12.50% <0.00%> (ø)
src/libsync/putmultifilejob.cpp 88.46% <50.00%> (ø)
src/libsync/networkjobs.cpp 59.30% <55.55%> (-0.78%) ⬇️
src/common/checksumcalculator.cpp 86.58% <100.00%> (ø)
src/libsync/propagatedownload.cpp 66.85% <100.00%> (ø)

... and 1 file with indirect coverage changes

@allexzander allexzander force-pushed the bugfix/fix-incorrect-seen-chat-notifications-removal branch from c80206c to db76f74 Compare July 17, 2023 08:12
@mgallien
Copy link
Collaborator

@allexzander all good for me
please squash

@mgallien mgallien force-pushed the bugfix/fix-incorrect-seen-chat-notifications-removal branch from db76f74 to 9ba6c02 Compare July 17, 2023 14:35
@mgallien mgallien enabled auto-merge July 17, 2023 14:35
allexzander and others added 2 commits July 18, 2023 09:57
… seen chat notifications removal. Improved notifications fetch code to avoid multiple calls from different threads.

Signed-off-by: alex-z <[email protected]>
@mgallien mgallien force-pushed the bugfix/fix-incorrect-seen-chat-notifications-removal branch from bd1fcb2 to 5991e78 Compare July 18, 2023 07:57
@sonarcloud
Copy link

sonarcloud bot commented Jul 18, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-5879-5991e780f553d6388a44461944c5783a8327fc52-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@mgallien mgallien merged commit 871812d into master Jul 18, 2023
11 of 12 checks passed
@mgallien mgallien deleted the bugfix/fix-incorrect-seen-chat-notifications-removal branch July 18, 2023 08:56
@allexzander allexzander changed the title Fix crash and incorrect implementation of seen chat notofications removal Fix crash and incorrect implementation of seen chat notifications removal Jul 24, 2023
@mgallien mgallien added this to the 3.10.0 milestone Sep 7, 2023
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.

3 participants