Repository navigation
Desktop RTDB: a transaction callback abort on a rerun completes the Future with kErrorNone #1905
Description
Environment
- Firebase C++ SDK 13.9.0 (desktop; the cited code is unchanged at the 13.11.0 tag)
Summary
On desktop, if the DoTransaction callback aborts on the first invocation, the future completes with kErrorWriteCanceled (11) — see Repo::StartTransaction:
firebase-cpp-sdk/database/src/desktop/core/repo.cc
Lines 659 to 729 in 3d7ce2a
| void Repo::StartTransaction(const Path& path, | |
| DoTransactionWithContext transaction_function, | |
| void* context, void (*delete_context)(void*), | |
| bool trigger_local_events, | |
| ReferenceCountedFutureImpl* api, | |
| SafeFutureHandle<DataSnapshot> handle) { | |
| // Make sure we're listening on this node. | |
| // Note: we can't do this asynchronously. To preserve event ordering, it has | |
| // to be done in this block. This is ok, this block is guaranteed to be our | |
| // own event loop | |
| DatabaseReferenceInternal* ref_impl = | |
| new DatabaseReferenceInternal(database_, path); | |
| DatabaseReference watch_ref(ref_impl); | |
| std::unique_ptr<NoopListener> listener = std::make_unique<NoopListener>(); | |
| NoopListener* listener_ptr = listener.get(); | |
| QuerySpec query_spec(path); | |
| AddEventCallback(std::make_unique<ValueEventRegistration>( | |
| database_, listener_ptr, query_spec)); | |
| TransactionDataPtr transaction_data = std::make_shared<TransactionData>( | |
| handle, api, query_spec.path, transaction_function, context, | |
| delete_context, trigger_local_events, std::move(listener)); | |
| // Run transaction initially. | |
| Variant current_state = GetLatestState(path); | |
| transaction_data->current_input_snapshot = current_state; | |
| MutableDataInternal* mutable_data_impl = | |
| new MutableDataInternal(database_, current_state); | |
| MutableData mutable_current(mutable_data_impl); | |
| TransactionResult result = transaction_function(&mutable_current, context); | |
| if (result != kTransactionResultSuccess) { | |
| // Abort the transaction. | |
| transaction_data->current_output_snapshot_raw = Variant::Null(); | |
| transaction_data->current_output_snapshot_resolved = Variant::Null(); | |
| transaction_data->status = TransactionData::kStatusNeedsAbort; | |
| transaction_data->ref_future->Complete(transaction_data->future_handle, | |
| kErrorWriteCanceled); | |
| // If there was an error, the listener must be removed to prevent calls to | |
| // it in case the listener is destroyed. | |
| RemoveEventCallback(listener_ptr, query_spec); | |
| } else { | |
| // Mark as run and add to our queue. | |
| transaction_data->status = TransactionData::kStatusRun; | |
| auto* queue_node = transaction_queue_tree_.GetOrMakeSubtree(path); | |
| if (!queue_node->value().has_value()) { | |
| queue_node->set_value(std::vector<TransactionDataPtr>()); | |
| } | |
| queue_node->value()->push_back(transaction_data); | |
| Variant server_values = GenerateServerValues(server_time_offset_); | |
| const Variant* new_node_unresolved = mutable_data_impl->GetNode(); | |
| Variant new_node_resolved = | |
| ResolveDeferredValueSnapshot(*new_node_unresolved, server_values); | |
| transaction_data->current_output_snapshot_raw = *new_node_unresolved; | |
| transaction_data->current_output_snapshot_resolved = new_node_resolved; | |
| transaction_data->current_write_id = GetNextWriteId(); | |
| std::vector<Event> events = server_sync_tree_->ApplyUserOverwrite( | |
| path, *new_node_unresolved, new_node_resolved, | |
| transaction_data->current_write_id, | |
| trigger_local_events ? kOverwriteVisible : kOverwriteInvisible, | |
| kDoNotPersist); | |
| PostEvents(events); | |
| SendAllReadyTransactions(); | |
| } | |
| } |
But if the abort happens on a rerun invocation (after a datastale response forces the transaction to run again), the abort reason is taken from a local error variable that was initialized to kErrorNone:
RerunTransactionQueue()initializesError error = kErrorNoneat line 1067. If the rerun callback returns abort, line 1095 assigns that unchanged zero value to the function-localabort_reason.- Lines 1111–1112 copy
abort_reasonand the current input intoFutureToComplete. No later assignment changes the reason: line 1129 references the queued value, and lines 1131–1134 complete the Future withCompleteWithResult(..., abort_reason, snapshot).
firebase-cpp-sdk/database/src/desktop/core/repo.cc
Lines 1062 to 1138 in 3d7ce2a
| transaction->current_input_snapshot = current_input; | |
| MutableDataInternal* mutable_data_impl = | |
| new MutableDataInternal(database_, current_input); | |
| MutableData mutable_data(mutable_data_impl); | |
| Error error = kErrorNone; | |
| TransactionResult result = transaction->transaction_function( | |
| &mutable_data, transaction->context); | |
| if (result == kTransactionResultSuccess) { | |
| WriteId old_write_id = transaction->current_write_id; | |
| Variant server_values = GenerateServerValues(server_time_offset_); | |
| Variant* new_data_node = mutable_data_impl->GetNode(); | |
| Variant new_node_resolved = | |
| ResolveDeferredValueSnapshot(*new_data_node, server_values); | |
| transaction->current_output_snapshot_raw = *new_data_node; | |
| transaction->current_output_snapshot_resolved = new_node_resolved; | |
| transaction->current_write_id = GetNextWriteId(); | |
| sets_to_ignore.push_back(old_write_id); | |
| Extend(&events, | |
| server_sync_tree_->ApplyUserOverwrite( | |
| transaction->path, *new_data_node, new_node_resolved, | |
| transaction->current_write_id, | |
| transaction->trigger_local_events ? kOverwriteVisible | |
| : kOverwriteInvisible, | |
| kPersist)); | |
| Extend(&events, server_sync_tree_->AckUserWrite( | |
| old_write_id, kAckRevert, kDoNotPersist, | |
| server_time_offset_)); | |
| } else { | |
| abort_transaction = true; | |
| abort_reason = error; | |
| Extend(&events, server_sync_tree_->AckUserWrite( | |
| transaction->current_write_id, kAckRevert, | |
| kDoNotPersist, server_time_offset_)); | |
| } | |
| } | |
| } | |
| PostEvents(events); | |
| if (abort_transaction) { | |
| transaction->status = TransactionData::kStatusComplete; | |
| DatabaseReferenceInternal* database_ref_impl = | |
| new DatabaseReferenceInternal(database_, path); | |
| DatabaseReference ref(database_ref_impl); | |
| futures_to_complete.push_back(FutureToComplete( | |
| transaction, abort_reason, transaction->current_input_snapshot)); | |
| // Removing a callback can trigger pruning which can muck with | |
| // merged_data/visible_data (as it prunes data). So defer removing the | |
| // callback until later. | |
| Repo::scheduler().Schedule(NewCallback( | |
| [](Repo* repo, TransactionDataPtr transaction) { | |
| repo->RemoveEventCallback(transaction->outstanding_listener.get(), | |
| QuerySpec(transaction->path)); | |
| }, | |
| this, transaction)); | |
| } | |
| } | |
| PruneCompletedTransactions(&transaction_queue_tree_); | |
| for (auto& future_to_complete : futures_to_complete) { | |
| TransactionDataPtr& transaction = future_to_complete.transaction; | |
| Error& abort_reason = future_to_complete.abort_reason; | |
| Variant& node = future_to_complete.node; | |
| DataSnapshot snapshot(new DataSnapshotInternal( | |
| database_, node, QuerySpec(transaction->path))); | |
| transaction->ref_future->CompleteWithResult(transaction->future_handle, | |
| abort_reason, snapshot); | |
| } | |
| SendAllReadyTransactions(); | |
| } |
The result: the Future completes with error() == kErrorNone and a snapshot — indistinguishable from a successful commit — for a transaction that was aborted and never committed.
Expected
A rerun-invocation abort should complete with the same abort code as a first-invocation abort (or, better, with kErrorTransactionAbortedByUser to match the mobile SDKs), never with kErrorNone.
Impact
Consumers that branch on the future's error to decide "did my transaction commit?" will believe an aborted transaction succeeded whenever the abort happened after a datastale rerun — e.g. optimistic/fenced updates that abort once fresh data shows another writer won. In the FlutterFire Windows plugin this surfaces as TransactionResult(committed: true) for an aborted transaction.
Related
Filed alongside a second desktop transaction-error report: Repo::HandleTransactionResponse collapses every non-datastale server error (including permission_denied) to kErrorUnknownError with an empty message. Both were found while debugging the same Windows application.
Activity
NH-Mason commentedon Aug 8, 2026
Related: #1904 (non-datastale server errors collapse to kErrorUnknownError with an empty message) and firebase/flutterfire#18549 (Windows plugin surface: initial-invocation aborts throw; a rerun abort hitting this defect instead reports committed: true).
NH-Mason commentedon Aug 9, 2026
Clarification on verification status: this report's rerun-abort result is source-analysis-derived at the pinned 13.9.0 commit
3d7ce2a; [NEEDS VERIFICATION: a runtime capture that forces adatastalererun and then aborts] does not yet exist. Our retained Windows runtime capture covers the adjacent, different initial-invocation abort path and shows the resultingFirebaseException(code: 'unknown', message: ''); the pinned source shows that path completes with native error 11 (kErrorWriteCanceled) and no message. For #1905, the source-derived chain is thatRerunTransactionQueue()initializeserrortokErrorNone, copies that unchanged value into the callback-abort reason, and ultimately callsCompleteWithResultwith error 0. We have now re-verified those statements line by line at3d7ce2a. We also compared the entiredatabase/src/desktop/core/repo.ccfile at Firebase C++ SDK 13.11.0 and found it byte-identical, so the analyzed control flow is unchanged there. A controlled runtime capture would strengthen the report, but the source-level defect remains: the aborted rerun path can reach Future completion with the SDK's success code.