Assert that thread-safe reference counting is used with
cross-thread NewRunnableMethod.
This assertion caught such an error in VisitedLinkMaster!
My approach, modify RunnableMethodTraits<T> to assert that
when ReleaseCallee happens on a different thread from
RetainCallee that the type supports thread-safe reference
counting. I do this by adding a static method to both
RefCounted<T> and RefCountedThreadSafe<T>.
This results in a little ugliness in cases where people
implement AddRef and Release by hand (to make the no-ops).
There may be a nicer way to deal with those few cases.
R=brettw
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/251012
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@27379 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 13f383ff5fc8ff095501794d4ce758f0067ff9b5
diff --git a/base/message_pump_glib_unittest.cc b/base/message_pump_glib_unittest.cc
index e0ad260..e3a04e8 100644
--- a/base/message_pump_glib_unittest.cc
+++ b/base/message_pump_glib_unittest.cc
@@ -185,8 +185,8 @@
// This lets us call NewRunnableMethod on EventInjector instances.
template<>
struct RunnableMethodTraits<EventInjector> {
- static void RetainCallee(EventInjector* obj) { }
- static void ReleaseCallee(EventInjector* obj) { }
+ void RetainCallee(EventInjector* obj) { }
+ void ReleaseCallee(EventInjector* obj) { }
};
TEST_F(MessagePumpGLibTest, TestQuit) {
diff --git a/base/ref_counted.h b/base/ref_counted.h
index 097d16e..70536b9 100644
--- a/base/ref_counted.h
+++ b/base/ref_counted.h
@@ -14,6 +14,8 @@
class RefCountedBase {
public:
+ static bool ImplementsThreadSafeReferenceCounting() { return false; }
+
bool HasOneRef() const { return ref_count_ == 1; }
protected:
@@ -38,6 +40,8 @@
class RefCountedThreadSafeBase {
public:
+ static bool ImplementsThreadSafeReferenceCounting() { return true; }
+
bool HasOneRef() const;
protected:
diff --git a/base/task.h b/base/task.h
index c827681..e9da8b3 100644
--- a/base/task.h
+++ b/base/task.h
@@ -205,12 +205,33 @@
template <class T>
struct RunnableMethodTraits {
- static void RetainCallee(T* obj) {
+ RunnableMethodTraits() {
+#ifndef NDEBUG
+ origin_thread_id_ = PlatformThread::CurrentId();
+#endif
+ }
+
+ ~RunnableMethodTraits() {
+#ifndef NDEBUG
+ // If destroyed on a separate thread, then we had better have been using
+ // thread-safe reference counting!
+ if (origin_thread_id_ != PlatformThread::CurrentId())
+ DCHECK(T::ImplementsThreadSafeReferenceCounting());
+#endif
+ }
+
+ void RetainCallee(T* obj) {
obj->AddRef();
}
- static void ReleaseCallee(T* obj) {
+
+ void ReleaseCallee(T* obj) {
obj->Release();
}
+
+ private:
+#ifndef NDEBUG
+ PlatformThreadId origin_thread_id_;
+#endif
};
// RunnableMethod and RunnableFunction -----------------------------------------
@@ -240,13 +261,13 @@
// RunnableMethod and NewRunnableMethod implementation -------------------------
template <class T, class Method, class Params>
-class RunnableMethod : public CancelableTask,
- public RunnableMethodTraits<T> {
+class RunnableMethod : public CancelableTask {
public:
RunnableMethod(T* obj, Method meth, const Params& params)
: obj_(obj), meth_(meth), params_(params) {
- RetainCallee(obj_);
+ traits_.RetainCallee(obj_);
}
+
~RunnableMethod() {
ReleaseCallee();
}
@@ -263,7 +284,7 @@
private:
void ReleaseCallee() {
if (obj_) {
- RunnableMethodTraits<T>::ReleaseCallee(obj_);
+ traits_.ReleaseCallee(obj_);
obj_ = NULL;
}
}
@@ -271,6 +292,7 @@
T* obj_;
Method meth_;
Params params_;
+ RunnableMethodTraits<T> traits_;
};
template <class T, class Method>
diff --git a/ipc/ipc_logging.cc b/ipc/ipc_logging.cc
index 9d4f4bb..e1684d7 100644
--- a/ipc/ipc_logging.cc
+++ b/ipc/ipc_logging.cc
@@ -36,8 +36,8 @@
// special retention program.
template <>
struct RunnableMethodTraits<IPC::Logging> {
- static void RetainCallee(IPC::Logging*) {}
- static void ReleaseCallee(IPC::Logging*) {}
+ void RetainCallee(IPC::Logging*) {}
+ void ReleaseCallee(IPC::Logging*) {}
};
namespace IPC {
diff --git a/ipc/ipc_sync_channel_unittest.cc b/ipc/ipc_sync_channel_unittest.cc
index f20f788..f6bf10e 100644
--- a/ipc/ipc_sync_channel_unittest.cc
+++ b/ipc/ipc_sync_channel_unittest.cc
@@ -66,6 +66,7 @@
}
void AddRef() { }
void Release() { }
+ static bool ImplementsThreadSafeReferenceCounting() { return true; }
bool Send(Message* msg) { return channel_->Send(msg); }
bool SendWithTimeout(Message* msg, int timeout_ms) {
return channel_->SendWithTimeout(msg, timeout_ms);