From 6a3f91eaafdfe86942e0e438ba5d844fe9b0c910 Mon Sep 17 00:00:00 2001 From: Kaveh Ahmadi Date: Sat, 28 Sep 2024 12:05:26 -0700 Subject: [PATCH] fix TreeInode::doPrefetch crash Summary: We have some crashes on macOS and Linux in `TreeInode::doPrefetch` See this example on macOS: https://fburl.com/scuba/osquery_crash_reports/nwpiy38w and these on linux: https://fburl.com/scuba/coredumper/gjno0jyf Looking at the code the ServerThreadPool here send run the futures and collect all the results with `collectAll`. The crash is happening inside this `thenValue`: https://www.internalfb.com/code/fbsource/[8e01d2961164]/fbcode/eden/fs/inodes/TreeInode.cpp?lines=4696-4698 If one of the futures fails, the `collectAll` returns exception for that one, and finish the function while the rest of them continue to work which could be the cause of the crash in accessing invalid memory. See the crash logs here: https://www.internalfb.com/phabricator/paste/view/P1614967024?lines=21-37 Changing `collectAll` to `collectAllSafe` then it wait for all the futures to return, and then return exception if any of them fail. This could fix that crash. Thanks to genevievehelsel for suggesting this fix and teaching me how `collectAll` and `collectAllSafe` works. Reviewed By: MichaelCuevas Differential Revision: D63467931 fbshipit-source-id: 3c05aab86b6bada7ab24c5693e55b3a4380560af --- eden/fs/inodes/TreeInode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eden/fs/inodes/TreeInode.cpp b/eden/fs/inodes/TreeInode.cpp index f5ee2439cd70d..0133c9736617d 100644 --- a/eden/fs/inodes/TreeInode.cpp +++ b/eden/fs/inodes/TreeInode.cpp @@ -4707,7 +4707,7 @@ void TreeInode::doPrefetch( load.finish(); } - return collectAll(std::move(inodeFutures)) + return collectAllSafe(std::move(inodeFutures)) .thenTry([lease = std::move(lease)](auto&&) { XLOG(DBG4) << "finished prefetch for " << lease.getTreeInode()->getLogPath();