From 037ff2f12bd10e526781ca4a1e8b0e453939f2db Mon Sep 17 00:00:00 2001 From: Dave Hylands Date: Wed, 19 Sep 2012 23:34:07 -0700 Subject: [PATCH] Bug 792682 - GonkHal vibrator code incorrectly uses a weak reference. r=qDot --- hal/gonk/GonkHal.cpp | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/hal/gonk/GonkHal.cpp b/hal/gonk/GonkHal.cpp index 9b259a30484c..4c775012685c 100644 --- a/hal/gonk/GonkHal.cpp +++ b/hal/gonk/GonkHal.cpp @@ -102,7 +102,6 @@ public: VibratorRunnable() : mMonitor("VibratorRunnable") , mIndex(0) - , mShuttingDown(false) { nsCOMPtr os = services::GetObserverService(); if (!os) { @@ -110,8 +109,9 @@ public: return; } - os->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, /* weak ref */ true); + os->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false); } + NS_DECL_ISUPPORTS NS_DECL_NSIRUNNABLE NS_DECL_NSIOBSERVER @@ -120,6 +120,8 @@ public: void Vibrate(const nsTArray &pattern); void CancelVibrate(); + static bool ShuttingDown() { return sShuttingDown; } + private: Monitor mMonitor; @@ -132,11 +134,15 @@ private: // Set to true in our shutdown observer. When this is true, we kill the // vibrator thread. - bool mShuttingDown; + static bool sShuttingDown; }; NS_IMPL_THREADSAFE_ISUPPORTS2(VibratorRunnable, nsIRunnable, nsIObserver); +bool VibratorRunnable::sShuttingDown = false; + +static nsRefPtr sVibratorRunnable; + NS_IMETHODIMP VibratorRunnable::Run() { @@ -151,7 +157,7 @@ VibratorRunnable::Run() // condvar onto another thread. Better just to be chill about small errors in // the timing here. - while (!mShuttingDown) { + while (!sShuttingDown) { if (mIndex < mPattern.Length()) { uint32_t duration = mPattern[mIndex]; if (mIndex % 2 == 0) { @@ -164,7 +170,7 @@ VibratorRunnable::Run() mMonitor.Wait(); } } - + sVibratorRunnable = NULL; return NS_OK; } @@ -174,8 +180,9 @@ VibratorRunnable::Observe(nsISupports *subject, const char *topic, { MOZ_ASSERT(strcmp(topic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0); MonitorAutoLock lock(mMonitor); - mShuttingDown = true; + sShuttingDown = true; mMonitor.Notify(); + return NS_OK; } @@ -198,8 +205,6 @@ VibratorRunnable::CancelVibrate() mMonitor.Notify(); } -VibratorRunnable *sVibratorRunnable = NULL; - void EnsureVibratorThreadInitialized() { @@ -207,8 +212,7 @@ EnsureVibratorThreadInitialized() return; } - nsRefPtr runnable = new VibratorRunnable(); - sVibratorRunnable = runnable; + sVibratorRunnable = new VibratorRunnable(); nsCOMPtr thread; NS_NewThread(getter_AddRefs(thread), sVibratorRunnable); } @@ -218,6 +222,10 @@ EnsureVibratorThreadInitialized() void Vibrate(const nsTArray &pattern, const hal::WindowIdentifier &) { + MOZ_ASSERT(NS_IsMainThread()); + if (VibratorRunnable::ShuttingDown()) { + return; + } EnsureVibratorThreadInitialized(); sVibratorRunnable->Vibrate(pattern); } @@ -225,6 +233,10 @@ Vibrate(const nsTArray &pattern, const hal::WindowIdentifier &) void CancelVibrate(const hal::WindowIdentifier &) { + MOZ_ASSERT(NS_IsMainThread()); + if (VibratorRunnable::ShuttingDown()) { + return; + } EnsureVibratorThreadInitialized(); sVibratorRunnable->CancelVibrate(); }