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

[BUG]: InMemoryBlockingCache is reusing its own CoroutineScope without a JobSupervisor #5355

Open
masclot opened this issue Mar 10, 2024 · 1 comment
Open

[BUG]: InMemoryBlockingCache is reusing its own CoroutineScope without a JobSupervisor #5355

masclot opened this issue Mar 10, 2024 · 1 comment
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Milestone
1.0 Global availa...

Comments

@masclot
Copy link
Collaborator

masclot commented Mar 10, 2024 •
edited

Describe the bug

InMemoryBlockingCache operates asynchronously on its own CoroutineScope. This scope is not configured with a JobSupervisor, nor are all operations protected with try/catch. Thus, if any operation fails, any follow up operation will also fail. Since InMemoryBlockingCache notifies change observers synchronously, if any change observer fails then the cache will stop working.

Steps To Reproduce

Theoretical problem. Might be related to #5063.

Expected Behavior

Failures in a change observer should not affect the future operations in the cache.

Screenshots/Videos

No response

What device/emulator are you using?

No response

Which Android version is your device/emulator running?

No response

Which version of the Oppia Android app are you using?

No response

Additional Context

No response
@masclot masclot added bug End user-perceivable behaviors which are not desirable. triage needed labels Mar 10, 2024
@BenHenning
Copy link
Sponsor Member

Ah very nice catch. I'll be honest, I introduced InMemoryBlockingCache before I fully understood how to manage CoroutineScopes :). I've actually run into the cascading failure problem which led to a lot of changes elsewhere in the app with scope management; I guess this class was missed in that effort.

@BenHenning BenHenning added this to the 1.0 Global availability milestone Mar 11, 2024
@adhiamboperes adhiamboperes added Impact: Low Low perceived user impact (e.g. edge cases). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. and removed triage needed labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Projects
[Team] Developer Workflow & Infrastructure - Android
Status: Todo
Milestone
1.0 Global availability
Development

No branches or pull requests
3 participants
@BenHenning @adhiamboperes @masclot