PeerConnection::StartRtcEventLog: Improve callback memory safety

By having a unique_ptr own the callback data instead of a raw pointer,
the compiler helps us ensure that it's destroyed exactly once,
and never used after being destroyed.

(This made the callback object move-only, so I had to add support
for move-only callbacks to rtc::Thread::Invoke().)

BUG=webrtc:8111

Change-Id: Ia0804e4662e63e91e5cee18ecc3f38d2cfe8a26b
Reviewed-on: https://webrtc-review.googlesource.com/10812
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20317}
This commit is contained in:
Karl Wiberg 2017-10-16 23:01:06 +02:00 committed by Commit Bot
parent b06b358207
commit d6b4819e4a
3 changed files with 13 additions and 12 deletions

View File

@ -1317,15 +1317,14 @@ bool PeerConnection::StartRtcEventLog(rtc::PlatformFile file,
bool PeerConnection::StartRtcEventLog(
std::unique_ptr<RtcEventLogOutput> output) {
// This code assumes that the functor would be executed. Otherwise, this
// could leak.
// TODO(eladalon): When we switch to C++14 (or later), we can get rid of this
// conversion to a raw pointer, passing the unique_ptr directly to the lambda.
RtcEventLogOutput* output_raw = output.release();
auto functor = [this, output_raw]() {
return StartRtcEventLog_w(rtc::WrapUnique<RtcEventLogOutput>(output_raw));
// TODO(eladalon): In C++14, this can be done with a lambda.
struct Functor {
bool operator()() { return pc->StartRtcEventLog_w(std::move(output)); }
PeerConnection* const pc;
std::unique_ptr<RtcEventLogOutput> output;
};
return worker_thread()->Invoke<bool>(RTC_FROM_HERE, functor);
return worker_thread()->Invoke<bool>(RTC_FROM_HERE,
Functor{this, std::move(output)});
}
void PeerConnection::StopRtcEventLog() {

View File

@ -38,8 +38,8 @@ class MessageHandler {
template <class ReturnT, class FunctorT>
class FunctorMessageHandler : public MessageHandler {
public:
explicit FunctorMessageHandler(const FunctorT& functor)
: functor_(functor) {}
explicit FunctorMessageHandler(FunctorT&& functor)
: functor_(std::forward<FunctorT>(functor)) {}
virtual void OnMessage(Message* msg) {
result_ = functor_();
}

View File

@ -15,6 +15,7 @@
#include <list>
#include <memory>
#include <string>
#include <utility>
#include <vector>
#if defined(WEBRTC_POSIX)
@ -175,8 +176,9 @@ class RTC_LOCKABLE Thread : public MessageQueue {
// NOTE: This function can only be called when synchronous calls are allowed.
// See ScopedDisallowBlockingCalls for details.
template <class ReturnT, class FunctorT>
ReturnT Invoke(const Location& posted_from, const FunctorT& functor) {
FunctorMessageHandler<ReturnT, FunctorT> handler(functor);
ReturnT Invoke(const Location& posted_from, FunctorT&& functor) {
FunctorMessageHandler<ReturnT, FunctorT> handler(
std::forward<FunctorT>(functor));
InvokeInternal(posted_from, &handler);
return handler.MoveResult();
}