Make Android/iOS local/remote description accessors thread safe.
Since the descriptions can be modified on the signaling thread,
ToString can only be safely called on that thread.
Bug: webrtc:11791
Change-Id: Icf6aada8aa66d00be94c6bda7b22e41b5d3bbc17
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/180541
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Anders Carlsson <andersc@webrtc.org>
Commit-Queue: Taylor <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31862}
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index b1beb84..09317b8 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -909,6 +909,10 @@
const std::string& label,
const DataChannelInit* config) = 0;
+ // NOTE: For the following 6 methods, it's only safe to dereference the
+ // SessionDescriptionInterface on signaling_thread() (for example, calling
+ // ToString).
+
// Returns the more recently applied description; "pending" if it exists, and
// otherwise "current". See below.
virtual const SessionDescriptionInterface* local_description() const = 0;
@@ -1136,6 +1140,14 @@
// thus the observer object can be safely destroyed.
virtual void Close() = 0;
+ // The thread on which all PeerConnectionObserver callbacks will be invoked,
+ // as well as callbacks for other classes such as DataChannelObserver.
+ //
+ // Also the only thread on which it's safe to use SessionDescriptionInterface
+ // pointers.
+ // TODO(deadbeef): Make pure virtual when all subclasses implement it.
+ virtual rtc::Thread* signaling_thread() const { return nullptr; }
+
protected:
// Dtor protected as objects shouldn't be deleted via this interface.
~PeerConnectionInterface() override = default;
diff --git a/api/peer_connection_proxy.h b/api/peer_connection_proxy.h
index aa72d7c..0cc3b3b 100644
--- a/api/peer_connection_proxy.h
+++ b/api/peer_connection_proxy.h
@@ -147,6 +147,7 @@
PROXY_METHOD1(bool, StartRtcEventLog, std::unique_ptr<RtcEventLogOutput>)
PROXY_METHOD0(void, StopRtcEventLog)
PROXY_METHOD0(void, Close)
+BYPASS_PROXY_CONSTMETHOD0(rtc::Thread*, signaling_thread)
END_PROXY_MAP()
} // namespace webrtc
diff --git a/api/proxy.h b/api/proxy.h
index b1ebe31..0e5d622 100644
--- a/api/proxy.h
+++ b/api/proxy.h
@@ -400,11 +400,13 @@
// For use when returning purely const state (set during construction).
// Use with caution. This method should only be used when the return value will
// always be the same.
-#define BYPASS_PROXY_CONSTMETHOD0(r, method) \
- r method() const override { \
- static_assert(!std::is_pointer<r>::value, "Type is a pointer"); \
- static_assert(!std::is_reference<r>::value, "Type is a reference"); \
- return c_->method(); \
+#define BYPASS_PROXY_CONSTMETHOD0(r, method) \
+ r method() const override { \
+ static_assert( \
+ std::is_same<r, rtc::Thread*>::value || !std::is_pointer<r>::value, \
+ "Type is a pointer"); \
+ static_assert(!std::is_reference<r>::value, "Type is a reference"); \
+ return c_->method(); \
}
} // namespace webrtc
diff --git a/api/test/dummy_peer_connection.h b/api/test/dummy_peer_connection.h
index 97a97d0..0ca7d3f 100644
--- a/api/test/dummy_peer_connection.h
+++ b/api/test/dummy_peer_connection.h
@@ -237,7 +237,11 @@
void StopRtcEventLog() { FATAL() << "Not implemented"; }
- void Close() {}
+ void Close() override {}
+
+ rtc::Thread* signaling_thread() const override {
+ return rtc::Thread::Current();
+ }
};
static_assert(