Checkpoint

eugeneo · Jan 3, 2025 · 1e7989d · 1e7989d
1 parent 1bda8f8
commit 1e7989d
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 7 deletions.
diff --git a/src/core/lib/event_engine/posix_engine/ev_epoll1_linux.cc b/src/core/lib/event_engine/posix_engine/ev_epoll1_linux.cc
@@ -377,7 +377,7 @@ void Epoll1Poller::Close() {
 
   if (g_epoll_set_.epfd.ready()) {
     system_api_.Close(g_epoll_set_.epfd);
-    g_epoll_set_.epfd = FileDescriptor(-1);
+    g_epoll_set_.epfd = FileDescriptor();
   }
 
   while (!free_epoll1_handles_list_.empty()) {

diff --git a/src/core/lib/event_engine/posix_engine/file_descriptors.cc b/src/core/lib/event_engine/posix_engine/file_descriptors.cc
@@ -16,6 +16,7 @@
 
 #include <grpc/support/port_platform.h>
 
+#include <atomic>
 #include <unordered_set>
 #include <utility>
 
@@ -45,8 +46,9 @@ FileDescriptor FileDescriptors::Add(int fd) {
     LOG(INFO) << "Adding FD: " << fd;
     grpc_core::MutexLock lock(&list_mu_);
     fds_.insert(fd);
+    return FileDescriptor(fd, generation_.load(std::memory_order_relaxed));
   }
-  return FileDescriptor{fd};
+  return FileDescriptor(fd, -1);
 }
 
 absl::optional<int> FileDescriptors::Remove(const FileDescriptor& fd) {
@@ -58,7 +60,8 @@ absl::optional<int> FileDescriptors::Remove(const FileDescriptor& fd) {
   return absl::nullopt;
 }
 
-std::unordered_set<int> FileDescriptors::Clear() {
+std::unordered_set<int> FileDescriptors::AdvanceGeneration() {
+  ++generation_;
   grpc_core::MutexLock lock(&list_mu_);
   std::unordered_set<int> ret;
   std::swap(fds_, ret);
@@ -67,6 +70,9 @@ std::unordered_set<int> FileDescriptors::Clear() {
 }
 
 absl::StatusOr<LockedFd> FileDescriptors::Lock(const FileDescriptor& fd) const {
+  if (fd.generation() != generation_.load(std::memory_order_relaxed)) {
+    return absl::InternalError("Fd from previous generation");
+  }
   auto posix_lock = PosixLock();
   if (!posix_lock.ok()) {
     return std::move(posix_lock).status();

diff --git a/src/core/lib/event_engine/posix_engine/file_descriptors.h b/src/core/lib/event_engine/posix_engine/file_descriptors.h
@@ -15,6 +15,7 @@
 #ifndef GRPC_SRC_CORE_LIB_EVENT_ENGINE_POSIX_ENGINE_FILE_DESCRIPTORS_H
 #define GRPC_SRC_CORE_LIB_EVENT_ENGINE_POSIX_ENGINE_FILE_DESCRIPTORS_H
 
+#include <atomic>
 #include <unordered_set>
 
 #include "absl/status/status.h"
@@ -62,17 +63,20 @@ class LockedFd {
 class FileDescriptor {
  public:
   FileDescriptor() = default;
-  explicit FileDescriptor(int fd) : fd_(fd) {}
+  explicit FileDescriptor(int fd, int generation)
+      : fd_(fd), generation_(generation) {}
 
   bool ready() const { return fd_ > 0; }
   void invalidate() { fd_ = -1; }
 
   // Not meant to use to access FD for I/O. Only used for debug logging.
   int debug_fd() const { return fd_; }
   int fd() const { return fd_; }
+  int generation() const { return generation_; }
 
  private:
   int fd_ = -1;
+  int generation_ = -1;
 };
 
 class FileDescriptors {
@@ -90,7 +94,7 @@ class FileDescriptors {
 
   FileDescriptor Add(int fd);
   absl::optional<int> Remove(const FileDescriptor& fd);
-  std::unordered_set<int> Clear();
+  std::unordered_set<int> AdvanceGeneration();
   absl::StatusOr<LockedFd> Lock(const FileDescriptor& fd) const;
   void Unlock(const FileDescriptor& fd) const;
   absl::StatusOr<ReentrantLock> PosixLock() const;
@@ -117,6 +121,7 @@ class FileDescriptors {
   mutable int locked_descriptors_ ABSL_GUARDED_BY(&mu_) = 0;
   State state_ ABSL_GUARDED_BY(&mu_) = State::kReady;
   mutable grpc_core::CondVar io_cond_;
+  std::atomic_int generation_{1};
 };
 
 }  // namespace experimental

diff --git a/src/core/lib/event_engine/posix_engine/posix_system_api.cc b/src/core/lib/event_engine/posix_engine/posix_system_api.cc
@@ -168,7 +168,7 @@ absl::StatusOr<FileDescriptor> SystemApi::Accept4(
 
 void SystemApi::AdvanceGeneration() {
   LOG(INFO) << "Advancing generation!";
-  for (int fd : fds_.Clear()) {
+  for (int fd : fds_.AdvanceGeneration()) {
     LOG(INFO) << "Closing " << fd;
     close(fd);
   }

diff --git a/test/core/event_engine/posix/posix_system_api_test.cc b/test/core/event_engine/posix/posix_system_api_test.cc
@@ -384,7 +384,7 @@ TEST(PosixSystemApiTest, ChildFork) {
   EXPECT_THAT(CallSayHello(stub), IsOkStatus());
   // Simulating fork
   auto ee = GetDefaultEventEngine();
-  LOG(INFO) << "EventEngine: " << ee.get();
+  LOG(INFO) << "EventEngine: " << ee.get() << " pid: " << getpid();
   PosixEventEngine* posix_ee = static_cast<PosixEventEngine*>(ee.get());
   ASSERT_THAT(posix_ee->HandlePreFork(), IsOk());
   ASSERT_THAT(posix_ee->HandleForkInChild(), IsOk());