Changeset 244352 in webkit

Timestamp:

Apr 16, 2019, 12:46:27 PM (7 years ago)

Author:

rmorisset@apple.com

Message:

[WTF] holdLock should be marked WARN_UNUSED_RETURN
​https://bugs.webkit.org/show_bug.cgi?id=196922

Reviewed by Keith Miller.

Source/JavaScriptCore:

There was one case where holdLock was used and the result ignored.
From a comment that was deleted in ​https://bugs.webkit.org/attachment.cgi?id=328438&action=prettypatch, I believe that it is on purpose.
So I brought back a variant of the comment, and made the ignoring of the return explicit.

• heap/BlockDirectory.cpp:

(JSC::BlockDirectory::isPagedOut):

Source/WTF:

• wtf/Locker.h:

Location:

trunk/Source

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/heap/BlockDirectory.cpp (modified) (1 diff)
• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/Locker.h (modified) (4 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-04-16  Robin Morisset  <rmorisset@apple.com>
+
+        [WTF] holdLock should be marked WARN_UNUSED_RETURN
+        https://bugs.webkit.org/show_bug.cgi?id=196922
+
+        Reviewed by Keith Miller.
+
+        There was one case where holdLock was used and the result ignored.
+        From a comment that was deleted in https://bugs.webkit.org/attachment.cgi?id=328438&action=prettypatch, I believe that it is on purpose.
+        So I brought back a variant of the comment, and made the ignoring of the return explicit.
+
+        * heap/BlockDirectory.cpp:
+        (JSC::BlockDirectory::isPagedOut):
+
 2019-04-16  Caitlin Potter  <caitp@igalia.com>
 

trunk/Source/JavaScriptCore/heap/BlockDirectory.cpp

@@ -62 +62 @@
     unsigned itersSinceLastTimeCheck = 0;
     for (auto* block : m_blocks) {
-        if (block)
-            holdLock(block->block().lock());
+        if (block) {
+            // We take and drop the lock as a way of touching that page of memory.
+            // FIXME: should we instead do a cheaper thing like a volatile load in the page?
+            (void) holdLock(block->block().lock());
+        }
         ++itersSinceLastTimeCheck;
         if (itersSinceLastTimeCheck >= Heap::s_timeCheckResolution) {

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2019-04-16  Robin Morisset  <rmorisset@apple.com>
+
+        [WTF] holdLock should be marked WARN_UNUSED_RETURN
+        https://bugs.webkit.org/show_bug.cgi?id=196922
+
+        Reviewed by Keith Miller.
+
+        * wtf/Locker.h:
+
 2019-04-16  Don Olmstead  <don.olmstead@sony.com>
 

trunk/Source/WTF/wtf/Locker.h

@@ -31 +31 @@
 #include <wtf/Assertions.h>
 #include <wtf/Atomics.h>
+#include <wtf/Compiler.h>
 #include <wtf/Noncopyable.h>
 
@@ -120 +121 @@
 // auto locker = holdLock(lock);
 template<typename LockType>
+Locker<LockType> holdLock(LockType&) WARN_UNUSED_RETURN;
+template<typename LockType>
 Locker<LockType> holdLock(LockType& lock)
 {
@@ -126 +129 @@
 
 template<typename LockType>
+Locker<LockType> holdLockIf(LockType&, bool predicate) WARN_UNUSED_RETURN;
+template<typename LockType>
 Locker<LockType> holdLockIf(LockType& lock, bool predicate)
 {
@@ -131 +136 @@
 }
 
+template<typename LockType>
+Locker<LockType> tryHoldLock(LockType&) WARN_UNUSED_RETURN;
 template<typename LockType>
 Locker<LockType> tryHoldLock(LockType& lock)