Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EventEngine] Add invalid handle types to the public API #32202

Merged
merged 5 commits into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/grpc/event_engine/event_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class EventEngine : public std::enable_shared_from_this<EventEngine> {
/// caller - the EventEngine will never delete a Closure, and upon
/// cancellation, the EventEngine will simply forget the Closure exists. The
/// caller is responsible for all necessary cleanup.

class Closure {
public:
Closure() = default;
Expand All @@ -103,12 +104,14 @@ class EventEngine : public std::enable_shared_from_this<EventEngine> {
struct TaskHandle {
intptr_t keys[2];
};
static constexpr TaskHandle kInvalidTaskHandle{-1, -1};
/// A handle to a cancellable connection attempt.
///
/// Returned by \a Connect, and can be passed to \a CancelConnect.
struct ConnectionHandle {
intptr_t keys[2];
};
static constexpr ConnectionHandle kInvalidConnectionHandle{-1, -1};
/// Thin wrapper around a platform-specific sockaddr type. A sockaddr struct
/// exists on all platforms that gRPC supports.
///
Expand Down
8 changes: 4 additions & 4 deletions src/core/lib/event_engine/posix_engine/posix_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ EventEngine::ConnectionHandle PosixEventEngine::ConnectInternal(
ep = absl::FailedPreconditionError(absl::StrCat(
"connect failed: ", "invalid addr: ",
addr_uri.value()))]() mutable { on_connect(std::move(ep)); });
return {0, 0};
return EventEngine::kInvalidConnectionHandle;
}

std::string name = absl::StrCat("tcp-client:", addr_uri.value());
Expand All @@ -253,7 +253,7 @@ EventEngine::ConnectionHandle PosixEventEngine::ConnectInternal(
std::move(allocator), options)]() mutable {
on_connect(std::move(ep));
});
return {0, 0};
return EventEngine::kInvalidConnectionHandle;
}
if (saved_errno != EWOULDBLOCK && saved_errno != EINPROGRESS) {
// Connection already failed. Return 0 to discourage any cancellation
Expand All @@ -265,7 +265,7 @@ EventEngine::ConnectionHandle PosixEventEngine::ConnectInternal(
" error: ", std::strerror(saved_errno)))]() mutable {
on_connect(std::move(ep));
});
return {0, 0};
return EventEngine::kInvalidConnectionHandle;
}
AsyncConnect* ac = new AsyncConnect(
std::move(on_connect), shared_from_this(), executor_.get(), handle,
Expand Down Expand Up @@ -554,7 +554,7 @@ EventEngine::ConnectionHandle PosixEventEngine::Connect(
if (!socket.ok()) {
Run([on_connect = std::move(on_connect),
status = socket.status()]() mutable { on_connect(status); });
return {0, 0};
return EventEngine::kInvalidConnectionHandle;
}
return ConnectInternal((*socket).sock, std::move(on_connect),
(*socket).mapped_target_addr,
Expand Down
16 changes: 8 additions & 8 deletions src/core/lib/event_engine/windows/windows_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ EventEngine::ConnectionHandle WindowsEventEngine::Connect(
Run([on_connect = std::move(on_connect), status = uri.status()]() mutable {
on_connect(status);
});
return invalid_connection_handle;
return EventEngine::kInvalidConnectionHandle;
}
GRPC_EVENT_ENGINE_TRACE("EventEngine::%p connecting to %s", this,
uri->c_str());
Expand All @@ -233,14 +233,14 @@ EventEngine::ConnectionHandle WindowsEventEngine::Connect(
status = GRPC_WSA_ERROR(WSAGetLastError(), "WSASocket")]() mutable {
on_connect(status);
});
return invalid_connection_handle;
return EventEngine::kInvalidConnectionHandle;
}
status = PrepareSocket(sock);
if (!status.ok()) {
Run([on_connect = std::move(on_connect), status]() mutable {
on_connect(status);
});
return invalid_connection_handle;
return EventEngine::kInvalidConnectionHandle;
}
// Grab the function pointer for ConnectEx for that specific socket It may
// change depending on the interface.
Expand All @@ -257,7 +257,7 @@ EventEngine::ConnectionHandle WindowsEventEngine::Connect(
"WSAIoctl(SIO_GET_EXTENSION_FUNCTION_POINTER)")]() mutable {
on_connect(status);
});
return invalid_connection_handle;
return EventEngine::kInvalidConnectionHandle;
}
// bind the local address
auto local_address = ResolvedAddressMakeWild6(0);
Expand All @@ -267,7 +267,7 @@ EventEngine::ConnectionHandle WindowsEventEngine::Connect(
status = GRPC_WSA_ERROR(WSAGetLastError(), "bind")]() mutable {
on_connect(status);
});
return invalid_connection_handle;
return EventEngine::kInvalidConnectionHandle;
}
// Connect
auto watched_socket = iocp_.Watch(sock);
Expand All @@ -285,7 +285,7 @@ EventEngine::ConnectionHandle WindowsEventEngine::Connect(
on_connect(status);
});
watched_socket->MaybeShutdown(status);
return invalid_connection_handle;
return EventEngine::kInvalidConnectionHandle;
}
}
GPR_ASSERT(watched_socket != nullptr);
Expand Down Expand Up @@ -321,8 +321,8 @@ EventEngine::ConnectionHandle WindowsEventEngine::Connect(
}

bool WindowsEventEngine::CancelConnect(EventEngine::ConnectionHandle handle) {
if (TaskHandleComparator<ConnectionHandle>::Eq()(handle,
invalid_connection_handle)) {
if (TaskHandleComparator<ConnectionHandle>::Eq()(
handle, EventEngine::kInvalidConnectionHandle)) {
GRPC_EVENT_ENGINE_TRACE("%s",
"Attempted to cancel an invalid connection handle");
return false;
Expand Down
4 changes: 0 additions & 4 deletions src/core/lib/event_engine/windows/windows_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ namespace experimental {
class WindowsEventEngine : public EventEngine,
public grpc_core::KeepsGrpcInitialized {
public:
constexpr static TaskHandle invalid_handle{-1, -1};
constexpr static EventEngine::ConnectionHandle invalid_connection_handle{-1,
-1};

class WindowsListener : public EventEngine::Listener {
public:
~WindowsListener() override;
Expand Down