FIX(client): Racecondition deleting ChannelListeners

As ChannelListeners are only synchronised with the server once
msgServerSync has been received, there is still a racecondition for them
even though we only save them if the ServerSync has happened yet.
The connection could be terminated before the answer of the server for
the request to add the respective listeners reaches the client. In that
case the listeners would be erased by the call to
ChannelListener::saveToDB().

Thus this commit introduces yet another flag indicating whether the
listeners have synchronised with the server yet.
This commit is contained in:
Robert Adam 2020-08-19 16:33:21 +02:00
parent f2acfb50f4
commit 3a55fb4a66
4 changed files with 46 additions and 1 deletions

View File

@ -32,6 +32,7 @@ ChannelListener::ChannelListener()
#ifdef MUMBLE
, m_volumeLock()
, m_listenerVolumeAdjustments()
, m_initialSyncDone(false)
#endif
{}
@ -125,6 +126,10 @@ QHash<int, float> ChannelListener::getAllListenerLocalVolumeAdjustmentsImpl(bool
return volumeMap;
}
}
void ChannelListener::setInitialServerSyncDoneImpl(bool done) {
m_initialSyncDone.store(done);
}
#endif
void ChannelListener::clearImpl() {
@ -241,12 +246,23 @@ QHash<int, float> ChannelListener::getAllListenerLocalVolumeAdjustments(bool fil
return get().getAllListenerLocalVolumeAdjustmentsImpl(filter);
}
void ChannelListener::setInitialServerSyncDone(bool done) {
get().setInitialServerSyncDoneImpl(done);
}
void ChannelListener::saveToDB() {
if (!g.sh || g.sh->qbaDigest.isEmpty() || g.uiSession == 0) {
// Can't save as we don't have enough context
return;
}
if (!get().m_initialSyncDone.load()) {
// If we were to save the listeners before the sync is done, we'd overwrite the list of listeners in the
// DB with an empty set, effectively erasing all set-up listeners.
qWarning("ChannelListener: Aborting save of user's listeners as initial ServerSync is not done yet");
return;
}
// Save the currently listened channels
g.db->setChannelListeners(g.sh->qbaDigest, ChannelListener::getListenedChannelsForUser(g.uiSession));
// And also the currently set volume adjustments (if they're not set to 1.0)

View File

@ -11,6 +11,8 @@
#include <QtCore/QSet>
#include <QtCore/QReadWriteLock>
#include <atomic>
class User;
class Channel;
@ -35,6 +37,9 @@ class ChannelListener : public QObject {
/// A map between channel IDs and local volume adjustments to be made for ChannelListeners
/// in that channel
QHash<int, float> m_listenerVolumeAdjustments;
/// A flag indicating whether the initial synchronization with the server has finished yet
std::atomic<bool> m_initialSyncDone;
#endif
/// The static ChannelListener instance returned by ChannelListener::get()
@ -99,6 +104,9 @@ class ChannelListener : public QObject {
/// @param filter Whether to filter out adjustments of 1 (which have no effect)
/// @returns A map between channel IDs and the currently set volume adjustment
QHash<int, float> getAllListenerLocalVolumeAdjustmentsImpl(bool filter = false) const;
/// @done Whether the initial synchronization with the server is done yet
void setInitialServerSyncDoneImpl(bool done);
#endif
/// Clears all ChannelListeners and volume adjustments
@ -227,6 +235,9 @@ class ChannelListener : public QObject {
/// @returns A map between channel IDs and the currently set volume adjustment
static QHash<int, float> getAllListenerLocalVolumeAdjustments(bool filter = false);
/// @done Whether the initial synchronization with the server is done yet
static void setInitialServerSyncDone(bool done);
/// Saves the current ChannelListener state to the database.
/// NOTE: This function may only be called from the main thread!
static void saveToDB();

View File

@ -834,6 +834,9 @@ void MainWindow::updateImagePath(QString filepath) const {
}
static void recreateServerHandler() {
// New server connection, so the sync has not happened yet
ChannelListener::setInitialServerSyncDone(false);
ServerHandlerPtr sh = g.sh;
if (sh && sh->isRunning()) {
g.mw->on_qaServerDisconnect_triggered();

View File

@ -189,7 +189,15 @@ void MainWindow::msgServerSync(const MumbleProto::ServerSync &msg) {
// Use the timer to execute the code in the main event loop as we have to access
// the database.
QTimer::singleShot(0, []() {
g.sh->startListeningToChannels(g.db->getChannelListeners(g.sh->qbaDigest));
QList<int> localListeners = g.db->getChannelListeners(g.sh->qbaDigest);
if (!localListeners.isEmpty()) {
ChannelListener::setInitialServerSyncDone(false);
g.sh->startListeningToChannels(localListeners);
} else {
// If there are no listeners, then no synchronization is needed in the first place
ChannelListener::setInitialServerSyncDone(true);
}
QHash<int, float> volumeMap = g.db->getChannelListenerLocalVolumeAdjustments(g.sh->qbaDigest);
@ -455,6 +463,13 @@ void MainWindow::msgUserState(const MumbleProto::UserState &msg) {
QString logMsg;
if (pDst == pSelf) {
logMsg = tr("You started listening to %1").arg(Log::formatChannel(c));
// Since ChannelListeners are sent out in bulks (all in a single message), the fact that we received
// a message that contains information about a ChannelListener of the local user means that we have
// succecssfully told the server that we are listening to the respective channels. Even if this message
// here has nothing to do with the actual initial synchronization, this means that we have been connected
// to the server long enough for the synchronization to be done.
ChannelListener::setInitialServerSyncDone(true);
} else if (pSelf && pSelf->cChannel == c) {
logMsg = tr("%1 started listening to your channel").arg(Log::formatClientUser(pDst, Log::Target));
}