Skip to content

fixing possible Error:EPERM:operation not permitted,rename ... #20

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

Closed
wants to merge 4 commits into from

Conversation

gaodeng
Copy link

@gaodeng gaodeng commented Jan 29, 2019

fix #17
fix #15

@gaodeng gaodeng changed the title fixing possible Error:EPERM:operation not permitted,rename fixing possible Error:EPERM:operation not permitted,rename ... Jan 29, 2019
@MaverickMartyn
Copy link

@gaodeng There seems to be an issue here.
According to the test output here,
storage.get("state") is returning undefined for some reason.
I am going to take a look, since I have my own fork now, in part because of the issues this PR is trying to fix.
In the meantime, perhaps you could have a look as well? I am not familiar with lodash, so it might take me some extra time.

@MaverickMartyn
Copy link

MaverickMartyn commented Jan 31, 2019

@gaodeng and @MrEmelianenko I have looked into this, and I don't think this is a very good solution.
The reason the tests fail, is because of the debounce's 1 second delay, causing the store to not have been persisted yet.
This then continues for the rest of the tests, since at no point is there more than 1 second between setState calls.

This would likely be the only real world scenario that would cause the error in the first place.
I therefore propose that we focus on providing options to limit the amount of times we save the state and limiting what is saved to it.
This could be done by for example deferring the persistance saving itself (while keeping the state in memory) and/or providing proper whitelist/blacklist support.

@gaodeng gaodeng closed this Aug 14, 2019
@summxu
Copy link

summxu commented Sep 9, 2019

thanks my bro 问题解决,爱你摸摸哒

fix #17
fix #15

@yansenlei
Copy link

Why not merge

@jensstigaard
Copy link

I have the same issue on Windows as addressed by the PR. I have just used this PR, and it seem to fix the issue in practice, since the changes are persisted with a possible delay, and I don't experience errors.
It is of course not ideal that Windows cannot do rename atomically, and the current solution is to throttle to file updates to 1000ms, it is not ideal, but it does the job in practice.
As @MaverickMartyn states, it should be solved by figuring out how to refactor to atomically wait for the locking of the file before renaming the file.

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.

Error:EPERM:operation not permitted,rename main process and renderer process save the same json data to disk
5 participants