dbghlp: Make mutex name unique to the process #518

ChrisDenton · 2023-03-31T21:02:08Z

Fixes #399.

ChrisDenton · 2023-03-31T21:08:06Z

Note: this PR only affects Windows. I'm not sure why the Ubuntu and Android tests are failing.

bjorn3 · 2023-03-31T21:11:15Z

src/dbghelp.rs

-                "Local\\RustBacktraceMutex\0".as_ptr() as _,
-            ) as usize;
+            let name = mutex_name();
+            lock = CreateMutexA(ptr::null_mut(), 0, name.as_ptr().cast::<i8>()) as usize;


Does windows remove the mutex again once all open handles are closed or will this keep accumulating mutexes until the session ends?

https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-createmutexa

Use the CloseHandle function to close the handle. The system closes the handle automatically when the process terminates. The mutex object is destroyed when its last handle has been closed.

p--b · 2023-05-15T09:16:18Z

Is there anything we can do to help get this PR merged? This bug is biting us in production currently.

workingjubilee

Rerunning the tests gets everything but the Android tests passing, and we're not blocking merges on the Android tests, so in this goes.

workingjubilee · 2023-06-22T02:39:28Z

src/dbghelp.rs

+    // Helper function for generating a name that's unique to the process.
+    fn mutex_name() -> [u8; 33] {
+        let mut name: [u8; 33] = *b"Local\\RustBacktraceMutex00000000\0";
+        let mut id = unsafe { GetCurrentProcessId() };


Windows guarantees the return value is unique across the system until the process dies. Since this effectively means a process-wide mutex, this could conceivably complicate the implementation of something like #473 further. However, that does not seem to have momentum, and this is a necessary fix, so it's fine to take this even if it inconveniences that.

workingjubilee · 2023-06-22T02:39:50Z

src/dbghelp.rs

+            name[index - 1] = match (id & 0xF) as u8 {
+                h @ 0..=9 => b'0' + h,
+                h => b'A' + (h - 10),
+            };
+            id >>= 4;
+            index -= 1;


That is... quite rudimentary. I'm impressed.

bjorn3 reviewed Mar 31, 2023

View reviewed changes

ChrisDenton force-pushed the mutex-name branch from 896daf8 to befb41d Compare March 31, 2023 21:55

dbghlp: Make mutex name unique to the process

e90def2

ChrisDenton force-pushed the mutex-name branch from befb41d to e90def2 Compare March 31, 2023 23:30

workingjubilee closed this Jun 22, 2023

workingjubilee reopened this Jun 22, 2023

workingjubilee approved these changes Jun 22, 2023

View reviewed changes

workingjubilee merged commit dab8b61 into rust-lang:master Jun 22, 2023

ChrisDenton deleted the mutex-name branch June 22, 2023 06:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dbghlp: Make mutex name unique to the process #518

dbghlp: Make mutex name unique to the process #518

ChrisDenton commented Mar 31, 2023

ChrisDenton commented Mar 31, 2023

bjorn3 Mar 31, 2023

ChrisDenton Mar 31, 2023

p--b commented May 15, 2023

workingjubilee left a comment

workingjubilee Jun 22, 2023

workingjubilee Jun 22, 2023

dbghlp: Make mutex name unique to the process #518

dbghlp: Make mutex name unique to the process #518

Conversation

ChrisDenton commented Mar 31, 2023

ChrisDenton commented Mar 31, 2023

bjorn3 Mar 31, 2023

Choose a reason for hiding this comment

ChrisDenton Mar 31, 2023

Choose a reason for hiding this comment

p--b commented May 15, 2023

workingjubilee left a comment

Choose a reason for hiding this comment

workingjubilee Jun 22, 2023

Choose a reason for hiding this comment

workingjubilee Jun 22, 2023

Choose a reason for hiding this comment