rgw/posix: start the Inotify thread last, after the rest is built

f62e811f9ef fixed the wfd/efd init-order race but missed a sibling: thrd
was still declared before map_mutex, the watch maps and the shutdown flag.
Members come up in declaration order, so building thrd kicks off ev_loop()
while those are still uninitialized.

That is bad news, because ev_loop() reads shutdown and, when an event
arrives, locks map_mutex and pokes at the maps. Doing any of that before
they are constructed is undefined behavior: reading the shutdown atomic
before its initializer has even stored false, or locking a std::mutex that
does not exist yet. Making shutdown a std::atomic<bool> made concurrent
access well-defined, but that does not help if the load happens before the
object is constructed.

So just declare thrd last, and the thread will not start until everything
it touches is ready. wfd/efd stay ahead of it, so the earlier fix still
holds.

helgrind caught the shutdown read in ev_loop() racing its own initializer
in the constructor.

Fixes: https://tracker.ceph.com/issues/75601
Signed-off-by: Kefu Chai <k.chai@proxmox.com>

diff --git a/src/rgw/driver/posix/notify.h b/src/rgw/driver/posix/notify.h
index bdfd5ec582ab04922c402c45a749d83f3f54dcd3..99c2397d4832bbcfffc0806e753649911e725788 100644 (file)
--- a/src/rgw/driver/posix/notify.h
+++ b/src/rgw/driver/posix/notify.h
@@ -112,11 +112,13 @@ namespace file::listing {
     using wd_remove_map_t = ankerl::unordered_dense::map<std::string, int>;
 
     int wfd, efd;
-    std::thread thrd;
     std::mutex map_mutex;  // protects wd_callback_map and wd_remove_map
     wd_callback_map_t wd_callback_map;
     wd_remove_map_t wd_remove_map;
     std::atomic<bool> shutdown{false};
+    /* must be last: starting the thread (ev_loop) reaches every member
+     * above, which are constructed in declaration order */
+    std::thread thrd;
 
     class AlignedBuf
     {