Skip to content

Commit

Permalink
Prevent that OnIceServerTupleRemoved() listener invokes RemoveTuple()…
Browse files Browse the repository at this point in the history
… again
  • Loading branch information
ibc committed May 8, 2024
1 parent de48032 commit 9b5738b
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 10 deletions.
2 changes: 1 addition & 1 deletion worker/include/RTC/IceServer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ namespace RTC
RTC::TransportTuple* selectedTuple{ nullptr };
TimerHandle* consentCheckTimer{ nullptr };
uint64_t lastConsentRequestReceivedAtMs{ 0u };
bool clearingAllTuples{ false };
bool isRemovingTuples{ false };
};
} // namespace RTC

Expand Down
23 changes: 14 additions & 9 deletions worker/src/RTC/IceServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ namespace RTC
}

// Clear all tuples.
this->clearingAllTuples = true;
this->isRemovingTuples = true;

for (const auto& it : this->tuples)
{
Expand All @@ -133,13 +133,13 @@ namespace RTC
this->listener->OnIceServerTupleRemoved(this, storedTuple);
}

this->isRemovingTuples = false;

// Clear all tuples.
// NOTE: Do it after notifying the listener since the listener may need to
// use/read the tuple being removed so we cannot free it yet.
this->tuples.clear();

this->clearingAllTuples = false;

// Unset selected tuple.
this->selectedTuple = nullptr;

Expand Down Expand Up @@ -227,9 +227,10 @@ namespace RTC
{
MS_TRACE();

// While IceServer is being destroyed, it may call listener methods that may
// end calling RemoveTuple(). We must ignore it to avoid double-free issues.
if (this->clearingAllTuples)
// If IceServer is removing a tuple or all tuples (for instance in the
// destructor), the OnIceServerTupleRemoved() callback may end triggering
// new calls to RemoveTuple(). We must ignore it to avoid double-free issues.
if (this->isRemovingTuples)
{
return;
}
Expand Down Expand Up @@ -258,7 +259,9 @@ namespace RTC
}

// Notify the listener.
this->isRemovingTuples = true;
this->listener->OnIceServerTupleRemoved(this, removedTuple);
this->isRemovingTuples = false;

// Remove it from the list of tuples.
// NOTE: Do it after notifying the listener since the listener may need to
Expand Down Expand Up @@ -812,7 +815,9 @@ namespace RTC
MS_ASSERT(removedTuple, "couldn't find any tuple to be removed");

// Notify the listener.
this->isRemovingTuples = true;
this->listener->OnIceServerTupleRemoved(this, removedTuple);
this->isRemovingTuples = false;

// Remove it from the list of tuples.
// NOTE: Do it after notifying the listener since the listener may need to
Expand Down Expand Up @@ -929,7 +934,7 @@ namespace RTC
this->remoteNomination = 0u;

// Clear all tuples.
this->clearingAllTuples = true;
this->isRemovingTuples = true;

for (const auto& it : this->tuples)
{
Expand All @@ -939,13 +944,13 @@ namespace RTC
this->listener->OnIceServerTupleRemoved(this, storedTuple);
}

this->isRemovingTuples = false;

// Clear all tuples.
// NOTE: Do it after notifying the listener since the listener may need to
// use/read the tuple being removed so we cannot free it yet.
this->tuples.clear();

this->clearingAllTuples = false;

// Unset selected tuple.
this->selectedTuple = nullptr;

Expand Down

0 comments on commit 9b5738b

Please sign in to comment.