Merge pull request #4617: FIX(client): Crash due to race-condition in TalkingUI

The race condition was that the channel of a user could be modified
and before it gets updated in the TalkingUI as well, the talking state
of that user changes. Then inside the talkingStateChanged function the
code would assume that the TalkingUIUser is in the container representing
the current channel of that user but in fact in the TalkingUI this user
is still in its own channel, causing the user-search to return nullptr
which then yields a SegFault as soon as the returned pointer gets
dereferenced.
This commit is contained in:
Robert Adam 2020-12-08 12:46:33 +01:00 committed by GitHub
commit 687e6982b3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 17 additions and 14 deletions

View File

@ -360,7 +360,7 @@ void TalkingUI::addChannel(const Channel *channel) {
}
}
void TalkingUI::addUser(const ClientUser *user) {
TalkingUIUser *TalkingUI::findOrAddUser(const ClientUser *user) {
// In a first step, it has to be made sure that the user's channel
// exists in this UI.
addChannel(user->cChannel);
@ -395,11 +395,12 @@ void TalkingUI::addUser(const ClientUser *user) {
std::unique_ptr< TalkingUIContainer > &channelContainer =
m_containers[findContainer(user->cChannel->iId, ContainerType::CHANNEL)];
if (!channelContainer) {
qCritical("TalkingUI::addUser requesting unknown channel!");
return;
qCritical("TalkingUI::findOrAddUser requesting unknown channel!");
return nullptr;
}
std::unique_ptr< TalkingUIUser > userEntry = std::make_unique< TalkingUIUser >(*user);
TalkingUIUser *newUserEntry = userEntry.get();
// * 1000 as the setting is in seconds whereas the timer expects milliseconds
userEntry->setLifeTime(g.s.iTalkingUI_SilentUserLifeTime * 1000);
@ -420,6 +421,10 @@ void TalkingUI::addUser(const ClientUser *user) {
channelContainer->addEntry(std::move(userEntry));
sortContainers();
return newUserEntry;
} else {
return oldUserEntry;
}
}
@ -602,15 +607,11 @@ void TalkingUI::on_talkingStateChanged() {
return;
}
addUser(user);
TalkingUIUser *userEntry = findOrAddUser(user);
// addUser puts the user in its current channel, so we can fetch that and know that it'll contain the user
std::unique_ptr< TalkingUIContainer > &channel =
m_containers[findContainer(user->cChannel->iId, ContainerType::CHANNEL)];
TalkingUIUser *userEntry = static_cast< TalkingUIUser * >(channel->get(user->uiSession, EntryType::USER));
userEntry->setTalkingState(user->tsState);
if (userEntry) {
userEntry->setTalkingState(user->tsState);
}
updateUI();
}
@ -673,7 +674,7 @@ void TalkingUI::on_serverSynchronized() {
// can't count on it to change its talking state right after it has connected to
// a server, we have to add it manually.
ClientUser *self = ClientUser::get(g.uiSession);
addUser(self);
findOrAddUser(self);
}
}
@ -763,7 +764,7 @@ void TalkingUI::on_settingsChanged() {
} else {
if (self && g.s.bTalkingUI_LocalUserStaysVisible) {
// Add the local user as it is requested to be displayed
addUser(self);
findOrAddUser(self);
}
}

View File

@ -73,7 +73,9 @@ private:
/// Adds an UI entry for the given User, if none exists yet.
///
/// @param channel A pointer to the user that shall be added
void addUser(const ClientUser *user);
/// @returns The pointer to the respective user entry in the TalkingUI
/// (may be nullptr in case of an error)
TalkingUIUser *findOrAddUser(const ClientUser *user);
/// Moves the given user into the given channel
///
/// @paam userSession The session ID of the user