Skip to content

fix(android, firestore): detach snapshot listeners before executor shutdown#8940

Merged
mikehardy merged 1 commit intoinvertase:mainfrom
christian-apollo:fix/android-firestore-invalidate-listener-order
Mar 31, 2026
Merged

fix(android, firestore): detach snapshot listeners before executor shutdown#8940
mikehardy merged 1 commit intoinvertase:mainfrom
christian-apollo:fix/android-firestore-invalidate-listener-order

Conversation

@christian-apollo
Copy link
Copy Markdown
Contributor

@christian-apollo christian-apollo commented Mar 25, 2026

Summary

Removes Firestore snapshot listeners before super.invalidate() in ReactNativeFirebaseFirestoreCollectionModule and ReactNativeFirebaseFirestoreDocumentModule.

ReactNativeFirebaseModule.invalidate() shuts down TaskExecutorService; sendOnSnapshotEvent uses Tasks.call(getTransactionalExecutor(...), ...). If the executor is terminated first, a late snapshot can throw RejectedExecutionException (terminated pool).

This matches the teardown order already used in ReactNativeFirebaseFirestoreTransactionModule.

Related issue

Test plan

  • Android: enable Firestore onSnapshot, trigger RN reload or scenario that invalidates native modules; confirm no crash (manual / existing e2e if any)

Made with Cursor

…utdown

Remove Firestore listeners before super.invalidate() so TaskExecutorService
is not shut down while snapshot callbacks may still run sendOnSnapshotEvent
(Tasks.call on a transactional executor).

Aligns with ReactNativeFirebaseFirestoreTransactionModule teardown order.

Fixes invertase#8939

Made-with: Cursor
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 25, 2026

@christian-apollo is attempting to deploy a commit to the Invertase Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 25, 2026

CLA assistant check
All committers have signed the CLA.

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, 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 addresses a potential crash in Android when React Native modules are invalidated. By adjusting the order of operations during module teardown, it ensures that Firestore snapshot listeners are properly unregistered before the underlying executor service is shut down, thereby preventing runtime exceptions and improving application stability.

Highlights

  • Firestore Snapshot Listener Teardown: Reordered the invalidation process in ReactNativeFirebaseFirestoreCollectionModule and ReactNativeFirebaseFirestoreDocumentModule to ensure Firestore snapshot listeners are detached before super.invalidate() is called, preventing RejectedExecutionException during module shutdown.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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.

Footnotes

  1. 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.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a 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 refactors the invalidate method in ReactNativeFirebaseFirestoreCollectionModule and ReactNativeFirebaseFirestoreDocumentModule. The super.invalidate() call is moved to the end of the method to ensure that module-specific listener cleanup and collection clearing occur before the superclass's invalidation logic. The review comments suggest an improvement to iterate SparseArrays using valueAt(i) for better readability and slightly improved performance.

@mikehardy
Copy link
Copy Markdown
Collaborator

Hey @christian-apollo 👋 - thanks for this - could you follow the link in either the comment or the CI check to read+accept the CLA? Happy to collaborate and merge but we have to have the CLA signed to take in code, thanks!

Copy link
Copy Markdown
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Looks like a nice fix

I'm okay with ignoring the suggested readability/performance optimizations from LLM review as I do not believe this will execute very frequently at all nor do I think the set of listeners will ever be large, it just doesn't seem consequential

waiting on CLA approval

@christian-apollo
Copy link
Copy Markdown
Contributor Author

signed!

Copy link
Copy Markdown
Collaborator

@MichaelVerdon MichaelVerdon left a comment

Choose a reason for hiding this comment

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

LGTM thanks for the PR

@mikehardy mikehardy merged commit 649e7ee into invertase:main Mar 31, 2026
19 of 20 checks passed
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.

fix(android, firestore): RejectedExecutionException in sendOnSnapshotEvent during invalidate

4 participants