[shared-struct] Fix unlocking in JSAtomicsMutex

Errors in the callback were not correctly unlocking the mutex, oops.

Bug: v8:12547
Change-Id: If44ebc023b8192605c9f29bfd4099a197110f5c4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3760986
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81708}

src/builtins/builtins-atomics-synchronization.cc

@@ -44,13 +44,11 @@ BUILTIN(AtomicsMutexLock) {
 
   Handle<Object> result;
   {
-    // TODO(syg): Make base::LockGuard work with Handles.
-    JSAtomicsMutex::Lock(isolate, js_mutex);
+    JSAtomicsMutex::LockGuard lock_guard(isolate, js_mutex);
     ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
         isolate, result,
         Execution::Call(isolate, run_under_lock,
                         isolate->factory()->undefined_value(), 0, nullptr));
-    js_mutex->Unlock(isolate);
   }
 
   return *result;
@@ -75,13 +73,13 @@ BUILTIN(AtomicsMutexTryLock) {
                                    NewTypeError(MessageTemplate::kNotCallable));
   }
 
-  if (js_mutex->TryLock()) {
+  JSAtomicsMutex::TryLockGuard try_lock_guard(isolate, js_mutex);
+  if (try_lock_guard.locked()) {
     Handle<Object> result;
     ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
         isolate, result,
         Execution::Call(isolate, run_under_lock,
                         isolate->factory()->undefined_value(), 0, nullptr));
-    js_mutex->Unlock(isolate);
     return ReadOnlyRoots(isolate).true_value();
   }
 

src/objects/js-atomics-synchronization-inl.h

@@ -23,6 +23,22 @@ TQ_OBJECT_CONSTRUCTORS_IMPL(JSAtomicsMutex)
 
 CAST_ACCESSOR(JSAtomicsMutex)
 
+JSAtomicsMutex::LockGuard::LockGuard(Isolate* isolate,
+                                     Handle<JSAtomicsMutex> mutex)
+    : isolate_(isolate), mutex_(mutex) {
+  JSAtomicsMutex::Lock(isolate, mutex);
+}
+
+JSAtomicsMutex::LockGuard::~LockGuard() { mutex_->Unlock(isolate_); }
+
+JSAtomicsMutex::TryLockGuard::TryLockGuard(Isolate* isolate,
+                                           Handle<JSAtomicsMutex> mutex)
+    : isolate_(isolate), mutex_(mutex), locked_(mutex->TryLock()) {}
+
+JSAtomicsMutex::TryLockGuard::~TryLockGuard() {
+  if (locked_) mutex_->Unlock(isolate_);
+}
+
 // static
 void JSAtomicsMutex::Lock(Isolate* requester, Handle<JSAtomicsMutex> mutex) {
   DisallowGarbageCollection no_gc;

src/objects/js-atomics-synchronization.h

@@ -37,12 +37,49 @@ class WaiterQueueNode;
 //     that the lock can only be used with main thread isolates (including
 //     workers) but not with helper threads that have their own LocalHeap.
 //
-// This mutex manages its own queue of waiting threads under contention, i.e. a
+// This mutex manages its own queue of waiting threads under contention, i.e.
 // it implements a futex in userland. The algorithm is inspired by WebKit's
 // ParkingLot.
 class JSAtomicsMutex
     : public TorqueGeneratedJSAtomicsMutex<JSAtomicsMutex, JSObject> {
  public:
+  // A non-copyable wrapper class that provides an RAII-style mechanism for
+  // owning the JSAtomicsMutex.
+  //
+  // The mutex is locked when a LockGuard object is created, and unlocked when
+  // the LockGuard object is destructed.
+  class V8_NODISCARD LockGuard final {
+   public:
+    inline LockGuard(Isolate* isolate, Handle<JSAtomicsMutex> mutex);
+    LockGuard(const LockGuard&) = delete;
+    LockGuard& operator=(const LockGuard&) = delete;
+    inline ~LockGuard();
+
+   private:
+    Isolate* isolate_;
+    Handle<JSAtomicsMutex> mutex_;
+  };
+
+  // A non-copyable wrapper class that provides an RAII-style mechanism for
+  // attempting to take ownership of a JSAtomicsMutex via its TryLock method.
+  //
+  // The mutex is attempted to be locked via TryLock when a TryLockGuard object
+  // is created. If the mutex was acquired, then it is released when the
+  // TryLockGuard object is destructed.
+  class V8_NODISCARD TryLockGuard final {
+   public:
+    inline TryLockGuard(Isolate* isolate, Handle<JSAtomicsMutex> mutex);
+    TryLockGuard(const TryLockGuard&) = delete;
+    TryLockGuard& operator=(const TryLockGuard&) = delete;
+    inline ~TryLockGuard();
+    bool locked() const { return locked_; }
+
+   private:
+    Isolate* isolate_;
+    Handle<JSAtomicsMutex> mutex_;
+    bool locked_;
+  };
+
   DECL_CAST(JSAtomicsMutex)
   DECL_PRINTER(JSAtomicsMutex)
   EXPORT_DECL_VERIFIER(JSAtomicsMutex)

test/mjsunit/shared-memory/mutex.js

@@ -31,3 +31,16 @@ Atomics.Mutex.lock(mutex, () => {
   assertFalse(Atomics.Mutex.tryLock(mutex, () => { throw "unreachable"; }));
 });
 assertEquals(locked_count, 4);
+
+// Throwing in the callback should unlock the mutex.
+assertThrowsEquals(() => {
+  Atomics.Mutex.lock(mutex, () => { throw 42; });
+}, 42);
+Atomics.Mutex.lock(mutex, () => { locked_count++; });
+assertEquals(locked_count, 5);
+
+assertThrowsEquals(() => {
+  Atomics.Mutex.tryLock(mutex, () => { throw 42; });
+}, 42);
+Atomics.Mutex.tryLock(mutex, () => { locked_count++; });
+assertEquals(locked_count, 6);