Skip to content

Commit 0b6fbae

Browse files
beekldbeekld
committed
test: fix EventCollector double-free race in sse client tests
1 parent 136aca7 commit 0b6fbae

1 file changed

Lines changed: 35 additions & 32 deletions

File tree

‎libs/server-sent-events/tests/client_test.cpp‎

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ class EventCollector {
9292
std::vector<Error> errors_;
9393
};
9494

95-
// Helper to run io_context in background thread
95+
// Helper to run io_context in background thread. ~IoContextRunner joins the
96+
// worker thread, so any state captured by receiver callbacks (e.g. an
97+
// EventCollector) must be declared *before* the runner so it outlives the
98+
// thread that may invoke those callbacks.
9699
class IoContextRunner {
97100
public:
98101
IoContextRunner() : work_guard_(boost::asio::make_work_guard(ioc_)) {
@@ -127,8 +130,8 @@ TEST(ClientTest, ConnectsToHttpServer) {
127130
// Give server a moment to start accepting connections
128131
std::this_thread::sleep_for(100ms);
129132

130-
IoContextRunner runner;
131133
EventCollector collector;
134+
IoContextRunner runner;
132135

133136
auto client =
134137
Builder(runner.context().get_executor(),
@@ -153,8 +156,8 @@ TEST(ClientTest, HandlesMultipleEvents) {
153156
auto port = server.start(
154157
TestHandlers::multiple_events({"event1", "event2", "event3"}));
155158

156-
IoContextRunner runner;
157159
EventCollector collector;
160+
IoContextRunner runner;
158161

159162
auto client =
160163
Builder(runner.context().get_executor(),
@@ -192,8 +195,8 @@ TEST(ClientTest, ParsesEventWithType) {
192195
close();
193196
});
194197

195-
IoContextRunner runner;
196198
EventCollector collector;
199+
IoContextRunner runner;
197200

198201
auto client =
199202
Builder(runner.context().get_executor(),
@@ -228,8 +231,8 @@ TEST(ClientTest, ParsesEventWithId) {
228231
close();
229232
});
230233

231-
IoContextRunner runner;
232234
EventCollector collector;
235+
IoContextRunner runner;
233236

234237
auto client =
235238
Builder(runner.context().get_executor(),
@@ -264,8 +267,8 @@ TEST(ClientTest, ParsesMultiLineData) {
264267
close();
265268
});
266269

267-
IoContextRunner runner;
268270
EventCollector collector;
271+
IoContextRunner runner;
269272

270273
auto client =
271274
Builder(runner.context().get_executor(),
@@ -305,8 +308,8 @@ TEST(ClientTest, HandlesComments) {
305308
close();
306309
});
307310

308-
IoContextRunner runner;
309311
EventCollector collector;
312+
IoContextRunner runner;
310313

311314
auto client =
312315
Builder(runner.context().get_executor(),
@@ -347,8 +350,8 @@ TEST(ClientTest, SupportsPostMethod) {
347350
close();
348351
});
349352

350-
IoContextRunner runner;
351353
EventCollector collector;
354+
IoContextRunner runner;
352355

353356
auto client =
354357
Builder(runner.context().get_executor(),
@@ -386,8 +389,8 @@ TEST(ClientTest, SupportsReportMethod) {
386389
close();
387390
});
388391

389-
IoContextRunner runner;
390392
EventCollector collector;
393+
IoContextRunner runner;
391394

392395
auto client =
393396
Builder(runner.context().get_executor(),
@@ -430,8 +433,8 @@ TEST(ClientTest, SendsCustomHeaders) {
430433
close();
431434
});
432435

433-
IoContextRunner runner;
434436
EventCollector collector;
437+
IoContextRunner runner;
435438

436439
auto client =
437440
Builder(runner.context().get_executor(),
@@ -456,8 +459,8 @@ TEST(ClientTest, Handles404Error) {
456459
MockSSEServer server;
457460
auto port = server.start(TestHandlers::http_error(http::status::not_found));
458461

459-
IoContextRunner runner;
460462
EventCollector collector;
463+
IoContextRunner runner;
461464

462465
auto client =
463466
Builder(runner.context().get_executor(),
@@ -495,8 +498,8 @@ TEST(ClientTest, Handles500Error) {
495498
MockSSEServer server;
496499
auto port = server.start(handler);
497500

498-
IoContextRunner runner;
499501
EventCollector collector;
502+
IoContextRunner runner;
500503

501504
auto client =
502505
Builder(runner.context().get_executor(),
@@ -535,8 +538,8 @@ TEST(ClientTest, FollowsRedirects) {
535538
auto redirect_port = redirect_server.start(TestHandlers::redirect(
536539
"http://localhost:" + std::to_string(target_port) + "/"));
537540

538-
IoContextRunner runner;
539541
EventCollector collector;
542+
IoContextRunner runner;
540543

541544
auto client =
542545
Builder(runner.context().get_executor(),
@@ -574,8 +577,8 @@ TEST(ClientTest, ShutdownStopsClient) {
574577
}
575578
});
576579

577-
IoContextRunner runner;
578580
EventCollector collector;
581+
IoContextRunner runner;
579582

580583
auto client =
581584
Builder(runner.context().get_executor(),
@@ -603,8 +606,8 @@ TEST(ClientTest, CanShutdownBeforeConnection) {
603606
MockSSEServer server;
604607
auto port = server.start(TestHandlers::simple_event("test"));
605608

606-
IoContextRunner runner;
607609
EventCollector collector;
610+
IoContextRunner runner;
608611

609612
auto client =
610613
Builder(runner.context().get_executor(),
@@ -632,8 +635,8 @@ TEST(ClientTest, HandlesImmediateClose) {
632635
MockSSEServer server;
633636
auto port = server.start(handler);
634637

635-
IoContextRunner runner;
636638
EventCollector collector;
639+
IoContextRunner runner;
637640

638641
auto client =
639642
Builder(runner.context().get_executor(),
@@ -679,8 +682,8 @@ TEST(ClientTest, RespectsReadTimeout) {
679682
std::this_thread::sleep_for(5000ms);
680683
});
681684

682-
IoContextRunner runner;
683685
EventCollector collector;
686+
IoContextRunner runner;
684687

685688
auto client =
686689
Builder(runner.context().get_executor(),
@@ -759,8 +762,8 @@ TEST(ClientTest, HandlesEmptyEventData) {
759762
close();
760763
});
761764

762-
IoContextRunner runner;
763765
EventCollector collector;
766+
IoContextRunner runner;
764767

765768
auto client =
766769
Builder(runner.context().get_executor(),
@@ -795,8 +798,8 @@ TEST(ClientTest, HandlesEventWithOnlyType) {
795798
close();
796799
});
797800

798-
IoContextRunner runner;
799801
EventCollector collector;
802+
IoContextRunner runner;
800803

801804
auto client =
802805
Builder(runner.context().get_executor(),
@@ -837,8 +840,8 @@ TEST(ClientTest, HandlesRapidEvents) {
837840
close();
838841
});
839842

840-
IoContextRunner runner;
841843
EventCollector collector;
844+
IoContextRunner runner;
842845

843846
auto client =
844847
Builder(runner.context().get_executor(),
@@ -874,8 +877,8 @@ TEST(ClientTest, ShutdownDuringBackoffDelay) {
874877
MockSSEServer server;
875878
auto port = server.start(handler);
876879

877-
IoContextRunner runner;
878880
EventCollector collector;
881+
IoContextRunner runner;
879882

880883
auto client =
881884
Builder(runner.context().get_executor(),
@@ -987,8 +990,8 @@ TEST(ClientTest, ShutdownDuringProgressCallback) {
987990
MockSSEServer server;
988991
auto port = server.start(handler);
989992

990-
IoContextRunner runner;
991993
EventCollector collector;
994+
IoContextRunner runner;
992995

993996
auto client =
994997
Builder(runner.context().get_executor(),
@@ -1020,8 +1023,8 @@ TEST(ClientTest, MultipleShutdownCalls) {
10201023
MockSSEServer server;
10211024
auto port = server.start(TestHandlers::simple_event("test"));
10221025

1023-
IoContextRunner runner;
10241026
EventCollector collector;
1027+
IoContextRunner runner;
10251028

10261029
auto client =
10271030
Builder(runner.context().get_executor(),
@@ -1062,8 +1065,8 @@ TEST(ClientTest, ShutdownAfterConnectionClosed) {
10621065
close(); // Server closes connection
10631066
});
10641067

1065-
IoContextRunner runner;
10661068
EventCollector collector;
1069+
IoContextRunner runner;
10671070

10681071
auto client =
10691072
Builder(runner.context().get_executor(),
@@ -1109,8 +1112,8 @@ TEST(ClientTest, ShutdownDuringConnectionAttempt) {
11091112
MockSSEServer server;
11101113
auto port = server.start(handler);
11111114

1112-
IoContextRunner runner;
11131115
EventCollector collector;
1116+
IoContextRunner runner;
11141117

11151118
auto client =
11161119
Builder(runner.context().get_executor(),
@@ -1144,8 +1147,8 @@ TEST(ClientTest, OnConnectHookInvokedBeforeRequest) {
11441147
MockSSEServer server;
11451148
auto port = server.start(TestHandlers::simple_event("hello"));
11461149

1147-
IoContextRunner runner;
11481150
EventCollector collector;
1151+
IoContextRunner runner;
11491152

11501153
std::atomic<int> hook_calls{0};
11511154
std::string seen_target;
@@ -1198,8 +1201,8 @@ TEST(ClientTest, OnConnectHookCanMutateTarget) {
11981201
close();
11991202
});
12001203

1201-
IoContextRunner runner;
12021204
EventCollector collector;
1205+
IoContextRunner runner;
12031206

12041207
auto client =
12051208
Builder(runner.context().get_executor(),
@@ -1246,8 +1249,8 @@ TEST(ClientTest, OnConnectHookCanMutateHeaders) {
12461249
close();
12471250
});
12481251

1249-
IoContextRunner runner;
12501252
EventCollector collector;
1253+
IoContextRunner runner;
12511254

12521255
auto client =
12531256
Builder(runner.context().get_executor(),
@@ -1286,8 +1289,8 @@ TEST(ClientTest, OnConnectHookInvokedOnEachReconnect) {
12861289
close();
12871290
});
12881291

1289-
IoContextRunner runner;
12901292
EventCollector collector;
1293+
IoContextRunner runner;
12911294

12921295
std::atomic<int> hook_calls{0};
12931296

@@ -1334,8 +1337,8 @@ TEST(ClientTest, OnConnectHookSeesPreviousMutations) {
13341337
close();
13351338
});
13361339

1337-
IoContextRunner runner;
13381340
EventCollector collector;
1341+
IoContextRunner runner;
13391342

13401343
auto client =
13411344
Builder(runner.context().get_executor(),
@@ -1396,8 +1399,8 @@ TEST(ClientTest, OnConnectHookCanMutateBody) {
13961399
close();
13971400
});
13981401

1399-
IoContextRunner runner;
14001402
EventCollector collector;
1403+
IoContextRunner runner;
14011404

14021405
std::string const build_time_body = "short";
14031406
std::string const hook_body =
@@ -1462,8 +1465,8 @@ TEST(ClientTest, OnConnectHookLastEventIdIsManagedByClient) {
14621465
close();
14631466
});
14641467

1465-
IoContextRunner runner;
14661468
EventCollector collector;
1469+
IoContextRunner runner;
14671470

14681471
auto client =
14691472
Builder(runner.context().get_executor(),

0 commit comments

Comments
 (0)