Skip to content

Commit 498ea4f

Browse files
authored
Fix concurrency bug in the test framework that caused Firestore's transaction_extra_test.cc to hang (#426)
1 parent 57cb2fc commit 498ea4f

File tree

2 files changed

+51
-21
lines changed

2 files changed

+51
-21
lines changed

firestore/integration_test_internal/src/transaction_extra_test.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ using TransactionExtraTest = FirestoreIntegrationTest;
3030

3131
TEST_F(TransactionExtraTest,
3232
TestRetriesWhenDocumentThatWasReadWithoutBeingWrittenChanges) {
33-
SKIP_TEST_ON_IOS; // TODO(b/183294303): Fix this test on iOS.
34-
3533
DocumentReference doc1 = TestFirestore()->Collection("counter").Document();
3634
DocumentReference doc2 = TestFirestore()->Collection("counter").Document();
3735
WriteDocument(doc1, MapFieldValue{{"count", FieldValue::Integer(15)}});
@@ -71,8 +69,6 @@ TEST_F(TransactionExtraTest,
7169
}
7270

7371
TEST_F(TransactionExtraTest, TestReadingADocTwiceWithDifferentVersions) {
74-
SKIP_TEST_ON_IOS; // TODO(b/183294303): Fix this test on iOS.
75-
7672
int counter = 0;
7773
DocumentReference doc = TestFirestore()->Collection("counters").Document();
7874
WriteDocument(doc, MapFieldValue{{"count", FieldValue::Double(15.0)}});

testing/sample_framework/src/ios/ios_app_framework.mm

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,19 @@ @interface FTAViewController : UIViewController
4747
// Test Loop on iOS doesn't provide the app under test a path to save logs to, so set it here.
4848
#define GAMELOOP_DEFAULT_LOG_FILE "Results1.json"
4949

50-
static int g_exit_status = 0;
51-
static bool g_shutdown = false;
52-
static NSCondition *g_shutdown_complete;
53-
static NSCondition *g_shutdown_signal;
50+
enum class RunningState {
51+
kRunning,
52+
kShuttingDown,
53+
kShutDown
54+
};
55+
56+
// Note: g_running_state and g_exit_status must only be accessed while holding the lock from
57+
// g_running_state_condition; also, any changes to these values should be followed up with a
58+
// call to [g_running_state_condition broadcast].
59+
static NSCondition *g_running_state_condition;
60+
static RunningState g_running_state = RunningState::kRunning;
61+
static int g_exit_status = -1;
62+
5463
static UITextView *g_text_view;
5564
static UIView *g_parent_view;
5665
static FTAViewController *g_view_controller;
@@ -70,9 +79,14 @@ - (void)viewDidLoad {
7079
char *argv[1];
7180
argv[0] = new char[strlen(TESTAPP_NAME) + 1];
7281
strcpy(argv[0], TESTAPP_NAME); // NOLINT
73-
[g_shutdown_signal lock];
74-
g_exit_status = common_main(1, argv);
75-
[g_shutdown_complete signal];
82+
int common_main_result = common_main(1, argv);
83+
84+
[g_running_state_condition lock];
85+
g_exit_status = common_main_result;
86+
g_running_state = RunningState::kShutDown;
87+
[g_running_state_condition broadcast];
88+
[g_running_state_condition unlock];
89+
7690
delete[] argv[0];
7791
argv[0] = nullptr;
7892
[NSThread sleepForTimeInterval:kGameLoopSecondsToPauseBeforeQuitting];
@@ -87,9 +101,17 @@ - (void)viewDidLoad {
87101
namespace app_framework {
88102

89103
bool ProcessEvents(int msec) {
90-
[g_shutdown_signal
91-
waitUntilDate:[NSDate dateWithTimeIntervalSinceNow:static_cast<float>(msec) / 1000.0f]];
92-
return g_shutdown;
104+
NSDate* endDate = [NSDate dateWithTimeIntervalSinceNow:static_cast<float>(msec) / 1000.0f];
105+
[g_running_state_condition lock];
106+
107+
if (g_running_state == RunningState::kRunning) {
108+
[g_running_state_condition waitUntilDate:endDate];
109+
}
110+
111+
RunningState running_status = g_running_state;
112+
[g_running_state_condition unlock];
113+
114+
return running_status != RunningState::kRunning;
93115
}
94116

95117
std::string PathForResource() {
@@ -297,8 +319,13 @@ int main(int argc, char* argv[]) {
297319
close(filedes[0]);
298320
close(filedes[1]);
299321

322+
int exit_status = -1;
323+
[g_running_state_condition lock];
324+
exit_status = g_exit_status;
325+
[g_running_state_condition unlock];
326+
300327
NSLog(@"Application Exit");
301-
return g_exit_status;
328+
return exit_status;
302329
}
303330

304331
@implementation AppDelegate
@@ -317,9 +344,7 @@ - (BOOL)application:(UIApplication *)app
317344

318345
- (BOOL)application:(UIApplication*)application
319346
didFinishLaunchingWithOptions:(NSDictionary*)launchOptions {
320-
g_shutdown_complete = [[NSCondition alloc] init];
321-
g_shutdown_signal = [[NSCondition alloc] init];
322-
[g_shutdown_complete lock];
347+
g_running_state_condition = [[NSCondition alloc] init];
323348

324349
self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
325350
g_view_controller = [[FTAViewController alloc] init];
@@ -339,8 +364,17 @@ - (BOOL)application:(UIApplication*)application
339364
}
340365

341366
- (void)applicationWillTerminate:(UIApplication *)application {
342-
g_shutdown = true;
343-
[g_shutdown_signal signal];
344-
[g_shutdown_complete wait];
367+
[g_running_state_condition lock];
368+
369+
if (g_running_state == RunningState::kRunning) {
370+
g_running_state = RunningState::kShuttingDown;
371+
[g_running_state_condition broadcast];
372+
}
373+
374+
while (g_running_state != RunningState::kShutDown) {
375+
[g_running_state_condition wait];
376+
}
377+
378+
[g_running_state_condition unlock];
345379
}
346380
@end

0 commit comments

Comments
 (0)