[EventEngine] Only use fork handlers when enabled via an environment variable (#33582)

Co-authored-by: drfloob <drfloob@users.noreply.github.com>

CMakeLists.txt

@@ -12314,8 +12314,6 @@ target_link_libraries(forkable_test
   ${_gRPC_PROTOBUF_LIBRARIES}
   ${_gRPC_ZLIB_LIBRARIES}
   ${_gRPC_ALLTARGETS_LIBRARIES}
-  absl::flat_hash_set
-  absl::statusor
   gpr
 )
 

build_autogenerated.yaml

@@ -8013,8 +8013,6 @@ targets:
   - src/core/lib/event_engine/forkable.cc
   - test/core/event_engine/forkable_test.cc
   deps:
-  - absl/container:flat_hash_set
-  - absl/status:statusor
   - gpr
 - name: format_request_test
   gtest: true

src/core/BUILD

@@ -1374,9 +1374,10 @@ grpc_cc_library(
     hdrs = [
         "lib/event_engine/forkable.h",
     ],
-    external_deps = ["absl/container:flat_hash_set"],
+    external_deps = ["absl/base:core_headers"],
     deps = [
         "no_destruct",
+        "//:config_vars",
         "//:gpr",
         "//:gpr_platform",
     ],

src/core/lib/event_engine/forkable.cc

@@ -16,12 +16,19 @@
 
 #include "src/core/lib/event_engine/forkable.h"
 
-#ifdef GRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK
+#include <grpc/support/log.h>
 
+#ifdef GRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK
 #include <pthread.h>
+#endif
+
+#include <algorithm>
+#include <utility>
+#include <vector>
 
-#include "absl/container/flat_hash_set.h"
+#include "absl/base/thread_annotations.h"
 
+#include "src/core/lib/config/config_vars.h"
 #include "src/core/lib/gprpp/no_destruct.h"
 #include "src/core/lib/gprpp/sync.h"
 
@@ -35,6 +42,11 @@ bool g_registered ABSL_GUARDED_BY(g_mu){false};
 // This must be ordered because there are ordering dependencies between
 // certain fork handlers.
 grpc_core::NoDestruct<std::vector<Forkable*>> g_forkables ABSL_GUARDED_BY(g_mu);
+
+bool IsForkEnabled() {
+  static bool enabled = grpc_core::ConfigVars::Get().EnableForkSupport();
+  return enabled;
+}
 }  // namespace
 
 Forkable::Forkable() { ManageForkable(this); }
@@ -42,65 +54,58 @@ Forkable::Forkable() { ManageForkable(this); }
 Forkable::~Forkable() { StopManagingForkable(this); }
 
 void RegisterForkHandlers() {
+  if (IsForkEnabled()) {
     grpc_core::MutexLock lock(g_mu.get());
     if (!std::exchange(g_registered, true)) {
+#ifdef GRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK
       pthread_atfork(PrepareFork, PostforkParent, PostforkChild);
+#endif
+    }
   }
 };
 
 void PrepareFork() {
+  if (IsForkEnabled()) {
     grpc_core::MutexLock lock(g_mu.get());
     for (auto forkable_iter = g_forkables->rbegin();
          forkable_iter != g_forkables->rend(); ++forkable_iter) {
       (*forkable_iter)->PrepareFork();
     }
   }
+}
 void PostforkParent() {
+  if (IsForkEnabled()) {
     grpc_core::MutexLock lock(g_mu.get());
     for (auto* forkable : *g_forkables) {
       forkable->PostforkParent();
     }
   }
+}
 
 void PostforkChild() {
+  if (IsForkEnabled()) {
     grpc_core::MutexLock lock(g_mu.get());
     for (auto* forkable : *g_forkables) {
       forkable->PostforkChild();
     }
   }
+}
 
 void ManageForkable(Forkable* forkable) {
+  if (IsForkEnabled()) {
     grpc_core::MutexLock lock(g_mu.get());
     g_forkables->push_back(forkable);
   }
+}
 
 void StopManagingForkable(Forkable* forkable) {
+  if (IsForkEnabled()) {
     grpc_core::MutexLock lock(g_mu.get());
     auto iter = std::find(g_forkables->begin(), g_forkables->end(), forkable);
     GPR_ASSERT(iter != g_forkables->end());
     g_forkables->erase(iter);
   }
+}
 
 }  // namespace experimental
 }  // namespace grpc_event_engine
-
-#else  // GRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK
-
-namespace grpc_event_engine {
-namespace experimental {
-
-Forkable::Forkable() {}
-Forkable::~Forkable() {}
-
-void RegisterForkHandlers() {}
-void PrepareFork() {}
-void PostforkParent() {}
-void PostforkChild() {}
-
-void ManageForkable(Forkable* /* forkable */) {}
-void StopManagingForkable(Forkable* /* forkable */) {}
-
-}  // namespace experimental
-}  // namespace grpc_event_engine
-
-#endif  // GRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK

test/core/event_engine/BUILD

