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

Fix writeEventFile failure #312

Closed
wants to merge 4 commits into from
Closed

Fix writeEventFile failure #312

wants to merge 4 commits into from

Conversation

kevinslashslash
Copy link
Contributor

Cache directory can be cleared by the user (or maybe automatically) while the app is running. Call mkdirs appropriately in writeEventFile instead of the constructor

Fixes #311

@kevinslashslash
Simplify EventDiskCacheTest
73e91fc 
This does the same thing but doesn't rely on EventDiskCache calling mkdirs
@kevinslashslash
Fix writeEventFile failure
ff94bc9 
Cache directory can be cleared by the user (or maybe automatically) while
the app is running. Call mkdirs appropriately in writeEventFile instead of
the constructor
@kevinslashslash
Remove use of deleteOnExit
982de5b 
On Android this doesn't really work. Even if it did, a potentially invalid
partially written file is best immediately removed
d4rken
d4rken approved these changes Jul 18, 2021
View reviewed changes
Copy link
Member

@d4rken d4rken left a comment

Choose a reason for hiding this comment

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

Thanks.

The idea behind the deleteOnExit was that if we fail to write to the file, the delete shortly afterwards could have a high chance of just blocking. I'm fine with changing it though.

@d4rken d4rken added the bug label Jul 18, 2021
@d4rken
Copy link
Member

d4rken commented Jul 18, 2021

Hm rebase+merge via github did not work out as I expected, but your commits were added ❤️.

@d4rken d4rken closed this Jul 18, 2021
@d4rken d4rken mentioned this pull request Jul 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d4rken d4rken d4rken approved these changes

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

Successfully merging this pull request may close these issues.

EventDiskCache.writeEventFile fails if user clears cache on app while running
2 participants
@kevinslashslash @d4rken