Revert "Fix an issue in DtlsIdentityStore when the store is destroyed before the worker thread task returns."
This reverts commit 45bc01a7172402aa4bb8d457474300533c273413.
TBR=pthatcher@webrtc.org
BUG=
Review URL: https://webrtc-codereview.appspot.com/47559004
Cr-Commit-Position: refs/heads/master@{#8711}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8711 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/dtlsidentitystore.cc b/talk/app/webrtc/dtlsidentitystore.cc
index 16ed798..7beb68d 100644
--- a/talk/app/webrtc/dtlsidentitystore.cc
+++ b/talk/app/webrtc/dtlsidentitystore.cc
@@ -44,48 +44,8 @@
};
typedef rtc::ScopedMessageData<rtc::SSLIdentity> IdentityResultMessageData;
-
} // namespace
-// This class runs on the worker thread to generate the identity. It's necessary
-// to separate this class from DtlsIdentityStore so that it can live on the
-// worker thread after DtlsIdentityStore is destroyed.
-class DtlsIdentityStore::WorkerTask : public sigslot::has_slots<>,
- public rtc::MessageHandler {
- public:
- explicit WorkerTask(DtlsIdentityStore* store) : store_(store) {
- store_->SignalDestroyed.connect(this, &WorkerTask::OnStoreDestroyed);
- };
-
- void GenerateIdentity() {
- rtc::scoped_ptr<rtc::SSLIdentity> identity(
- rtc::SSLIdentity::Generate(DtlsIdentityStore::kIdentityName));
-
- {
- rtc::CritScope cs(&cs_);
- if (store_) {
- store_->PostGenerateIdentityResult_w(identity.Pass());
- }
- }
- }
-
- void OnMessage(rtc::Message* msg) override {
- DCHECK(msg->message_id == MSG_GENERATE_IDENTITY);
- GenerateIdentity();
- // Deleting msg->pdata will destroy the WorkerTask.
- delete msg->pdata;
- }
-
- private:
- void OnStoreDestroyed() {
- rtc::CritScope cs(&cs_);
- store_ = NULL;
- }
-
- rtc::CriticalSection cs_;
- DtlsIdentityStore* store_;
-};
-
// Arbitrary constant used as common name for the identity.
// Chosen to make the certificates more readable.
const char DtlsIdentityStore::kIdentityName[] = "WebRTC";
@@ -96,16 +56,10 @@
worker_thread_(worker_thread),
pending_jobs_(0) {}
-DtlsIdentityStore::~DtlsIdentityStore() {
- SignalDestroyed();
-}
+DtlsIdentityStore::~DtlsIdentityStore() {}
void DtlsIdentityStore::Initialize() {
- // Do not aggressively generate the free identity if the worker thread and the
- // signaling thread are the same.
- if (worker_thread_ != signaling_thread_) {
- GenerateIdentity();
- }
+ GenerateIdentity();
}
void DtlsIdentityStore::RequestIdentity(DTLSIdentityRequestObserver* observer) {
@@ -125,6 +79,9 @@
void DtlsIdentityStore::OnMessage(rtc::Message* msg) {
switch (msg->message_id) {
+ case MSG_GENERATE_IDENTITY:
+ GenerateIdentity_w();
+ break;
case MSG_GENERATE_IDENTITY_RESULT: {
rtc::scoped_ptr<IdentityResultMessageData> pdata(
static_cast<IdentityResultMessageData*>(msg->pdata));
@@ -148,12 +105,7 @@
pending_jobs_++;
LOG(LS_VERBOSE) << "New DTLS identity generation is posted, "
<< "pending_identities=" << pending_jobs_;
-
- WorkerTask* task = new WorkerTask(this);
- // The WorkerTask is owned by the message data to make sure it will not be
- // leaked even if the task does not get run.
- IdentityTaskMessageData* msg = new IdentityTaskMessageData(task);
- worker_thread_->Post(task, MSG_GENERATE_IDENTITY, msg);
+ worker_thread_->Post(this, MSG_GENERATE_IDENTITY, NULL);
}
void DtlsIdentityStore::OnIdentityGenerated(
@@ -191,22 +143,19 @@
LOG(LS_WARNING) << "Failed to generate SSL identity";
}
- // Do not aggressively generate the free identity if the worker thread and the
- // signaling thread are the same.
- if (worker_thread_ != signaling_thread_ &&
- pending_observers_.empty() &&
- pending_jobs_ == 0) {
- // Generate a free identity in the background.
- GenerateIdentity();
+ if (pending_observers_.empty() && pending_jobs_ == 0) {
+ // Generate a free identity in the background.
+ GenerateIdentity();
}
}
-void DtlsIdentityStore::PostGenerateIdentityResult_w(
- rtc::scoped_ptr<rtc::SSLIdentity> identity) {
+void DtlsIdentityStore::GenerateIdentity_w() {
DCHECK(rtc::Thread::Current() == worker_thread_);
- IdentityResultMessageData* msg =
- new IdentityResultMessageData(identity.release());
+ rtc::SSLIdentity* identity = rtc::SSLIdentity::Generate(kIdentityName);
+
+ IdentityResultMessageData* msg = new IdentityResultMessageData(identity);
signaling_thread_->Post(this, MSG_GENERATE_IDENTITY_RESULT, msg);
}
+
} // namespace webrtc
diff --git a/talk/app/webrtc/dtlsidentitystore.h b/talk/app/webrtc/dtlsidentitystore.h
index ffe8067..2d52249 100644
--- a/talk/app/webrtc/dtlsidentitystore.h
+++ b/talk/app/webrtc/dtlsidentitystore.h
@@ -33,7 +33,6 @@
#include "talk/app/webrtc/peerconnectioninterface.h"
#include "webrtc/base/messagehandler.h"
-#include "webrtc/base/messagequeue.h"
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/base/scoped_ref_ptr.h"
@@ -52,7 +51,7 @@
DtlsIdentityStore(rtc::Thread* signaling_thread,
rtc::Thread* worker_thread);
- virtual ~DtlsIdentityStore();
+ ~DtlsIdentityStore();
// Initialize will start generating the free identity in the background.
void Initialize();
@@ -68,16 +67,11 @@
bool HasFreeIdentityForTesting() const;
private:
- sigslot::signal0<sigslot::multi_threaded_local> SignalDestroyed;
- class WorkerTask;
- typedef rtc::ScopedMessageData<DtlsIdentityStore::WorkerTask>
- IdentityTaskMessageData;
-
void GenerateIdentity();
void OnIdentityGenerated(rtc::scoped_ptr<rtc::SSLIdentity> identity);
void ReturnIdentity(rtc::scoped_ptr<rtc::SSLIdentity> identity);
- void PostGenerateIdentityResult_w(rtc::scoped_ptr<rtc::SSLIdentity> identity);
+ void GenerateIdentity_w();
rtc::Thread* signaling_thread_;
rtc::Thread* worker_thread_;
diff --git a/talk/app/webrtc/dtlsidentitystore_unittest.cc b/talk/app/webrtc/dtlsidentitystore_unittest.cc
index 262e773..67b7cb2 100644
--- a/talk/app/webrtc/dtlsidentitystore_unittest.cc
+++ b/talk/app/webrtc/dtlsidentitystore_unittest.cc
@@ -80,12 +80,10 @@
class DtlsIdentityStoreTest : public testing::Test {
protected:
DtlsIdentityStoreTest()
- : worker_thread_(new rtc::Thread()),
- store_(new DtlsIdentityStore(rtc::Thread::Current(),
- worker_thread_.get())),
+ : store_(new DtlsIdentityStore(rtc::Thread::Current(),
+ rtc::Thread::Current())),
observer_(
new rtc::RefCountedObject<MockDtlsIdentityRequestObserver>()) {
- CHECK(worker_thread_->Start());
store_->Initialize();
}
~DtlsIdentityStoreTest() {}
@@ -97,7 +95,6 @@
rtc::CleanupSSL();
}
- rtc::scoped_ptr<rtc::Thread> worker_thread_;
rtc::scoped_ptr<DtlsIdentityStore> store_;
rtc::scoped_refptr<MockDtlsIdentityRequestObserver> observer_;
};
@@ -109,21 +106,4 @@
EXPECT_TRUE_WAIT(observer_->LastRequestSucceeded(), kTimeoutMs);
EXPECT_TRUE_WAIT(store_->HasFreeIdentityForTesting(), kTimeoutMs);
-
- observer_->Reset();
-
- // Verifies that the callback is async when a free identity is ready.
- store_->RequestIdentity(observer_.get());
- EXPECT_FALSE(observer_->call_back_called());
- EXPECT_TRUE_WAIT(observer_->LastRequestSucceeded(), kTimeoutMs);
-}
-
-TEST_F(DtlsIdentityStoreTest, DeleteStoreEarlyNoCrash) {
- EXPECT_FALSE(store_->HasFreeIdentityForTesting());
-
- store_->RequestIdentity(observer_.get());
- store_.reset();
-
- worker_thread_->Stop();
- EXPECT_FALSE(observer_->call_back_called());
}
diff --git a/talk/app/webrtc/peerconnectionfactory.cc b/talk/app/webrtc/peerconnectionfactory.cc
index 1c93376..9f1e858 100644
--- a/talk/app/webrtc/peerconnectionfactory.cc
+++ b/talk/app/webrtc/peerconnectionfactory.cc
@@ -132,11 +132,6 @@
DCHECK(signaling_thread_->IsCurrent());
channel_manager_.reset(NULL);
default_allocator_factory_ = NULL;
-
- // Make sure |worker_thread_| and |signaling_thread_| outlive
- // |dtls_identity_store_|.
- dtls_identity_store_.reset(NULL);
-
if (owns_ptrs_) {
if (wraps_current_thread_)
rtc::ThreadManager::Instance()->UnwrapCurrentThread();
