Skip to content

Commit

Permalink
[EventEngine] Add invalid handle types to the public API (#32202)
Browse files Browse the repository at this point in the history
* [EventEngine] Add invalid handle types to the public API

* Automated change: Fix sanity tests

* add definition for static constexpr members

* sanitize

Co-authored-by: drfloob <drfloob@users.noreply.github.com>
  • Loading branch information
2 people authored and wanlin31 committed May 18, 2023
1 parent 91b30bc commit d68dd6c
Show file tree
Hide file tree
Showing 18 changed files with 64 additions and 16 deletions.
5 changes: 5 additions & 0 deletions CMakeLists.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Makefile

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions build_autogenerated.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions config.m4

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions config.w32

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions gRPC-Core.podspec

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions grpc.gemspec

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions grpc.gyp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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
1 change: 1 addition & 0 deletions package.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions src/core/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ grpc_cc_library(
grpc_cc_library(
name = "event_engine_common",
srcs = [
"lib/event_engine/event_engine.cc",
"lib/event_engine/resolved_address.cc",
"lib/event_engine/slice.cc",
"lib/event_engine/slice_buffer.cc",
Expand Down
25 changes: 25 additions & 0 deletions src/core/lib/event_engine/event_engine.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2023 The gRPC Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include <grpc/support/port_platform.h>

#include <grpc/event_engine/event_engine.h>

namespace grpc_event_engine {
namespace experimental {

constexpr EventEngine::TaskHandle EventEngine::kInvalidTaskHandle;
constexpr EventEngine::ConnectionHandle EventEngine::kInvalidConnectionHandle;

} // namespace experimental
} // namespace grpc_event_engine
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
1 change: 1 addition & 0 deletions src/python/grpcio/grpc_core_dependencies.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tools/doxygen/Doxyfile.c++.internal

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tools/doxygen/Doxyfile.core.internal

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d68dd6c

Please sign in to comment.