From e340688b94845cf9a37848b757c7b97aec549d94 Mon Sep 17 00:00:00 2001 From: sudden6 Date: Mon, 20 Jan 2020 23:19:08 +0100 Subject: [PATCH] fix(coreav): fix assert because c-toxcore calls from unexpected thread c-toxcore calls the groupCallCallback from it's main thread instead of the ToxAV thread as expected, this was triggering an assertion. Aditionally the destructors of Core and CoreAV were fixed, because they now either crashed or deadlocked qTox when it was closed while a group call was still running. (cherry picked from commit 141cbf887026835d83c3763f920daed3276b674e) --- src/core/core.cpp | 8 +++++--- src/core/coreav.cpp | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/core/core.cpp b/src/core/core.cpp index 4186cb2d5..78c26a8a3 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -159,12 +159,14 @@ Core::Core(QThread* coreThread) Core::~Core() { - // need to reset av first, because it uses tox - av.reset(); - + /* + * First stop the thread to stop the timer and avoid Core emitting callbacks + * into an already destructed CoreAV. + */ coreThread->exit(0); coreThread->wait(); + av.reset(); tox.reset(); } diff --git a/src/core/coreav.cpp b/src/core/coreav.cpp index 16a8e9614..ce17ae4a9 100644 --- a/src/core/coreav.cpp +++ b/src/core/coreav.cpp @@ -151,9 +151,16 @@ IAudioControl* CoreAV::getAudio() CoreAV::~CoreAV() { + /* Gracefully leave calls and group calls to avoid deadlocks in destructor */ for (const auto& call : calls) { cancelCall(call.first); } + for (const auto& call : groupCalls) { + leaveGroupCall(call.first); + } + + assert(calls.empty()); + assert(groupCalls.empty()); coreavThread->exit(0); coreavThread->wait(); @@ -452,12 +459,19 @@ void CoreAV::toggleMuteCallOutput(const Friend* f) void CoreAV::groupCallCallback(void* tox, uint32_t group, uint32_t peer, const int16_t* data, unsigned samples, uint8_t channels, uint32_t sample_rate, void* core) { + /* + * Currently group call audio decoding is handled in the Tox thread by c-toxcore, + * so we can be sure that this function is always called from the Core thread. + * To change this, an API change in c-toxcore is needed and this function probably must be + * changed. + * See https://github.com/TokTok/c-toxcore/issues/1364 for details. + */ + Q_UNUSED(tox); Core* c = static_cast(core); CoreAV* cav = c->getAv(); QReadLocker locker{&cav->callsLock}; - assert(QThread::currentThread() == cav->coreavThread.get()); const ToxPk peerPk = c->getGroupPeerPk(group, peer); const Settings& s = Settings::getInstance();