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.

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

check if cache loop task is running before starting new thread #39

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

check if cache loop task is running before starting new thread #39

wants to merge 3 commits into from

Conversation

jokorone
Copy link

@jokorone jokorone commented Jun 30, 2023 •
edited
Loading

Fixes #26, resolves #41

@jokorone
Copy link
Author

I'm very sorry for the linting, clippy doing it's thing 🤷

@RaphGL
Copy link
Contributor

RaphGL commented Jun 30, 2023

I don't think this should be merged. At least not yet.As stated here #26 (comment) this would be a work around. We could possibly fix the thing causing the panic instead of just spawning a new thread every 30 seconds needlessly.

@conaticus
Copy link
Owner

conaticus commented Jun 30, 2023 •
edited
Loading

Thanks for the PR, this needs to be opened on the dev branch please.

Nevermind I forgot I could edit it 🤦‍♂️I agree with Raph that we shouldn't merge this as we need it fully tested first.

@conaticus conaticus closed this Jun 30, 2023
@conaticus conaticus reopened this Jun 30, 2023
@conaticus conaticus changed the base branch from main to dev June 30, 2023 15:58
@conaticus conaticus self-requested a review June 30, 2023 15:59
@jokorone
Copy link
Author

I merged the dev branch into my branch and added a task_running flag to check if the old thread is still running.
While it's still debatable if a 30sec interval does the job, now it won't run into infinites If I thought this through right.

@jokorone jokorone changed the title add match guard clause when accessing state, catch poison… check if cache loop task is running before starting new thread Jun 30, 2023
@conaticus
Copy link
Owner

I merged the dev branch into my branch and added a task_running flag to check if the old thread is still running. While it's still debatable if a 30sec interval does the job, now it won't run into infinites If I thought this through right.

I will test this with a Mac user later in addition to #33. Also please make sure you claim issues by pinging me in the comments so I can assign you. This is just in case someone is already working on it. Duplicate PRs have occurred few times before so I'll work on some contribution guidelines.

Thanks for your help!

@conaticus conaticus mentioned this pull request Jul 1, 2023
Merged
afiqzx
afiqzx reviewed Jul 7, 2023
View reviewed changes
src-tauri/src/filesystem/cache.rs
interval.tick().await; // Wait 30 seconds before doing first re-cache

loop {
interval.tick().await;

let guard = &mut state_clone.lock().unwrap();
save_to_cache(guard);
if !task_running.load(Ordering::SeqCst) {
Copy link

Choose a reason for hiding this comment

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

Shouldn't this be swap instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afiqzx afiqzx afiqzx left review comments

@conaticus conaticus Awaiting requested review from conaticus

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dev build crashes on macOS
4 participants
@jokorone @RaphGL @conaticus @afiqzx