Run WebSocket keepalive on the connection worker queue

[dajiaohuang]

Summary

• Run the WebSocket keepalive dispatch-source timer on the connection worker queue, preserving the interval/reset threshold and canceling on close/deallocation without reinstalling after close.
• Route watchOS receive completion to the worker queue before touching connection state.
• Cover worker emission/reset and close/cancellation with regressions. The retained callback uses a weak connection reference, avoiding a retain cycle and the CocoaPods warning introduced by the test.
• Update Unreleased changelog with Run WebSocket keepalive on the connection worker queue #16767.

Validation and current blockers

• clang-format 23.1.2 and git diff --check pass.
• Actual iOS verification with -Werror=arc-retain-cycles compiled the real Database unit-test target and passed all four FWebSocketConnectionTest cases, including both keepalive regressions: https://github.com/dajiaohuang/firebase-ios-sdk/actions/runs/37102380275/job/111144293128 .
• In current upstream run 37102421987, the reported DatabaseUnit, SPM and pod-lint matrix checks pass. The Database quickstart job fails SwiftPM dependency resolution: firebase-ios-sdk requires gtm-session-fetcher 4.0.0..<6.0.0, while FirebaseUI 16 through GoogleSignIn 7 requires 1.5.0..<4.0.0. This PR changes no dependency declarations, so that failure does not validate the WebSocket change.
• fix(database): report connection failure when the connect attempt times out #16725 remains OPEN. The requested rebase after its merge remains pending; full Database/watchOS integration is not claimed.

[gemini-code-assist]

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 the 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 counterproductive. 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.

[paulb777]

Thanks for the contribution! A few housekeeping items:

• CHANGELOG: The entry in FirebaseDatabase/CHANGELOG.md was added above # 12.19.0 without a heading, so it reads as part of a release that has already shipped. Please move it under a # Unreleased heading at the top of the file (add the heading if it isn't there yet).
• PR number: end the entry with this PR's number, e.g. - [fixed] Description. (#16767).
• Formatting: after updating, please run scripts/style.sh so the infra.check job passes. It needs clang-format 23 and swiftformat (see scripts/setup_check.sh). If you can't run it locally, the infra.check log lists the files that need formatting.

[paulb777]

Thanks for looking at this. I agree the keepalive NSTimer is touched from both the main thread and the worker queue, but I don't think this change has the right shape yet:

• resetKeepAlive now dispatches a block to main for every sent message and every received frame, where before it was a cheap inline check on the worker queue.
• The cross-thread access that matters more is still there (see inline).
• It overlaps with #16725, which changes the same source and test files (the test file would conflict), so please rebase after that lands.

A cleaner approach would be to replace the NSTimer with a dispatch_source_t timer on self.dispatchQueue. Then the timer, isClosed and webSocket are only touched on the worker queue, and the main thread drops out of the picture.

[dajiaohuang]

Replaced the NSTimer/main-queue approach with a dispatch-source timer on the connection's worker queue. The timer callback reads connection state and sends the backend's "0" on that same queue. Resetting its deadline is inline and retains the five-second reset threshold; close cancels the timer, and a closed connection cannot reinstall it. The event handler holds self weakly and deallocation cancels any remaining timer. The watchOS receive completion also hops to the worker queue before handling frames or connection state.

Replaced the main-thread timer-spy test with worker-queue emission/inline-reset and cancellation/no-reinstallation regressions. Moved the changelog under Unreleased/#16767, and clang-format 23.1.2 plus git diff --check pass. These new XCTest cases have not run on this Windows host; native Apple CI is required.

#16725 is still OPEN, so the requested rebase after it lands remains pending. I kept the updates on this existing PR rather than adding an overlapping PR.

[paulb777]

/gemini review

[gemini-code-assist]

Code Review

This pull request migrates the Realtime Database WebSocket keepalive timer from NSTimer on the main queue to a dispatch source timer (dispatch_source_t) running on the worker queue to prevent cross-thread access. It also synchronizes access to the connection state (isClosed) and updates incoming frame handling to run on the worker queue. However, changing the signature of -nop: to -nop leaves an undeclared timer variable in the method's else block, which will cause a compilation error. The obsolete else block should be removed entirely.

[paulb777]

Please address CI failures

[dajiaohuang]

Downloaded the native DatabaseUnit iOS xcodebuild artifact. It fails compilation at FWebSocketConnection.m:275 because the closed-connection branch still references the removed NSTimer variable timer.

Fixed that branch to cancel and clear the actual keepAlive dispatch source and its fire date, matching the existing shutdown path. clang-format 23.1.2 and git diff --check pass. Apple builds/tests are unavailable locally; the updated native CI result remains pending.

[dajiaohuang]

The compile fix is pushed at 73b34fb. The current sdk.database run has not executed: GitHub reports action_required for run 37086587716, along with the other PR workflows. Only scanners and CLA currently report success. Could you approve the new workflow runs so the Database compile and tests can validate this correction?

[dajiaohuang]

The removed-timer compile repair now has actual Apple build/test evidence at the unchanged PR source73b34fb359777f9a87ea09ff2bc6445d5c176dc8. All4 FWebSocketConnectionTest cases pass on both macOS and an iOS simulator, including dispatch-source worker emission/reset and cancellation/no-reinstallation after close.

macOS: https://github.com/dajiaohuang/firebase-ios-sdk/actions/runs/37092391926 . iOS: https://github.com/dajiaohuang/firebase-ios-sdk/actions/runs/37092594582 . The separate fork validation commits only add/change the workflow, preserving PR production and test sources; each run retains logs and xcresult. This is focused native unit evidence, not full Database/watchOS integration. The upstream workflow approval request and the requested rebase after #16725 merges remain outstanding; #16725 is still OPEN.

[dajiaohuang]

Fixed the CocoaPods validation warning in the retained test callback at3103154f36451c637529aae0853799f705932152: weakly capture the connection, promote it inside the callback, assert nonnil, and preserve the actual worker-send/reset/close assertions. Production code is unchanged.

Actual Xcode26.4/iOS validation compiled the real Database unit-test target with -Werror=arc-retain-cycles and executed all four FWebSocketConnectionTest cases with zero failures. Native run37102380275/job111144293128: https://github.com/dajiaohuang/firebase-ios-sdk/actions/runs/37102380275/job/111144293128 . This proves the specific new warning is removed and the timer regressions still pass; full Database/watchOS acceptance is not claimed. #16725 is still OPEN, so the requested post-merge rebase remains pending.

Upstream workflows on this exact head ended action_required; please approve the updated head so the full upstream matrix can run.