libuv: Failed test process_title_threadsafe

I guess now I know what is happening. So @richardlau, your original question seem to be valid, and very much relevant to the issue. This is the sequence I figured out that led to this one byte corruption:

• so many uv_set_process_title and uv_get_process_title are being called, with the former on many threads, and the later on the main.

• modification as well as retrieval of the title string is carried out under the cover of a mutex

• the title string itself is a static non-local variable for persistence across calls.

• in uv_get_process_title we calculate the length of the title string before entering the mutex. In the problematic scenario, this can cause:

  • A thread acquires the mutex and wants to modify the title
  • It sees the title has a value, so it frees it, before attaching the new one.
  • after it frees but before it could attach a new value, main thread calls getter method
  • main thread calculates the length of an already freed data, which it gets as 0.
  • later, when it acquires the lock, the other thread would have written the new string, but the length computed was 0.
  • so memcpy is attempted with (0+1 = 1) byte on the new string.
  • by +1 our intention was to copy NULL byte, but in this case as we are not copying the null, the one byte string continues through the remnant of the previous string, upto its length.

and hence this result.

test case remains in tact, and a comprehensive fix IMO should be:

diff --git a/src/unix/aix.c b/src/unix/aix.c
index 92de814..e41b31d 100644
--- a/src/unix/aix.c
+++ b/src/unix/aix.c
@@ -886,16 +887,23 @@ int uv_set_process_title(const char* title) {
 
 int uv_get_process_title(char* buffer, size_t size) {
   size_t len;
-  len = strlen(process_argv[0]);
   if (buffer == NULL || size == 0)
     return UV_EINVAL;
-  else if (size <= len)
-    return UV_ENOBUFS;
 
   uv_once(&process_title_mutex_once, init_process_title_mutex_once);
   uv_mutex_lock(&process_title_mutex);
 
-  memcpy(buffer, process_argv[0], len + 1);
+  len = strlen(process_argv[0]);
+
+  if (size <= len) {
+    uv_mutex_unlock(&process_title_mutex);
+    return UV_ENOBUFS;
+  }
+
+  assert(len > 0);
+  
+  memcpy(buffer, process_argv[0], len);
+  buffer[len] = NULL;
 
   uv_mutex_unlock(&process_title_mutex);
 

Ran a 2K times and did not see any failure. I will raise a PR for this, but would appreciate if someone agrees to my theory.

@gireeshpunathil Maybe also print len? len == 0 would cause the memcpy not to copy anything.

that may be rac-y, but given all the titles are of equal length (in this case), that does not look to be a problem source - for example, whether we got the length of the previous title or the next tile, it is always going to be the same.