[PATCH] D54521: tsan: Add pthread_tryjoin_np and pthread_timedjoin_np interceptors

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/tsan/rtl/tsan_interceptors.cc:1057
+  else
+    ThreadNotJoined(thr, pc, tid, (uptr)th);
+  return res;
----------------
Why is this necessary?
We've already set it in CreateThread.
Also this looks racy and can CHECK-fail. Consider that this pthread_tryjoin fails but another thread concurrently also executes pthread_tryjoin and it succeeds and thread is destroyed.


================
Comment at: test/tsan/Linux/thread_timedjoin.c:15
+  pthread_create(&t, 0, Thread, 0);
+  barrier_wait(&barrier);
+  struct timespec ts;
----------------
Why do we need a barrier in this test? It looks like it's deterministic without the barrier.


================
Comment at: test/tsan/Linux/thread_timedjoin.c:22
+  }
+  fprintf(stderr, "PASS\n");
+  return 0;
----------------
Let's also check that pthread_timedjoin_np provides necessary synchronization. It's easy to add to this test and that's an important guarantee of joining a thread.
I.e. create a global var, write to it in the thread function and write in main after join. It must not race.
That will also check that pthread_timedjoin_np did not return some other error.



================
Comment at: test/tsan/Linux/thread_tryjoin.c:15
+  pthread_create(&t, 0, Thread, 0);
+  barrier_wait(&barrier);
+  while (pthread_tryjoin_np(t, 0) == EBUSY)
----------------
The same applies here.


https://reviews.llvm.org/D54521