From e164ef746075421af2b8d70c7adfab1908bfd053 Mon Sep 17 00:00:00 2001
From: AJ Heller <hork@google.com>
Date: Tue, 28 Feb 2023 18:30:53 -0800
Subject: [PATCH] [EventEngine] Add Listener Bind-after-Start test (#32502)

This also alters changes the PosixEventEngine to return a failure status
instead of crashing.

cc @yijiem
---
 .../posix_engine/posix_engine_listener.cc     |  7 +++++--
 .../posix/oracle_event_engine_posix.cc        |  4 ++++
 .../test_suite/tests/server_test.cc           | 20 +++++++++++++++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/core/lib/event_engine/posix_engine/posix_engine_listener.cc b/src/core/lib/event_engine/posix_engine/posix_engine_listener.cc
index 9290784879c..b395bff00d6 100644
--- a/src/core/lib/event_engine/posix_engine/posix_engine_listener.cc
+++ b/src/core/lib/event_engine/posix_engine/posix_engine_listener.cc
@@ -64,11 +64,14 @@ PosixEngineListenerImpl::PosixEngineListenerImpl(
 absl::StatusOr<int> PosixEngineListenerImpl::Bind(
     const EventEngine::ResolvedAddress& addr,
     PosixListenerWithFdSupport::OnPosixBindNewFdCallback on_bind_new_fd) {
+  absl::MutexLock lock(&this->mu_);
+  if (this->started_) {
+    return absl::FailedPreconditionError(
+        "Listener is already started, ports can no longer be bound");
+  }
   EventEngine::ResolvedAddress res_addr = addr;
   EventEngine::ResolvedAddress addr6_v4mapped;
   int requested_port = ResolvedAddressGetPort(res_addr);
-  absl::MutexLock lock(&this->mu_);
-  GPR_ASSERT(!this->started_);
   GPR_ASSERT(addr.size() <= EventEngine::ResolvedAddress::MAX_SIZE_BYTES);
   UnlinkIfUnixDomainSocket(addr);
 
diff --git a/test/core/event_engine/test_suite/posix/oracle_event_engine_posix.cc b/test/core/event_engine/test_suite/posix/oracle_event_engine_posix.cc
index d1490e47ecf..ee3bb8bae9b 100644
--- a/test/core/event_engine/test_suite/posix/oracle_event_engine_posix.cc
+++ b/test/core/event_engine/test_suite/posix/oracle_event_engine_posix.cc
@@ -388,6 +388,10 @@ void PosixOracleListener::HandleIncomingConnections() {
 absl::StatusOr<int> PosixOracleListener::Bind(
     const EventEngine::ResolvedAddress& addr) {
   grpc_core::MutexLock lock(&mu_);
+  if (is_started_) {
+    return absl::FailedPreconditionError(
+        "Listener is already started, ports can no longer be bound");
+  }
   int new_socket;
   int opt = -1;
   grpc_resolved_address address = CreateGRPCResolvedAddress(addr);
diff --git a/test/core/event_engine/test_suite/tests/server_test.cc b/test/core/event_engine/test_suite/tests/server_test.cc
index ad1d270640d..d36dabd6ff1 100644
--- a/test/core/event_engine/test_suite/tests/server_test.cc
+++ b/test/core/event_engine/test_suite/tests/server_test.cc
@@ -74,6 +74,26 @@ constexpr int kNumExchangedMessages = 100;
 
 }  // namespace
 
+TEST_F(EventEngineServerTest, CannotBindAfterStarted) {
+  std::shared_ptr<EventEngine> engine(this->NewEventEngine());
+  ChannelArgsEndpointConfig config;
+  auto listener = engine->CreateListener(
+      [](std::unique_ptr<Endpoint>, grpc_core::MemoryAllocator) {},
+      [](absl::Status) {}, config,
+      std::make_unique<grpc_core::MemoryQuota>("foo"));
+  // Bind an initial port to ensure normal listener startup
+  auto resolved_addr = URIToResolvedAddress(absl::StrCat(
+      "ipv6:[::1]:", std::to_string(grpc_pick_unused_port_or_die())));
+  ASSERT_TRUE(resolved_addr.ok());
+  ASSERT_TRUE((*listener)->Bind(*resolved_addr).ok());
+  ASSERT_TRUE((*listener)->Start().ok());
+  // A subsequent bind, which should fail
+  auto resolved_addr2 = URIToResolvedAddress(absl::StrCat(
+      "ipv6:[::1]:", std::to_string(grpc_pick_unused_port_or_die())));
+  ASSERT_TRUE(resolved_addr2.ok());
+  ASSERT_FALSE((*listener)->Bind(*resolved_addr2).ok());
+}
+
 // Create a connection using the oracle EventEngine to a listener created
 // by the Test EventEngine and exchange bi-di data over the connection.
 // For each data transfer, verify that data written at one end of the stream