[EventEngine] Update contract for Listener's on_shutdown execution (#…

…33003)

See `event_engine.h` for the contract change. All other changes are
cleanup.

I confirmed that both the Posix and Windows implementations comply with
this already.

On Windows, the `WindowsEventEngineListener` will only call
`on_shutdown` after all `SinglePortSocketListener`s have been destroyed,
which ensures that no `on_accept` callback will be executed, even if
there is still trailing overlapped activity on the listening socket.

On Posix, the `PosixEngineListenerImpl` will only call `on_shutdown`
after all `AsyncConnectionAcceptor`s have been destroyed, which ensures
`EventHandle::OrphanHandle` has been called. The `OrphanHandle` contract
indicates that all existing notify closures must have already run. The
implementation looks to comply, so if it does not, that's a bug.
https://github.com/grpc/grpc/blob/3aae08d25e69af36445de7618072d708d839642d/src/core/lib/event_engine/posix_engine/event_poller.h#L48-L50

include/grpc/event_engine/event_engine.h

@@ -283,8 +283,9 @@ class EventEngine : public std::enable_shared_from_this<EventEngine> {
   /// \a on_shutdown will never be called.
   ///
   /// If this method returns a Listener, then \a on_shutdown will be invoked
-  /// exactly once, when the Listener is shut down. The status passed to it will
-  /// indicate if there was a problem during shutdown.
+  /// exactly once when the Listener is shut down, and only after all
+  /// \a on_accept callbacks have finished executing. The status passed to it
+  /// will indicate if there was a problem during shutdown.
   ///
   /// The provided \a MemoryAllocatorFactory is used to create \a
   /// MemoryAllocators for Endpoint construction.

src/core/lib/event_engine/posix_engine/posix_engine_listener.cc

@@ -278,7 +278,7 @@ void PosixEngineListenerImpl::TriggerShutdown() {
 }
 
 PosixEngineListenerImpl::~PosixEngineListenerImpl() {
-  // This should get invoked only after all the AsyncConnectionAcceptor's have
+  // This should get invoked only after all the AsyncConnectionAcceptors have
   // been destroyed. This is because each AsyncConnectionAcceptor has a
   // shared_ptr ref to the parent PosixEngineListenerImpl.
   if (on_shutdown_ != nullptr) {

src/core/lib/event_engine/posix_engine/posix_engine_listener.h

@@ -79,7 +79,7 @@ class PosixEngineListenerImpl
   // This class represents accepting for one bind fd belonging to the listener.
   // Each AsyncConnectionAcceptor takes a ref to the parent
   // PosixEngineListenerImpl object. So the PosixEngineListenerImpl can be
-  // deleted only after all AsyncConnectionAcceptor's get destroyed.
+  // deleted only after all AsyncConnectionAcceptors get destroyed.
   class AsyncConnectionAcceptor {
    public:
     AsyncConnectionAcceptor(std::shared_ptr<EventEngine> engine,
@@ -143,12 +143,11 @@ class PosixEngineListenerImpl
     absl::StatusOr<ListenerSocket> Find(
         const grpc_event_engine::experimental::EventEngine::ResolvedAddress&
             addr) override {
-      for (auto acceptor = acceptors_.begin(); acceptor != acceptors_.end();
-           ++acceptor) {
-        if ((*acceptor)->Socket().addr.size() == addr.size() &&
-            memcmp((*acceptor)->Socket().addr.address(), addr.address(),
+      for (auto* acceptor : acceptors_) {
+        if (acceptor->Socket().addr.size() == addr.size() &&
+            memcmp(acceptor->Socket().addr.address(), addr.address(),
                    addr.size()) == 0) {
-          return (*acceptor)->Socket();
+          return acceptor->Socket();
         }
       }
       return absl::NotFoundError("Socket not found!");