@@ -39,11 +39,11 @@ grpc_cc_test(
     name = "forkable_test",
     srcs = ["forkable_test.cc"],
     external_deps = [
-        "absl/time",
+        "absl/types:optional",
         "gtest",
     ],
     deps = [
-        "//:event_engine_base_hdrs",
+        "//:config_vars",
         "//:gpr",
         "//:gpr_platform",
         "//src/core:forkable",

test/core/event_engine/forkable_test.cc

@@ -14,25 +14,21 @@
 
 #include <grpc/support/port_platform.h>
 
-#ifndef GRPC_ENABLE_FORK_SUPPORT
-
-// Test nothing, everything is fine
-int main(int /* argc */, char** /* argv */) { return 0; }
-
-#else  // GRPC_ENABLE_FORK_SUPPORT
+#include "src/core/lib/event_engine/forkable.h"
 
+#ifdef GPR_POSIX_SUBPROCESS
+#include <errno.h>
+#include <stdlib.h>
 #include <sys/wait.h>
 #include <unistd.h>
+#endif  // GPR_POSIX_SUBPROCESS
 
-#include <gtest/gtest.h>
-
-#include "absl/time/clock.h"
+#include "absl/types/optional.h"
+#include "gtest/gtest.h"
 
-#include <grpc/grpc.h>
 #include <grpc/support/log.h>
 
-#include "src/core/lib/event_engine/forkable.h"
-#include "src/core/lib/gprpp/crash.h"
+#include "src/core/lib/config/config_vars.h"
 
 namespace {
 using ::grpc_event_engine::experimental::Forkable;
@@ -41,7 +37,7 @@ using ::grpc_event_engine::experimental::RegisterForkHandlers;
 
 class ForkableTest : public testing::Test {};
 
-#ifdef GRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK
+#ifdef GPR_POSIX_SUBPROCESS
 TEST_F(ForkableTest, BasicPthreadAtForkOperations) {
   class SomeForkable : public Forkable {
    public:
@@ -50,15 +46,27 @@ TEST_F(ForkableTest, BasicPthreadAtForkOperations) {
     void PostforkChild() override { child_called_ = true; }
 
     void CheckParent() {
+#ifdef GRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK
       EXPECT_TRUE(prepare_called_);
       EXPECT_TRUE(parent_called_);
       EXPECT_FALSE(child_called_);
+#else
+      EXPECT_FALSE(prepare_called_);
+      EXPECT_FALSE(parent_called_);
+      EXPECT_FALSE(child_called_);
+#endif
     }
 
     void CheckChild() {
+#ifdef GRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK
       EXPECT_TRUE(prepare_called_);
       EXPECT_FALSE(parent_called_);
       EXPECT_TRUE(child_called_);
+#else
+      EXPECT_FALSE(prepare_called_);
+      EXPECT_FALSE(parent_called_);
+      EXPECT_FALSE(child_called_);
+#endif
     }
 
    private:
@@ -92,13 +100,50 @@ TEST_F(ForkableTest, BasicPthreadAtForkOperations) {
     }
   }
 }
-#endif  // GRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK
+#endif  // GPR_POSIX_SUBPROCESS
+
+TEST_F(ForkableTest, NonPthreadManualForkOperations) {
+  // Manually simulates a fork event for non-pthread-enabled environments
+#ifdef GRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK
+  // This platform does not need to exercise fork support manually.
+  GTEST_SKIP("Unnecessary test, this platform supports pthreads.");
+#endif
+
+  class SomeForkable : public Forkable {
+   public:
+    void PrepareFork() override { prepare_called_ = true; }
+    void PostforkParent() override { parent_called_ = true; }
+    void PostforkChild() override { child_called_ = true; }
+
+    void AssertStates(bool prepare, bool parent, bool child) {
+      EXPECT_EQ(prepare_called_, prepare);
+      EXPECT_EQ(parent_called_, parent);
+      EXPECT_EQ(child_called_, child);
+    }
+
+   private:
+    bool prepare_called_ = false;
+    bool parent_called_ = false;
+    bool child_called_ = false;
+  };
+
+  SomeForkable forkable;
+  forkable.AssertStates(/*prepare=*/false, /*parent=*/false, /*child=*/false);
+  grpc_event_engine::experimental::PrepareFork();
+  forkable.AssertStates(/*prepare=*/true, /*parent=*/false, /*child=*/false);
+  grpc_event_engine::experimental::PostforkParent();
+  forkable.AssertStates(/*prepare=*/true, /*parent=*/true, /*child=*/false);
+  grpc_event_engine::experimental::PostforkChild();
+  forkable.AssertStates(/*prepare=*/true, /*parent=*/true, /*child=*/true);
+}
 
 int main(int argc, char** argv) {
   testing::InitGoogleTest(&argc, argv);
+  // Force enable fork support to allow testing the fork handler registry.
+  grpc_core::ConfigVars::Overrides config_overrides;
+  config_overrides.enable_fork_support = true;
+  grpc_core::ConfigVars::SetOverrides(config_overrides);
   RegisterForkHandlers();
   auto result = RUN_ALL_TESTS();
   return result;
 }
-
-#endif  // GRPC_ENABLE_FORK_SUPPORT