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

[CIS-1087] Fix data races in message list's message count #1347

Merged
merged 4 commits into from
Aug 10, 2021

Conversation

dmigach
Copy link
Contributor

@dmigach dmigach commented Aug 9, 2021

My idea was the following:
From the reports on the issue it seemed like the issue is in messages count going out of sync with view's state, so I though of caching message's count and only updating it when we need it

And then I noticed that for channel list there actually already was such a mechanism and it seems like channel list does not experience this kind of issues

The only difference of the suggested fix from the one on the channel list is that message list's structure is a little bit more complicated because it has a VC and a view, but essentially it's the same

@dmigach dmigach changed the title [CIS-1]Fix data races in message list's message count [CIS-1087] Fix data races in message list's message count Aug 9, 2021
@dmigach dmigach force-pushed the CIS-1087-message-list-updates-crash branch from 170d5af to cad77d7 Compare August 9, 2021 13:25
@codecov-commenter
Copy link

codecov-commenter commented Aug 9, 2021

Codecov Report

Merging #1347 (f96b9ad) into main (08b7477) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1347   +/-   ##
=======================================
  Coverage   90.75%   90.75%           
=======================================
  Files         224      224           
  Lines        9800     9800           
=======================================
  Hits         8894     8894           
  Misses        906      906           
Flag Coverage Δ
llc-tests 90.75% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 08b7477...f96b9ad. Read the comment docs.

Copy link
Member

@nuno-vieira nuno-vieira left a comment

Choose a reason for hiding this comment

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

LGTM!

@tbarbugli tbarbugli requested a review from nuno-vieira August 9, 2021 17:25
Copy link
Member

@nuno-vieira nuno-vieira left a comment

Choose a reason for hiding this comment

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

LGTM

@tbarbugli tbarbugli merged commit e657a92 into main Aug 10, 2021
@tbarbugli tbarbugli deleted the CIS-1087-message-list-updates-crash branch August 10, 2021 11:51
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.

4 participants