Skip to content

Commit 028ea3b

Browse files
mr comments, update cpp collection example
1 parent 36a2e31 commit 028ea3b

13 files changed

Lines changed: 184 additions & 124 deletions

include/livekit/room.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ class LIVEKIT_API Room {
152152
/// The returned handle is non-owning. Call @c lock() to obtain a usable
153153
/// @c weak_ptr; the result is empty (`lock() == nullptr`) before connect,
154154
/// after room end-of-stream teardown, or once the room is destroyed. This
155-
/// lets callers that cache the handle detect teardown instead of holding a
155+
/// lets callers that cache the handle detect object lifetime instead of holding a
156156
/// dangling pointer.
157157
///
158158
/// @return Weak handle to the local participant.
@@ -287,14 +287,17 @@ class LIVEKIT_API Room {
287287
RoomDelegate* delegate_ = nullptr; // Not owned
288288
RoomInfoData room_info_;
289289
std::shared_ptr<FfiHandle> room_handle_;
290+
/// The local participant is owned by the room and is not shared with other objects.
291+
/// It is a shared_ptr just to utilize the weak_ptr interface for the localParticipant() accessor.
290292
std::shared_ptr<LocalParticipant> local_participant_;
291293
std::unordered_map<std::string, std::shared_ptr<RemoteParticipant>> remote_participants_;
292294
// Data stream
293295
std::unordered_map<std::string, TextStreamHandler> text_stream_handlers_;
294296
std::unordered_map<std::string, ByteStreamHandler> byte_stream_handlers_;
295297
std::unordered_map<std::string, std::shared_ptr<TextStreamReader>> text_stream_readers_;
296298
std::unordered_map<std::string, std::shared_ptr<ByteStreamReader>> byte_stream_readers_;
297-
// E2EE
299+
// The E2EE manager is owned by the room and is not shared with other objects.
300+
// It is a shared_ptr just to utilize the weak_ptr interface for the e2eeManager() accessor.
298301
std::shared_ptr<E2EEManager> e2ee_manager_;
299302
std::shared_ptr<SubscriptionThreadDispatcher> subscription_thread_dispatcher_;
300303

src/tests/common/test_common.h

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,49 @@ inline bool waitForParticipant(Room* room, const std::string& identity, std::chr
138138
return false;
139139
}
140140

141+
/// Safely promote the local participant weak handle to a shared_ptr.
142+
///
143+
/// Room::localParticipant() returns a std::weak_ptr whose lock() yields nullptr
144+
/// once the room is torn down (or before connect). Dereferencing the result of
145+
/// lock() blindly is undefined behavior, so tests must go through this helper,
146+
/// which throws instead of crashing when the handle is expired.
147+
inline std::shared_ptr<LocalParticipant> lockLocalParticipant(const Room& room) {
148+
auto participant = room.localParticipant().lock();
149+
if (!participant) {
150+
throw std::runtime_error("Local participant handle is expired");
151+
}
152+
return participant;
153+
}
154+
155+
/// Pointer overload of lockLocalParticipant(); throws if @p room is null.
156+
inline std::shared_ptr<LocalParticipant> lockLocalParticipant(const Room* room) {
157+
if (room == nullptr) {
158+
throw std::runtime_error("Room is null");
159+
}
160+
return lockLocalParticipant(*room);
161+
}
162+
163+
/// Safely promote a remote participant weak handle to a shared_ptr.
164+
///
165+
/// Mirrors lockLocalParticipant(): Room::remoteParticipant() returns a
166+
/// std::weak_ptr that lock()s to nullptr once the participant disconnects, so
167+
/// this helper throws rather than letting callers dereference a null pointer.
168+
inline std::shared_ptr<RemoteParticipant> lockRemoteParticipant(const Room& room, const std::string& identity) {
169+
auto participant = room.remoteParticipant(identity).lock();
170+
if (!participant) {
171+
throw std::runtime_error("Remote participant '" + identity + "' handle is expired");
172+
}
173+
return participant;
174+
}
175+
176+
/// Pointer overload of lockRemoteParticipant(); throws if @p room is null.
177+
inline std::shared_ptr<RemoteParticipant> lockRemoteParticipant(const Room* room, const std::string& identity) {
178+
if (room == nullptr) {
179+
throw std::runtime_error("Room is null");
180+
}
181+
return lockRemoteParticipant(*room, identity);
182+
}
183+
141184
inline std::array<std::string, 2> getDataTrackTestTokens() {
142185
const char* token_a = std::getenv("LIVEKIT_TOKEN_A");
143186
if (token_a == nullptr || std::string(token_a).empty()) {
@@ -164,7 +207,7 @@ inline void waitForParticipantVisibility(const std::vector<std::unique_ptr<Room>
164207
if (!room || room->localParticipant().expired()) {
165208
throw std::runtime_error("Test room is missing a local participant after connect");
166209
}
167-
participant_identities.push_back(room->localParticipant().lock()->identity());
210+
participant_identities.push_back(lockLocalParticipant(room.get())->identity());
168211
}
169212

170213
auto start = std::chrono::steady_clock::now();

src/tests/integration/test_data_track.cpp

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,13 @@ std::string describeDataTrackError(const Error& error) {
113113
return "code=" + std::to_string(static_cast<std::uint32_t>(error.code)) + " message=" + error.message;
114114
}
115115

116-
std::shared_ptr<LocalDataTrack> requirePublishedTrack(LocalParticipant* participant, const std::string& name) {
117-
auto result = participant->publishDataTrack(name);
116+
std::shared_ptr<LocalDataTrack> requirePublishedTrack(const std::weak_ptr<LocalParticipant>& participant,
117+
const std::string& name) {
118+
auto locked = participant.lock();
119+
if (!locked) {
120+
throw std::runtime_error("Local participant handle is expired");
121+
}
122+
auto result = locked->publishDataTrack(name);
118123
if (!result) {
119124
throw std::runtime_error("Failed to publish data track: " + describeDataTrackError(result.error()));
120125
}
@@ -216,7 +221,7 @@ void runEncryptedDataTrackRoundTrip(KeyDerivationFunction key_derivation_functio
216221
EXPECT_EQ(publisher_room->e2eeManager().lock()->keyProvider()->exportSharedKey(), e2eeSharedKey());
217222
EXPECT_EQ(subscriber_room->e2eeManager().lock()->keyProvider()->exportSharedKey(), e2eeSharedKey());
218223

219-
auto publish_result = publisher_room->localParticipant().lock()->publishDataTrack(track_name);
224+
auto publish_result = lockLocalParticipant(*publisher_room)->publishDataTrack(track_name);
220225
if (!publish_result) {
221226
FAIL() << describeDataTrackError(publish_result.error());
222227
}
@@ -307,9 +312,9 @@ TEST_P(DataTrackTransportTest, PublishesAndReceivesFramesEndToEnd) {
307312

308313
auto rooms = testRooms(room_configs);
309314
auto& publisher_room = rooms[0];
310-
const auto publisher_identity = publisher_room->localParticipant().lock()->identity();
315+
const auto publisher_identity = lockLocalParticipant(*publisher_room)->identity();
311316

312-
auto local_track = requirePublishedTrack(publisher_room->localParticipant().lock().get(), track_name);
317+
auto local_track = requirePublishedTrack(publisher_room->localParticipant(), track_name);
313318
ASSERT_TRUE(local_track->isPublished());
314319
EXPECT_FALSE(local_track->info().uses_e2ee);
315320
EXPECT_EQ(local_track->info().name, track_name);
@@ -372,7 +377,7 @@ TEST_F(DataTrackE2ETest, UnpublishUpdatesPublishedStateEndToEnd) {
372377
auto rooms = testRooms(room_configs);
373378
auto& publisher_room = rooms[0];
374379

375-
auto publish_result = publisher_room->localParticipant().lock()->publishDataTrack(track_name);
380+
auto publish_result = lockLocalParticipant(*publisher_room)->publishDataTrack(track_name);
376381
if (!publish_result) {
377382
FAIL() << describeDataTrackError(publish_result.error());
378383
}
@@ -401,7 +406,7 @@ TEST_F(DataTrackE2ETest, SubscribeAfterUnpublishReportsTerminalError) {
401406
auto rooms = testRooms(room_configs);
402407
auto& publisher_room = rooms[0];
403408

404-
auto local_track = requirePublishedTrack(publisher_room->localParticipant().lock().get(), track_name);
409+
auto local_track = requirePublishedTrack(publisher_room->localParticipant(), track_name);
405410
ASSERT_TRUE(local_track->isPublished());
406411

407412
auto remote_track = subscriber_delegate.waitForTrack(kTrackWaitTimeout);
@@ -455,7 +460,7 @@ TEST_F(DataTrackE2ETest, PublishManyTracks) {
455460
const auto start = std::chrono::steady_clock::now();
456461
for (int index = 0; index < kPublishManyTrackCount; ++index) {
457462
const auto track_name = "track_" + std::to_string(index);
458-
auto publish_result = room->localParticipant().lock()->publishDataTrack(track_name);
463+
auto publish_result = lockLocalParticipant(*room)->publishDataTrack(track_name);
459464
if (!publish_result) {
460465
FAIL() << "Failed to publish track " << track_name << ": " << describeDataTrackError(publish_result.error());
461466
}
@@ -496,14 +501,14 @@ TEST_F(DataTrackE2ETest, PublishDuplicateName) {
496501
auto rooms = testRooms(1);
497502
auto& room = rooms[0];
498503

499-
auto first_track_result = room->localParticipant().lock()->publishDataTrack("first");
504+
auto first_track_result = lockLocalParticipant(*room)->publishDataTrack("first");
500505
if (!first_track_result) {
501506
FAIL() << describeDataTrackError(first_track_result.error());
502507
}
503508
auto first_track = first_track_result.value();
504509
ASSERT_TRUE(first_track->isPublished());
505510

506-
auto duplicate_result = room->localParticipant().lock()->publishDataTrack("first");
511+
auto duplicate_result = lockLocalParticipant(*room)->publishDataTrack("first");
507512
ASSERT_FALSE(duplicate_result) << "Expected duplicate data-track name to be rejected";
508513
EXPECT_EQ(duplicate_result.error().code, PublishDataTrackErrorCode::DUPLICATE_NAME);
509514
EXPECT_FALSE(duplicate_result.error().message.empty());
@@ -525,7 +530,7 @@ TEST_F(DataTrackE2ETest, CanResubscribeToRemoteDataTrack) {
525530
std::exception_ptr publish_error;
526531
std::thread publisher([&]() {
527532
try {
528-
auto track = requirePublishedTrack(publisher_room->localParticipant().lock().get(), track_name);
533+
auto track = requirePublishedTrack(publisher_room->localParticipant(), track_name);
529534
if (!track->isPublished()) {
530535
throw std::runtime_error("Publisher failed to publish data track");
531536
}
@@ -582,7 +587,7 @@ TEST_F(DataTrackE2ETest, FfiClientSubscribeDataTrackReturnsSyncResult) {
582587

583588
for (std::size_t idx = 0; idx < kTopicCount; ++idx) {
584589
const auto track_name = "test_" + std::to_string(idx);
585-
auto publish_result = publisher_room->localParticipant().lock()->publishDataTrack(track_name);
590+
auto publish_result = lockLocalParticipant(*publisher_room)->publishDataTrack(track_name);
586591
if (!publish_result) {
587592
FAIL() << "Failed to publish " << track_name << ": " << describeDataTrackError(publish_result.error());
588593
}
@@ -644,7 +649,7 @@ TEST_F(DataTrackE2ETest, PreservesUserTimestampEndToEnd) {
644649
auto rooms = testRooms(room_configs);
645650
auto& publisher_room = rooms[0];
646651

647-
auto publish_result = publisher_room->localParticipant().lock()->publishDataTrack(track_name);
652+
auto publish_result = lockLocalParticipant(*publisher_room)->publishDataTrack(track_name);
648653
if (!publish_result) {
649654
FAIL() << describeDataTrackError(publish_result.error());
650655
}
@@ -735,7 +740,7 @@ TEST_F(DataTrackE2ETest, PreservesUserTimestampOnEncryptedDataTrack) {
735740
publisher_room->e2eeManager().lock()->setEnabled(true);
736741
subscriber_room->e2eeManager().lock()->setEnabled(true);
737742

738-
auto publish_result = publisher_room->localParticipant().lock()->publishDataTrack(track_name);
743+
auto publish_result = lockLocalParticipant(*publisher_room)->publishDataTrack(track_name);
739744
if (!publish_result) {
740745
FAIL() << describeDataTrackError(publish_result.error());
741746
}

src/tests/integration/test_late_join_track_publication.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -366,10 +366,10 @@ TEST_P(LateJoinTrackPublicationIntegrationTest, ConsumerReceivesAlreadyPublished
366366
ASSERT_TRUE(publisher_room.connect(config_.url, config_.token_a, options)) << "Publisher failed to connect";
367367
ASSERT_FALSE(publisher_room.localParticipant().expired());
368368

369-
const std::string publisher_identity = publisher_room.localParticipant().lock()->identity();
369+
const std::string publisher_identity = lockLocalParticipant(publisher_room)->identity();
370370
ASSERT_FALSE(publisher_identity.empty());
371371

372-
PublishedTrackGuard published_tracks(publisher_room.localParticipant().lock().get());
372+
PublishedTrackGuard published_tracks(lockLocalParticipant(publisher_room).get());
373373
MediaLoopGuard media_loops;
374374
std::vector<ExpectedPublication> expected_media;
375375

@@ -380,7 +380,7 @@ TEST_P(LateJoinTrackPublicationIntegrationTest, ConsumerReceivesAlreadyPublished
380380
TrackPublishOptions publish_options;
381381
publish_options.source = TrackSource::SOURCE_MICROPHONE;
382382

383-
ASSERT_NO_THROW(publisher_room.localParticipant().lock()->publishTrack(track, publish_options));
383+
ASSERT_NO_THROW(lockLocalParticipant(publisher_room)->publishTrack(track, publish_options));
384384
ASSERT_NE(track->publication(), nullptr) << "Audio track was not locally published";
385385

386386
published_tracks.addMediaTrack(track, track->publication()->sid());
@@ -470,10 +470,10 @@ TEST_P(LateJoinTrackPublicationIntegrationTest, ConsumerReceivesAlreadyPublished
470470
ASSERT_TRUE(publisher_room.connect(config_.url, config_.token_a, options)) << "Publisher failed to connect";
471471
ASSERT_FALSE(publisher_room.localParticipant().expired());
472472

473-
const std::string publisher_identity = publisher_room.localParticipant().lock()->identity();
473+
const std::string publisher_identity = lockLocalParticipant(publisher_room)->identity();
474474
ASSERT_FALSE(publisher_identity.empty());
475475

476-
PublishedTrackGuard published_tracks(publisher_room.localParticipant().lock().get());
476+
PublishedTrackGuard published_tracks(lockLocalParticipant(publisher_room).get());
477477
MediaLoopGuard media_loops;
478478
std::vector<ExpectedPublication> expected_media;
479479

@@ -485,7 +485,7 @@ TEST_P(LateJoinTrackPublicationIntegrationTest, ConsumerReceivesAlreadyPublished
485485
publish_options.source = TrackSource::SOURCE_CAMERA;
486486
publish_options.simulcast = false;
487487

488-
ASSERT_NO_THROW(publisher_room.localParticipant().lock()->publishTrack(track, publish_options));
488+
ASSERT_NO_THROW(lockLocalParticipant(publisher_room)->publishTrack(track, publish_options));
489489
ASSERT_NE(track->publication(), nullptr) << "Video track was not locally published";
490490

491491
published_tracks.addMediaTrack(track, track->publication()->sid());
@@ -575,15 +575,15 @@ TEST_P(LateJoinTrackPublicationIntegrationTest, ConsumerReceivesAlreadyPublished
575575
ASSERT_TRUE(publisher_room.connect(config_.url, config_.token_a, options)) << "Publisher failed to connect";
576576
ASSERT_FALSE(publisher_room.localParticipant().expired());
577577

578-
const std::string publisher_identity = publisher_room.localParticipant().lock()->identity();
578+
const std::string publisher_identity = lockLocalParticipant(publisher_room)->identity();
579579
ASSERT_FALSE(publisher_identity.empty());
580580

581-
PublishedTrackGuard published_tracks(publisher_room.localParticipant().lock().get());
581+
PublishedTrackGuard published_tracks(lockLocalParticipant(publisher_room).get());
582582
std::set<std::string> expected_data;
583583

584584
for (int i = 0; i < kDataTrackCount; ++i) {
585585
const std::string track_name = makeTrackName("late-join-data", i);
586-
auto publish_result = publisher_room.localParticipant().lock()->publishDataTrack(track_name);
586+
auto publish_result = lockLocalParticipant(publisher_room)->publishDataTrack(track_name);
587587
ASSERT_TRUE(publish_result) << "Failed to publish data track " << track_name << ": "
588588
<< publish_result.error().message;
589589

src/tests/integration/test_media_multistream.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,7 @@ void MediaMultiStreamIntegrationTest::runPublishTwoVideoAndTwoAudioTracks(bool s
101101
auto sender_room = std::make_unique<Room>();
102102
ASSERT_TRUE(sender_room->connect(config_.url, config_.token_a, options)) << "Sender failed to connect";
103103

104-
const std::string receiver_identity = receiver_room->localParticipant().lock()->identity();
105-
const std::string sender_identity = sender_room->localParticipant().lock()->identity();
104+
const std::string sender_identity = lockLocalParticipant(*sender_room)->identity();
106105

107106
constexpr int kVideoTrackCount = 10;
108107
constexpr int kAudioTrackCount = 10;
@@ -126,7 +125,7 @@ void MediaMultiStreamIntegrationTest::runPublishTwoVideoAndTwoAudioTracks(bool s
126125
auto track = LocalVideoTrack::createLocalVideoTrack(name, source);
127126
TrackPublishOptions opts;
128127
opts.source = (i % 2 == 0) ? TrackSource::SOURCE_CAMERA : TrackSource::SOURCE_SCREENSHARE;
129-
sender_room->localParticipant().lock()->publishTrack(track, opts);
128+
lockLocalParticipant(*sender_room)->publishTrack(track, opts);
130129
std::cerr << "[MediaMultiStream] published video " << name << " sid=" << track->sid() << std::endl;
131130
video_sources.push_back(source);
132131
video_tracks.push_back(track);
@@ -139,7 +138,7 @@ void MediaMultiStreamIntegrationTest::runPublishTwoVideoAndTwoAudioTracks(bool s
139138
auto track = LocalAudioTrack::createLocalAudioTrack(name, source);
140139
TrackPublishOptions opts;
141140
opts.source = (i % 2 == 0) ? TrackSource::SOURCE_MICROPHONE : TrackSource::SOURCE_SCREENSHARE_AUDIO;
142-
sender_room->localParticipant().lock()->publishTrack(track, opts);
141+
lockLocalParticipant(*sender_room)->publishTrack(track, opts);
143142
std::cerr << "[MediaMultiStream] published audio " << name << " sid=" << track->sid() << std::endl;
144143
audio_sources.push_back(source);
145144
audio_tracks.push_back(track);
@@ -210,12 +209,12 @@ void MediaMultiStreamIntegrationTest::runPublishTwoVideoAndTwoAudioTracks(bool s
210209

211210
for (const auto& track : video_tracks) {
212211
if (track->publication()) {
213-
sender_room->localParticipant().lock()->unpublishTrack(track->publication()->sid());
212+
lockLocalParticipant(*sender_room)->unpublishTrack(track->publication()->sid());
214213
}
215214
}
216215
for (const auto& track : audio_tracks) {
217216
if (track->publication()) {
218-
sender_room->localParticipant().lock()->unpublishTrack(track->publication()->sid());
217+
lockLocalParticipant(*sender_room)->unpublishTrack(track->publication()->sid());
219218
}
220219
}
221220
}

src/tests/integration/test_room.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ TEST_F(RoomLifecycleTest, ParticipantHandlesExpireOnRoomDestruction) {
9999
ASSERT_TRUE(peer->connect(config_.url, config_.token_b, options)) << "Peer failed to connect";
100100

101101
ASSERT_FALSE(peer->localParticipant().expired());
102-
const std::string peer_identity = peer->localParticipant().lock()->identity();
102+
const std::string peer_identity = lockLocalParticipant(*peer)->identity();
103103
ASSERT_TRUE(waitForParticipant(room.get(), peer_identity, 10s)) << "Peer not visible to room";
104104

105105
// 2. Store the local participant handle. Keep the weak_ptr itself - locking

src/tests/integration/test_room_listener_cleanup.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ void expectFailedConnectDoesNotDuplicateParticipantCallbacks(const TestConfig& c
9191
Room peer_room;
9292
ASSERT_TRUE(peer_room.connect(config.url, config.token_b, options)) << "Peer failed to connect";
9393
ASSERT_FALSE(peer_room.localParticipant().expired());
94-
const std::string peer_identity = peer_room.localParticipant().lock()->identity();
94+
const std::string peer_identity = lockLocalParticipant(peer_room)->identity();
9595
ASSERT_FALSE(peer_identity.empty());
9696

9797
expectSingleParticipantConnectedCallback(counter, peer_identity);
@@ -137,7 +137,7 @@ TEST_F(RoomListenerCleanupIntegrationTest, AlreadyConnectedConnectDoesNotReplace
137137
Room peer_room;
138138
ASSERT_TRUE(peer_room.connect(config_.url, config_.token_b, options)) << "Peer failed to connect";
139139
ASSERT_FALSE(peer_room.localParticipant().expired());
140-
const std::string peer_identity = peer_room.localParticipant().lock()->identity();
140+
const std::string peer_identity = lockLocalParticipant(peer_room)->identity();
141141
ASSERT_FALSE(peer_identity.empty());
142142

143143
expectSingleParticipantConnectedCallback(counter, peer_identity);

0 commit comments

Comments
 (0)