Skip to content

Commit ed1a551

Browse files
LyokoneLyokone
authored
fix(firestore,windows): Prevents lost Windows transaction responses (#18448)
1 parent 068c309 commit ed1a551

3 files changed

Lines changed: 112 additions & 22 deletions

File tree

‎packages/cloud_firestore/cloud_firestore/example/integration_test/transaction_e2e.dart‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,27 @@ void runTransactionTests() {
6969
expect(response, equals(randomValue));
7070
});
7171

72+
test(
73+
'runs after reading a document',
74+
() async {
75+
final documentReference =
76+
await initializeTest('transaction-after-get');
77+
await documentReference.set({'value': 0});
78+
await documentReference.get();
79+
80+
await firestore.runTransaction((transaction) async {
81+
final snapshot = await transaction.get(documentReference);
82+
transaction.update(documentReference, {
83+
'value': snapshot.data()!['value'] + 1,
84+
});
85+
});
86+
87+
final snapshot = await documentReference.get();
88+
expect(snapshot.data()!['value'], 1);
89+
},
90+
skip: defaultTargetPlatform != TargetPlatform.windows,
91+
);
92+
7293
test('should abort if thrown and not continue', () async {
7394
DocumentReference<Map<String, dynamic>> documentReference =
7495
await initializeTest('transaction-abort');

‎packages/cloud_firestore/cloud_firestore/windows/cloud_firestore_plugin.cpp‎

Lines changed: 62 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,48 @@ namespace cloud_firestore_windows {
4444

4545
static std::string kLibraryName = "flutter-fire-fst";
4646

47+
void TransactionResponse::Reset() {
48+
std::lock_guard<std::mutex> lock(mutex_);
49+
response_received_ = false;
50+
commands_.clear();
51+
}
52+
53+
void TransactionResponse::Complete(
54+
InternalTransactionResult result,
55+
std::vector<InternalTransactionCommand> commands) {
56+
{
57+
std::lock_guard<std::mutex> lock(mutex_);
58+
result_ = result;
59+
commands_ = std::move(commands);
60+
response_received_ = true;
61+
}
62+
condition_.notify_one();
63+
}
64+
65+
TransactionResponseStatus TransactionResponse::WaitFor(
66+
std::chrono::milliseconds timeout, InternalTransactionResult& result,
67+
std::vector<InternalTransactionCommand>& commands) {
68+
std::unique_lock<std::mutex> lock(mutex_);
69+
if (!condition_.wait_for(
70+
lock, timeout, [this] { return response_received_ || cancelled_; })) {
71+
return TransactionResponseStatus::kTimedOut;
72+
}
73+
if (cancelled_) {
74+
return TransactionResponseStatus::kCancelled;
75+
}
76+
result = result_;
77+
commands = std::move(commands_);
78+
return TransactionResponseStatus::kReceived;
79+
}
80+
81+
void TransactionResponse::Cancel() {
82+
{
83+
std::lock_guard<std::mutex> lock(mutex_);
84+
cancelled_ = true;
85+
}
86+
condition_.notify_one();
87+
}
88+
4789
namespace {
4890

4991
constexpr wchar_t kTaskRunnerWindowClassName[] =
@@ -996,10 +1038,7 @@ class TransactionStreamHandler
9961038
void ReceiveTransactionResponse(
9971039
InternalTransactionResult resultType,
9981040
std::vector<InternalTransactionCommand> commands) {
999-
std::lock_guard<std::mutex> lock(commands_mutex_);
1000-
resultType_ = resultType;
1001-
commands_ = commands;
1002-
cv_.notify_one();
1041+
response_.Complete(resultType, std::move(commands));
10031042
}
10041043

10051044
std::unique_ptr<flutter::StreamHandlerError<flutter::EncodableValue>>
@@ -1024,25 +1063,31 @@ class TransactionStreamHandler
10241063

10251064
flutter::EncodableMap map;
10261065
map.emplace("appName", firestore_->app()->name());
1066+
response_.Reset();
10271067
SendSuccessOnPlatformThread(events_state_,
10281068
flutter::EncodableValue(map));
10291069

1030-
std::unique_lock<std::mutex> lock(mtx_);
1031-
if (cv_.wait_for(lock, std::chrono::milliseconds(timeout_)) ==
1032-
std::cv_status::timeout) {
1033-
SendErrorOnPlatformThread(events_state_, "Timeout",
1034-
"Transaction timed out.",
1035-
flutter::EncodableValue(), true);
1036-
return Error::kErrorDeadlineExceeded;
1070+
InternalTransactionResult resultType;
1071+
std::vector<InternalTransactionCommand> commands;
1072+
switch (response_.WaitFor(std::chrono::milliseconds(timeout_),
1073+
resultType, commands)) {
1074+
case TransactionResponseStatus::kTimedOut:
1075+
SendErrorOnPlatformThread(events_state_, "Timeout",
1076+
"Transaction timed out.",
1077+
flutter::EncodableValue(), true);
1078+
return Error::kErrorDeadlineExceeded;
1079+
case TransactionResponseStatus::kCancelled:
1080+
return Error::kErrorCancelled;
1081+
case TransactionResponseStatus::kReceived:
1082+
break;
10371083
}
10381084

1039-
std::lock_guard<std::mutex> command_lock(commands_mutex_);
1040-
if (resultType_ == InternalTransactionResult::kFailure) {
1085+
if (resultType == InternalTransactionResult::kFailure) {
10411086
return Error::kErrorAborted;
10421087
}
1043-
if (commands_.empty()) return Error::kErrorOk;
1088+
if (commands.empty()) return Error::kErrorOk;
10441089

1045-
for (InternalTransactionCommand& command : commands_) {
1090+
for (InternalTransactionCommand& command : commands) {
10461091
std::string path = command.path();
10471092
InternalTransactionType type = command.type();
10481093
if (path.empty() /* or some other invalid condition */) {
@@ -1114,8 +1159,7 @@ class TransactionStreamHandler
11141159

11151160
std::unique_ptr<flutter::StreamHandlerError<flutter::EncodableValue>>
11161161
OnCancelInternal(const flutter::EncodableValue* arguments) override {
1117-
std::unique_lock<std::mutex> lock(mtx_);
1118-
cv_.notify_one();
1162+
response_.Cancel();
11191163
EndStreamOnPlatformThread(events_state_);
11201164
return nullptr;
11211165
}
@@ -1125,11 +1169,7 @@ class TransactionStreamHandler
11251169
long timeout_;
11261170
int maxAttempts_;
11271171
std::string transactionId_;
1128-
std::vector<InternalTransactionCommand> commands_;
1129-
InternalTransactionResult resultType_ = InternalTransactionResult::kSuccess;
1130-
std::mutex mtx_;
1131-
std::mutex commands_mutex_;
1132-
std::condition_variable cv_;
1172+
TransactionResponse response_;
11331173
std::shared_ptr<EventSinkState> events_state_;
11341174
};
11351175

‎packages/cloud_firestore/cloud_firestore/windows/cloud_firestore_plugin.h‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@
1111
#include <flutter/method_channel.h>
1212
#include <flutter/plugin_registrar_windows.h>
1313

14+
#include <chrono>
15+
#include <condition_variable>
1416
#include <memory>
17+
#include <mutex>
18+
#include <vector>
1519

1620
#include "firebase/app.h"
1721
#include "firebase/firestore.h"
@@ -21,6 +25,31 @@
2125

2226
namespace cloud_firestore_windows {
2327

28+
enum class TransactionResponseStatus {
29+
kReceived,
30+
kTimedOut,
31+
kCancelled,
32+
};
33+
34+
class TransactionResponse {
35+
public:
36+
void Reset();
37+
void Complete(InternalTransactionResult result,
38+
std::vector<InternalTransactionCommand> commands);
39+
TransactionResponseStatus WaitFor(
40+
std::chrono::milliseconds timeout, InternalTransactionResult& result,
41+
std::vector<InternalTransactionCommand>& commands);
42+
void Cancel();
43+
44+
private:
45+
std::mutex mutex_;
46+
std::condition_variable condition_;
47+
bool response_received_ = false;
48+
bool cancelled_ = false;
49+
InternalTransactionResult result_ = InternalTransactionResult::kSuccess;
50+
std::vector<InternalTransactionCommand> commands_;
51+
};
52+
2453
class CloudFirestorePlugin : public flutter::Plugin,
2554
public FirebaseFirestoreHostApi {
2655
public:

0 commit comments

Comments
 (0)