avoid reading of uninit data

vvfedorenko · facebook-github-bot · commit 6c2902b56f4e · 2025-06-03T06:26:00.000-07:00
Summary:
the tests showed that the reader thread can start before the writer did the first write. That means the load function will return uninit data. Let's use a sequence value of 0 as special indicator of uninit structure
add a bit more print info for the failed test

Reviewed By: t3lurid3

Differential Revision: D75869421

fbshipit-source-id: c12c6689578b1ec705a0dad224b3b21c5d50578f
diff --git a/fbclock/cpp_test/test.cpp b/fbclock/cpp_test/test.cpp
@@ -179,6 +179,7 @@ int reader_thread_v2(fbclock_shmdata_v2* shmp, int tries) {
   for (int i = 0; i < tries; i++) {
     err = fbclock_clockdata_load_data_v2(shmp, &data);
     if (err != 0) {
+      printf("load v2 data failed: %d\n", err);
       return err;
     }
     if (data.ingress_time_ns * 2 != data.error_bound_ns) {
diff --git a/fbclock/fbclock.c b/fbclock/fbclock.c
@@ -104,6 +104,8 @@ int fbclock_clockdata_store_data_v2(uint32_t fd, fbclock_clockdata_v2* data) {
   memcpy(&shmp->data, data, FBCLOCK_CLOCKDATA_V2_SIZE);
   __sync_synchronize();
   seq++;
+  if (!seq)
+    seq += 2; // avoid 0 value on wraparound
   atomic_store(&shmp->seq, seq);
   munmap(shmp, FBCLOCK_SHMDATA_V2_SIZE);
   return FBCLOCK_E_NO_ERROR;
@@ -133,6 +135,11 @@ int fbclock_clockdata_load_data_v2(
     fbclock_clockdata_v2* data) {
   for (int i = 0; i < FBCLOCK_MAX_READ_TRIES; i++) {
     uint64_t seq = atomic_load(&shmp->seq);
+    if (!seq) { // 0 value means uninitialized
+      usleep(10);
+      __sync_synchronize();
+      continue;
+    }
     if (seq & 1) {
       __sync_synchronize();
       continue;

Original file line number	Diff line number	Diff line change
`@@ -179,6 +179,7 @@ int reader_thread_v2(fbclock_shmdata_v2* shmp, int tries) {`
`179`	`179`	`for (int i = 0; i < tries; i++) {`
`180`	`180`	`err = fbclock_clockdata_load_data_v2(shmp, &data);`
`181`	`181`	`if (err != 0) {`
	`182`	`+ printf("load v2 data failed: %d\n", err);`
`182`	`183`	`return err;`
`183`	`184`	`}`
`184`	`185`	`if (data.ingress_time_ns * 2 != data.error_bound_ns) {`