[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.

[apparentlymart]

I found myself here while trying to understand the current implementation status of the POSIX threads API, so I'm really just a random passer-by here, but FWIW I saw the two mentions of a mutex rework patch above without any direct link to the relevant change and so I went digging and I'm guessing that was referring to #92168, in which case that has now been merged and does appear to include an implementation of try_lock for Linux, here:

llvm-project/libc/src/__support/threads/linux/mutex.h

Lines 80 to 84 in de44a55

	LIBC_INLINE MutexError try_lock() {
	if (this->RawMutex::try_lock())
	return MutexError::NONE;
	return MutexError::BUSY;
	}

llvm-project/libc/src/__support/threads/linux/raw_mutex.h

Lines 99 to 104 in de44a55

	[[nodiscard]] LIBC_INLINE bool try_lock() {
	FutexWordType expected = UNLOCKED;
	// Use strong version since this is a one-time operation.
	return futex.compare_exchange_strong(
	expected, LOCKED, cpp::MemoryOrder::ACQUIRE, cpp::MemoryOrder::RELAXED);
	}

However, as far as I can tell there is still (in main at the time of writing this comment) not an entrypoint for pthread_mutex_trylock, and so this PR could still potentially add that in terms of the Mutex::try_lock function as described in the previous comment.

I'm just sharing this in case it's useful to a future visitor to this PR. I'm sorry if anything I said here is inaccurate.

[SchrodingerZhu]

@apparentlymart Thank you for the nice summary.