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

Update Cache docs to mention that it's not thread safe and should be a singleton #8083

Closed
odbol opened this issue Oct 29, 2023 · 2 comments · Fixed by #8208
Closed

Update Cache docs to mention that it's not thread safe and should be a singleton #8083

odbol opened this issue Oct 29, 2023 · 2 comments · Fixed by #8208
Labels
documentation Documentation task enhancement Feature not a bug

Comments

@odbol
Copy link

odbol commented Oct 29, 2023

Quite a few people have been running into some concurrency issues with Cache (see #8080, #3938, #6280, voghDev/PdfViewPager#175, etc).

Most likely for all of them they are either creating more than one Cache object on the same directory/file, or accessing it from different threads. But it's not clear from the docs that this would cause a problem.

The doc comment for Cache should be updated to mention that it is not thread-safe and that only one Cache object per directory should be created (i.e. make it a singleton).

@odbol odbol added the bug Bug in existing code label Oct 29, 2023
@yschimke
Copy link
Collaborator

As well as some docs, we should consider something like checking a static set of locks. ExoPlayer SimpleCache does this.

https://github.com/androidx/media/blob/3f366109b7e5cbf7b7e5b46e8b923e167e9ae893/libraries/datasource/src/main/java/androidx/media3/datasource/cache/SimpleCache.java#L83C38-L83C57

@yschimke yschimke added enhancement Feature not a bug documentation Documentation task and removed bug Bug in existing code labels Oct 30, 2023
@yschimke
Copy link
Collaborator

@odbol I don't think it's lack of thread safety, I think it's not safe with multiple cache instances on the same directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation task enhancement Feature not a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants