Skip to content

Commit

Permalink
fix TreeInode::doPrefetch crash
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kavehahmadi60 authored and facebook-github-bot committed Sep 28, 2024
1 parent 3bdc1a8 commit 6a3f91e
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion eden/fs/inodes/TreeInode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 6a3f91e

Please sign in to comment.