[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.
3aae08d25e/src/core/lib/event_engine/posix_engine/event_poller.h (L48-L50)
pull/33006/head
AJ Heller 2 years ago committed by GitHub
parent 0982f82f47
commit 18c1cc5a51
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 5
      include/grpc/event_engine/event_engine.h
  2. 2
      src/core/lib/event_engine/posix_engine/posix_engine_listener.cc
  3. 11
      src/core/lib/event_engine/posix_engine/posix_engine_listener.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.

@ -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) {

@ -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!");

Loading…
Cancel
Save