update_engine: Make sure watched fd outlives the watcher
Make sure the file descriptors that we are watching from curl outlive
base::FileDescriptorWatcher::Controller. Since curl can internally
close some of the fds it has handed out, we make and watch duplicated
fds instead so that we can control their lifetimes.
Closing an fd before destroying the watcher controller object will soon
become a DCHECK failure with the move to a new message loop
implementation.
BUG=b:274837113
TEST=cros_sdk FEATURES="nostrip test" P2_TEST_FILTER="*HttpFetcherTest*FailureTest*" emerge-nissa update_engine
Change-Id: I0bf668356327ff5db550153cb80cd3439f98a60a
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/4639284
Reviewed-by: Jae Hoon Kim <[email protected]>
Commit-Queue: Jae Hoon Kim <[email protected]>
Tested-by: Sami Kyöstilä <[email protected]>
Auto-Submit: Sami Kyöstilä <[email protected]>
diff --git a/libcurl_http_fetcher.cc b/libcurl_http_fetcher.cc
index ab70712..c3b89aa 100644
--- a/libcurl_http_fetcher.cc
+++ b/libcurl_http_fetcher.cc
@@ -727,21 +727,29 @@
if (tracked)
continue;
- // Track a new fd.
+ // Track a new fd. Instead of watching the original fd we watch a
+ // duplicate so we can ensure the fd outlives the file descriptor watcher.
switch (t) {
- case 0: // Read
- fd_controller_maps_[t][fd] =
+ case 0: { // Read
+ int watched_fd = HANDLE_EINTR(dup(fd));
+ fd_controller_maps_[t][fd] = WatchedFd{
+ base::ScopedFD(watched_fd),
base::FileDescriptorWatcher::WatchReadable(
- fd,
+ watched_fd,
base::BindRepeating(&LibcurlHttpFetcher::CurlPerformOnce,
- base::Unretained(this)));
+ base::Unretained(this)))};
break;
- case 1: // Write
- fd_controller_maps_[t][fd] =
+ }
+ case 1: { // Write
+ int watched_fd = HANDLE_EINTR(dup(fd));
+ fd_controller_maps_[t][fd] = WatchedFd{
+ base::ScopedFD(watched_fd),
base::FileDescriptorWatcher::WatchWritable(
- fd,
+ watched_fd,
base::BindRepeating(&LibcurlHttpFetcher::CurlPerformOnce,
- base::Unretained(this)));
+ base::Unretained(this)))};
+ break;
+ }
}
static int io_counter = 0;
io_counter++;
diff --git a/libcurl_http_fetcher.h b/libcurl_http_fetcher.h
index a0dcf9c..00c9b9f 100644
--- a/libcurl_http_fetcher.h
+++ b/libcurl_http_fetcher.h
@@ -25,6 +25,7 @@
#include <curl/curl.h>
#include <base/files/file_descriptor_watcher_posix.h>
+#include <base/files/scoped_file.h>
#include <base/logging.h>
#include <brillo/message_loops/message_loop.h>
@@ -269,8 +270,11 @@
// the message loop. libcurl may open/close descriptors and switch their
// directions so maintain two separate lists so that watch conditions can be
// set appropriately.
- std::map<int, std::unique_ptr<base::FileDescriptorWatcher::Controller>>
- fd_controller_maps_[2];
+ struct WatchedFd {
+ base::ScopedFD fd;
+ std::unique_ptr<base::FileDescriptorWatcher::Controller> controller;
+ };
+ std::map<int, WatchedFd> fd_controller_maps_[2];
// The TaskId of the timer we're waiting on. kTaskIdNull if we are not waiting
// on it.
