[libc] implement pthread_mutex_trylock

[changkhothuychung]

fix issue #85278

[llvmbot]

@llvm/pr-subscribers-libc

Author: Nhat Nguyen (changkhothuychung)

Changes

fix issue #85278

---

Full diff: https://github.com/llvm/llvm-project/pull/93359.diff

1 Files Affected:

• (modified) libc/src/__support/threads/linux/mutex.h (+21-1)

diff --git a/libc/src/__support/threads/linux/mutex.h b/libc/src/__support/threads/linux/mutex.h
index 6702de4651686..c7f9b4f320dc3 100644
--- a/libc/src/__support/threads/linux/mutex.h
+++ b/libc/src/__support/threads/linux/mutex.h
@@ -116,7 +116,27 @@ struct Mutex {
     }
   }
 
-  MutexError trylock();
+  MutexError trylock() {
+    FutexWordType mutex_status = FutexWordType(LockState::Free);
+    FutexWordType locked_status = FutexWordType(LockState::Locked);
+
+    if (futex_word.compare_exchange_strong(mutex_status,
+                                           FutexWordType(LockState::Locked))) {
+      return MutexError::NONE;
+    }
+
+    switch (LockState(mutex_status)) {
+    case LockState::Locked:
+      if (recursive && this == owner) {
+        lock_count++;
+        return MutexError::NONE;
+      }
+
+    case LockState::Free:
+      // If it was LockState::Free, we shouldn't be here at all.
+      return MutexError::BAD_LOCK_STATE;
+    }
+  }
 };
 
 } // namespace LIBC_NAMESPACE

[SchrodingerZhu]

I think this will conflict with the mutex rework. I will get that part fixed over the weekend. Sorry for that PR lasting longer than I have expected. My jet lag does not agree with me when coding at night 🤦.

[changkhothuychung]

I think this will conflict with the mutex rework. I will get that part fixed over the weekend. Sorry for that PR lasting longer than I have expected. My jet lag does not agree with me when coding at night 🤦.

which PR are you referring to? can u send me the link here? Im not aware of that.

[nickdesaulniers]

Needs unit tests. If you implement
C11 https://en.cppreference.com/w/c/thread/mtx_trylock in terms of this, then you can test it in test/integration/src/threads/mtx_test.cpp. I'd test an uncontended and a contended vanilla Mutex to start with.

[nickdesaulniers]

Suggested change

	if (futex_word.compare_exchange_strong(mutex_status,
	FutexWordType(LockState::Locked))) {
	return MutexError::NONE;
	}
	if (futex_word.compare_exchange_strong(mutex_status,
	FutexWordType(LockState::Locked)))
	return MutexError::NONE;

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

[nickdesaulniers]

This variable appears unused?

[nickdesaulniers]

Suggested change

	if (recursive && this == owner) {
	lock_count++;
	}
	if (recursive && this == owner)
	lock_count++;

Though I think we don't care what the resulting mutex_status value is if it's NOT Lock state::Locked, we should simply return MutexError::BUSY;.

[SchrodingerZhu]

@nickdesaulniers I hope to put a pause to this PR since my mutex patch actually exposes try_lock api. (Sorry again for such conflicts.) It is okay to discuss suggested changes but please keep in mind that the implementation of try_lock using strong CAS may be already covered in another PR in flight.

It can be valuable if you can extend this PR to actually implement the POSIX API and provide test cases for that API.