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

services: fix START_STICKY #2401

Merged
merged 6 commits into from
Jan 14, 2022
Merged

services: fix START_STICKY #2401

merged 6 commits into from
Jan 14, 2022

Conversation

mzakharo
Copy link
Contributor

@mzakharo mzakharo commented Jan 5, 2021

This patchet is fixing a Java stack-trace during a restart of sticky service by the OS.

Specifically:
E AndroidRuntime: Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'android.os.Bundle android.content.Intent.getExtras()' on a null object reference

To reproduce, one can force a system memory pressure event by allocating a bunch of ram in another process.

Sticky background services + ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS allows for creating resilient background services that get auto-restarted by Android OS under memory/resource pressure, and thus can outlast normal services created using the official auto-restart documentation

This patchset is inspired by this stackoverflow post

Copy link
Member

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

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

Thanks for investigating and fixing. I left few comments/questions, but I think we're already close to getting something we can merge.
Another path that might be worth investigating is the START_REDELIVER_INTENT bit.
If I understood correctly, it may behave like the START_STICKY, but providing the intent this time. That would make the diff much shorter as just changing START_STICKY to START_REDELIVER_INTENT. That's something to investigate

AndreMiras
AndreMiras previously approved these changes Jan 6, 2021
Copy link
Member

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

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

Thanks for the clean up and the feedback on the comments.
I think we're OK. I'll leave it there a few days before merging in case someone (@inclement?) has something to say.
This part of the code is not heavily used, nor tested, it has few edge cases that're easy to overlook

@mzakharo
Copy link
Contributor Author

Turns out there are usecases where both foreground + STICKY is useful, in which case, the original CL was inusfficient.

latest CL addresses the foreground service support, ads a comment for why we dont turn off sticky services on app termination.

Also, this adds a new getDefaultIntet method, which removes logic duplication while starting a service from a custom broadcast receiver.

        ix = ServiceMycustomservice.getDefaultIntent(context, "");
        ix.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
        context.startService(ix);

@mzakharo mzakharo changed the title fix sticky (background) services services: fix START_STICKY Jan 15, 2021
@mzakharo
Copy link
Contributor Author

mzakharo commented Jan 13, 2022

Can someone please review this PR?

Copy link
Member

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

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

Thanks for updating with latest develop change. I usually prefer a rebase tho.
I will squash & merge

@AndreMiras AndreMiras merged commit 4ecaa5f into kivy:develop Jan 14, 2022
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.

2 participants