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

proc: process info files should be locked during writing #53

Closed
pmrowla opened this issue Jun 1, 2022 · 0 comments · Fixed by #65
Closed

proc: process info files should be locked during writing #53

pmrowla opened this issue Jun 1, 2022 · 0 comments · Fixed by #65
Assignees
Labels
bug Something isn't working

Comments

@pmrowla
Copy link
Contributor

pmrowla commented Jun 1, 2022

In certain situations it's possible that a reader tries to load proc info from file while it's being written to which causes a jsondecodeerror. ProcessManager should provide minimal amount of synchronization for these cases.

Alternatively, at a minimum we should just write to temporary paths (i.e. output_path.json.<random suffix>) and then fs.move to replace the existing file in:

def _dump(self):
self._make_wdir()
with open(self.info_path, "w", encoding="utf-8") as fobj:
json.dump(self.info.asdict(), fobj)
with open(self.pidfile_path, "w", encoding="utf-8") as fobj:
fobj.write(str(self.pid))

@pmrowla pmrowla added the bug Something isn't working label Jun 1, 2022
@pmrowla pmrowla assigned pmrowla and karajan1001 and unassigned pmrowla Jun 15, 2022
karajan1001 added a commit that referenced this issue Jun 16, 2022
fix: #53
Currently, the dump and load process info file at a time may cause some
error. Write to a temp file and then rename it eases this problem. But a
final solution would still be required in someday later.
pmrowla pushed a commit that referenced this issue Jun 16, 2022
fix: #53
Currently, the dump and load process info file at a time may cause some
error. Write to a temp file and then rename it eases this problem. But a
final solution would still be required in someday later.
karajan1001 added a commit that referenced this issue Nov 8, 2022
fix: #93

Our old solution on #53, can reduce possibility of race condition, but
it still happens occasionally.

1. Add locks to write and read operations to prevent this kind of error.
2. Use reraise to simplify the code in `__getitem__`
karajan1001 added a commit that referenced this issue Nov 8, 2022
fix: #93

Our old solution on #53, can reduce possibility of race condition, but
it still happens occasionally.

1. Add locks to write and read operations to prevent this kind of error.
2. Remove old file replacing solution.
3. Use reraise to simplify the code in `__getitem__`
pmrowla pushed a commit that referenced this issue Nov 8, 2022
fix: #93

Our old solution on #53, can reduce possibility of race condition, but
it still happens occasionally.

1. Add locks to write and read operations to prevent this kind of error.
2. Remove old file replacing solution.
3. Use reraise to simplify the code in `__getitem__`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants