Skip to content
This repository has been archived by the owner on Feb 12, 2023. It is now read-only.

Commit

Permalink
fix(coreav): fix assert because c-toxcore calls from unexpected thread
Browse files Browse the repository at this point in the history
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 141cbf8)
  • Loading branch information
sudden6 authored and anthonybilinski committed Mar 20, 2020
1 parent 723a8e5 commit e340688
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
8 changes: 5 additions & 3 deletions src/core/core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
16 changes: 15 additions & 1 deletion src/core/coreav.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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*>(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();
Expand Down

0 comments on commit e340688

Please sign in to comment.