Skip to content

Commit 76b0c2c

Browse files
MirkoBonadeiWebRTC LUCI CQ
authored andcommitted
Revert "Reland "Port: migrate to TaskQueue.""
This reverts commit e2ab77b. See bugs, this CL seems to be the culprit of crashes in cricket::TurnPort::OnMessage and jingle_glue::JingleThreadWrapper::Dispatch. [email protected], [email protected] Bug: chromium:1227839, chromium:1228462 Change-Id: I7521210970fe543a01682bb08de31ac025e79981 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225880 Reviewed-by: Mirko Bonadei <[email protected]> Commit-Queue: Mirko Bonadei <[email protected]> Cr-Commit-Position: refs/heads/master@{#34462}
1 parent 3ead44a commit 76b0c2c

File tree

4 files changed

+27
-19
lines changed

4 files changed

+27
-19
lines changed

p2p/base/port.cc

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
#include "rtc_base/string_encode.h"
3333
#include "rtc_base/string_utils.h"
3434
#include "rtc_base/strings/string_builder.h"
35-
#include "rtc_base/task_utils/to_queued_task.h"
3635
#include "rtc_base/third_party/base64/base64.h"
3736
#include "rtc_base/trace_event.h"
3837
#include "system_wrappers/include/field_trial.h"
@@ -174,13 +173,15 @@ void Port::Construct() {
174173
network_->SignalTypeChanged.connect(this, &Port::OnNetworkTypeChanged);
175174
network_cost_ = network_->GetCost();
176175

177-
ScheduleDelayedDestructionIfDead();
176+
thread_->PostDelayed(RTC_FROM_HERE, timeout_delay_, this,
177+
MSG_DESTROY_IF_DEAD);
178178
RTC_LOG(LS_INFO) << ToString() << ": Port created with network cost "
179179
<< network_cost_;
180180
}
181181

182182
Port::~Port() {
183183
RTC_DCHECK_RUN_ON(thread_);
184+
CancelPendingTasks();
184185

185186
// Delete all of the remaining connections. We copy the list up front
186187
// because each deletion will cause it to be modified.
@@ -821,11 +822,19 @@ void Port::KeepAliveUntilPruned() {
821822

822823
void Port::Prune() {
823824
state_ = State::PRUNED;
824-
thread_->PostTask(webrtc::ToQueuedTask(safety_, [this] { DestroyIfDead(); }));
825+
thread_->Post(RTC_FROM_HERE, this, MSG_DESTROY_IF_DEAD);
825826
}
826827

827-
void Port::DestroyIfDead() {
828+
// Call to stop any currently pending operations from running.
829+
void Port::CancelPendingTasks() {
830+
TRACE_EVENT0("webrtc", "Port::CancelPendingTasks");
828831
RTC_DCHECK_RUN_ON(thread_);
832+
thread_->Clear(this);
833+
}
834+
835+
void Port::OnMessage(rtc::Message* pmsg) {
836+
RTC_DCHECK_RUN_ON(thread_);
837+
RTC_DCHECK(pmsg->message_id == MSG_DESTROY_IF_DEAD);
829838
bool dead =
830839
(state_ == State::INIT || state_ == State::PRUNED) &&
831840
connections_.empty() &&
@@ -849,12 +858,6 @@ void Port::OnNetworkTypeChanged(const rtc::Network* network) {
849858
UpdateNetworkCost();
850859
}
851860

852-
void Port::ScheduleDelayedDestructionIfDead() {
853-
thread_->PostDelayedTask(
854-
webrtc::ToQueuedTask(safety_, [this] { DestroyIfDead(); }),
855-
timeout_delay_);
856-
}
857-
858861
std::string Port::ToString() const {
859862
rtc::StringBuilder ss;
860863
ss << "Port[" << rtc::ToHex(reinterpret_cast<uintptr_t>(this)) << ":"
@@ -905,7 +908,8 @@ void Port::OnConnectionDestroyed(Connection* conn) {
905908
// not cause the Port to be destroyed.
906909
if (connections_.empty()) {
907910
last_time_all_connections_removed_ = rtc::TimeMillis();
908-
ScheduleDelayedDestructionIfDead();
911+
thread_->PostDelayed(RTC_FROM_HERE, timeout_delay_, this,
912+
MSG_DESTROY_IF_DEAD);
909913
}
910914
}
911915

p2p/base/port.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
#include "rtc_base/rate_tracker.h"
4242
#include "rtc_base/socket_address.h"
4343
#include "rtc_base/system/rtc_export.h"
44-
#include "rtc_base/task_utils/pending_task_safety_flag.h"
4544
#include "rtc_base/third_party/sigslot/sigslot.h"
4645
#include "rtc_base/thread.h"
4746
#include "rtc_base/weak_ptr.h"
@@ -172,6 +171,7 @@ typedef std::set<rtc::SocketAddress> ServerAddresses;
172171
// connections to similar mechanisms of the other client. Subclasses of this
173172
// one add support for specific mechanisms like local UDP ports.
174173
class Port : public PortInterface,
174+
public rtc::MessageHandler,
175175
public sigslot::has_slots<> {
176176
public:
177177
// INIT: The state when a port is just created.
@@ -220,6 +220,9 @@ class Port : public PortInterface,
220220
// Allows a port to be destroyed if no connection is using it.
221221
void Prune();
222222

223+
// Call to stop any currently pending operations from running.
224+
void CancelPendingTasks();
225+
223226
// The thread on which this port performs its I/O.
224227
rtc::Thread* thread() { return thread_; }
225228

@@ -325,6 +328,8 @@ class Port : public PortInterface,
325328
// Called if the port has no connections and is no longer useful.
326329
void Destroy();
327330

331+
void OnMessage(rtc::Message* pmsg) override;
332+
328333
// Debugging description of this port
329334
std::string ToString() const override;
330335
uint16_t min_port() { return min_port_; }
@@ -375,6 +380,8 @@ class Port : public PortInterface,
375380
const rtc::SocketAddress& base_address);
376381

377382
protected:
383+
enum { MSG_DESTROY_IF_DEAD = 0, MSG_FIRST_AVAILABLE };
384+
378385
virtual void UpdateNetworkCost();
379386

380387
void set_type(const std::string& type) { type_ = type; }
@@ -441,9 +448,8 @@ class Port : public PortInterface,
441448
void Construct();
442449
// Called when one of our connections deletes itself.
443450
void OnConnectionDestroyed(Connection* conn);
451+
444452
void OnNetworkTypeChanged(const rtc::Network* network);
445-
void ScheduleDelayedDestructionIfDead();
446-
void DestroyIfDead();
447453

448454
rtc::Thread* const thread_;
449455
rtc::PacketSocketFactory* const factory_;
@@ -493,7 +499,6 @@ class Port : public PortInterface,
493499

494500
friend class Connection;
495501
webrtc::CallbackList<PortInterface*> port_destroyed_callback_list_;
496-
webrtc::ScopedTaskSafety safety_;
497502
};
498503

499504
} // namespace cricket

p2p/base/turn_port.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -990,7 +990,7 @@ void TurnPort::OnMessage(rtc::Message* message) {
990990
Close();
991991
break;
992992
default:
993-
RTC_NOTREACHED();
993+
Port::OnMessage(message);
994994
}
995995
}
996996

p2p/base/turn_port.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
#include "p2p/client/basic_port_allocator.h"
2626
#include "rtc_base/async_packet_socket.h"
2727
#include "rtc_base/async_resolver_interface.h"
28-
#include "rtc_base/message_handler.h"
2928
#include "rtc_base/ssl_certificate.h"
3029
#include "rtc_base/task_utils/pending_task_safety_flag.h"
3130

@@ -42,7 +41,7 @@ extern const char TURN_PORT_TYPE[];
4241
class TurnAllocateRequest;
4342
class TurnEntry;
4443

45-
class TurnPort : public Port, public rtc::MessageHandler {
44+
class TurnPort : public Port {
4645
public:
4746
enum PortState {
4847
STATE_CONNECTING, // Initial state, cannot send any packets.
@@ -299,7 +298,7 @@ class TurnPort : public Port, public rtc::MessageHandler {
299298

300299
private:
301300
enum {
302-
MSG_ALLOCATE_ERROR,
301+
MSG_ALLOCATE_ERROR = MSG_FIRST_AVAILABLE,
303302
MSG_ALLOCATE_MISMATCH,
304303
MSG_TRY_ALTERNATE_SERVER,
305304
MSG_REFRESH_ERROR,

0 commit comments

Comments
 (0)