-
Notifications
You must be signed in to change notification settings - Fork 640
Restore SessionLifecycleService as no-op #7318
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
base: main
Are you sure you want to change the base?
Restore SessionLifecycleService as no-op #7318
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @jrodiz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request resolves a critical ClassNotFoundException affecting certain Android 14/15 devices following a Firebase SDK upgrade. The issue stemmed from the Android system's attempt to interact with a previously removed service. The solution involves re-introducing this service as a disabled, no-operational component to prevent crashes, while also outlining a safer, phased approach for future service deprecations.
Highlights
- Problem Identification: Addresses a ClassNotFoundException occurring on specific Android 14/15 devices (Samsung/Oppo) after upgrading to Firebase BoM 34.1.0, which was linked to the prior removal of the
SessionLifecycleService
. - Root Cause Analysis: The crash is attributed to Android's system caching or state retention, where the OS attempts to re-establish connections to the previously existing
SessionLifecycleService
even after it had been removed from the application's code. - Solution Implementation: Re-introduces the
SessionLifecycleService
as a no-operational class. ItsonBind
method now immediately returnsnull
, and its manifest entry is explicitly disabled (android:enabled="false"
) to prevent the system from attempting to instantiate or bind to it in a meaningful way, thus resolving the crash. - Future Mitigation Strategy: Proposes a refined two-step process for future service removals: first, deprecate and disable the service in one version to allow for graceful system state updates, and then fully remove it in a subsequent version.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request restores the SessionLifecycleService
as a no-op service to address a ClassNotFoundException
on specific Android devices. The implementation correctly re-introduces the service class with an onBind
method that returns null
and disables the service in the AndroidManifest.xml
via android:enabled="false"
. This is a solid workaround for the described issue. I have a couple of minor suggestions to improve code style and adhere to Kotlin conventions.
|
||
internal class SessionLifecycleService : Service() { | ||
|
||
override fun onBind(p0: Intent?): IBinder? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter p0
in the onBind
method is unused. According to Kotlin coding conventions, it's idiomatic to name unused parameters with an underscore (_
) to improve code clarity and explicitly mark them as unused.1
override fun onBind(p0: Intent?): IBinder? { | |
override fun onBind(_: Intent?): IBinder? { |
Style Guide References
Footnotes
-
The official Kotlin style guide suggests using an underscore for lambda parameters that are not used. This practice is widely extended to any unused function or method parameter to signal that it is intentionally ignored. ↩
const val TAG = "SessionLifecycleService" | ||
|
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is missing a newline at the end. It's a widely-accepted convention to end files with a single newline character. This prevents issues with certain tools and version control systems, and ensures file content is handled consistently.1
} | |
} | |
Style Guide References
Footnotes
-
Many style guides and POSIX standards recommend that all text files end with a newline character. This ensures proper handling by various command-line tools and prevents unexpected behavior during file processing or concatenation. ↩
In context of class not found exception: #7296
This PR restore a previously deleted service as no operational
The user reported a crash after upgrading to BoM 34.1.0 only for some Samsung and Oppo devices in Android 14/15. https://firebase.google.com/support/release-notes/android
There was a major refactor on the way of monitoring application lifecycle events and determining when/if a new session should be generated. Which was the main purpose for
SessionLifecycleService
. Firebase Session is used by Crashlytics and Performance internally to measure sessionsThe refactor proposes the elimination of the service
SessionLifecycleService
in favor ofSharedSessionRepository
.This refactor was released on version
releases/m167
(see commit here).See diff of m167 with m166 here.
Looking for
SessionLifecycleService
usages into the SDK I found just a leftover reference in an Android Test Manifest. So I can conclude this is not production code.Reviewing the original
SessionLifecycleService
class I found out this service contains aBinder
implementationIt sounds like the Android system on those specific devices (Samsung/Oppo on Android 14/15) might be trying to recreate the
SessionLifecycleService
based on a cached manifest from the previous version of the host app, even though the service class itself is no longer present in the new version. This leads to theClassNotFoundException
.Conclusions:
A) Stale Service Connections (I see this as the root cause, I’d like to review g3 code to confirm there is not source code having references to the removed service):
If any component within the host app (or the SDK itself) was bound to
SessionLifecycleService
and theServiceConnection
wasn't properly released or unbound before the app update (or if the system is trying to restore a previous binding state), the system will attempt to callonBind
on the service.When it tries to instantiate
SessionLifecycleService
to do this and finds the class missing, it results in theClassNotFoundException
.The git diff showed
android:exported="false"
for the service, which is good as it means only the SDK could bind to it.Why this still leads to
ClassNotFoundException
on update:Even if the new code doesn't try to bind to the removed service anymore, the Android system might still attempt to re-establish connections that it considers active or pending from the previous app session, especially if the app process didn't terminate cleanly or if the system is trying to restore the app's state after an update. If a
ServiceConnection
was active when the app was last running the old version, the system might try to callonServiceConnected
for that connection, which requires instantiating the service.tl; dr:
B) Android /System Caching/State Retention:
The Android Package Manager or Activity Manager on certain devices/OS versions might retain information about previously registered services.
If the service was started in a "sticky" way, the system will try to restart it after it's killed or after an app update if it believes the service should still be running. When it tries to do so with the updated app that lacks the service class, it crashes.
C) Stale System State: Sometimes, especially with certain OEM customizations or newer Android versions, the system's package manager or activity manager might not perfectly clear all references to components from an old app version immediately upon an update. For users experiencing this: The most reliable fix is often a clean reinstall.
D) OEM Customizations: Samsung and Oppo devices have their own modifications to Android, which can sometimes lead to different behaviors in how app updates and service lifecycles are handled.
Proposed workaround:
Future Service Removals (Refined Mitigation Strategy for Bound Services):
Version N+1 (Graceful Shutdown & Deprecation): Return the service class in the codebase, modify its onBind(Intent intent) method to return null immediately. This signals to any clients attempting to bind that the service is not available or is shutting down. Crucially, mark the service as
android:enabled="false"
in the AndroidManifest.xml. This explicitly tells the system the service should not be started or bound.Version N+2 (Full Removal): Now that clients have had a version where the service gracefully refuses bindings and is disabled, it's safer to remove the service class and its manifest entry.