[Webkit-unassigned] [Bug 147107] Adding default implementation for JavaScript watchdog.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jul 22 09:29:11 PDT 2015
https://bugs.webkit.org/show_bug.cgi?id=147107
Mark Lam <mark.lam at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #257262|review? |review-
Flags| |
--- Comment #6 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 257262
--> https://bugs.webkit.org/attachment.cgi?id=257262
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=257262&action=review
> Source/JavaScriptCore/runtime/WatchdogGeneric.cpp:36
> + unique_lock<std::mutex> lock(m_mutex);
Let's use a "std::lock_guard<std::mutex> lock(m_mutex);" instead since it also communicates that the mutex will not be unlocked until this scope exits. The unique_lock<std::mutex> implies that you may unlock it mid scope (which you don't).
> Source/JavaScriptCore/runtime/WatchdogGeneric.cpp:47
> + m_running = false;
> + m_active = false;
> + m_cond.notify_all();
> + m_mutex.unlock();
It is wrong to force unlock the mutex here, especially since you have not locked it. The right thing to do is to put these 3 lines above this in a scope, and add a "std::lock_guard<std::mutex> lock(m_mutex);" at the top of the scope.
> Source/JavaScriptCore/runtime/WatchdogGeneric.cpp:71
> + m_timeout = chrono::steady_clock::now() + limit;
> + m_active = true;
> + m_cond.notify_all();
You're should add a "std::lock_guard<std::mutex> lock(m_mutex);" at the top of this scope. Otherwise, you'll have a race condition with Watchdog::run() due to spurious wakes from m_cond.wait().
> Source/JavaScriptCore/runtime/WatchdogGeneric.cpp:77
> + m_active = false;
> + m_cond.notify_all();
You're should add a "std::lock_guard<std::mutex> lock(m_mutex);" at the top of this scope. You might be able to get away without locking in this case here, but let's be pedantic about it so that it is clear that m_active and m_cond are under the guard of m_mutex.
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150722/625a99af/attachment-0001.html>
More information about the webkit-unassigned
mailing list