8000 Fix race-condition with ObservedFileList::cleanRefsToExpiredDirs. by gin-ahirsch · Pull Request #218 · nickbnf/glogg · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix race-condition with ObservedFileList::cleanRefsToExpiredDirs. #218

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 18, 2018
Merged

Fix race-condition with ObservedFileList::cleanRefsToExpiredDirs. #218

merged 1 commit into from
Jun 18, 2018

Conversation

gin-ahirsch
Copy link
Contributor

I noticed what I think is a race-condition in with ObservedFileList::cleanRefsToExpiredDirs while reading the code.
This should fix that.

@gin-ahirsch
Copy link
Contributor Author

I just realized that heartBeat_ wasn't added for multi-threading, but simply because of complex/unknown data lifetimes.
I'll look if it can't be beautified still.

@gin-ahirsch
Improve ObservedFileList::heartBeat_
5751f35 
Instead of just being a shared_ptr with a dummy value, we store the
this-pointer in it, which makes the closure in addWatchedDirectory
smaller since we can use the weak_ptr in favor of additionally passing
in the ObservedFileList-pointer.
As a bonus, this may make the intent of the heartBeat_ more clear.
@gin-ahirsch gin-ahirsch force-pushed the observedfilelist-race branch from 637d20b to 5751f35 Compare June 14, 2018 08:32
@gin-ahirsch
Copy link
Contributor Author

Alright, this change should be better.
The title of the PR is now wrong, since this doesn't make this class thread-safe. I can open a new one with a proper title if you want.

@nickbnf nickbnf merged commit 4cdb845 into nickbnf:master Jun 18, 2018
@gin-ahirsch gin-ahirsch deleted the observedfilelist-race branch June 21, 2018 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gin-ahirsch @nickbnf
0