Convert chttp2 server connection-handling code to C++.#23104
Convert chttp2 server connection-handling code to C++.#23104markdroth merged 1 commit intogrpc:masterfrom
Conversation
| #include "test/core/util/test_config.h" | ||
|
|
||
| void test_unparsable_target(void) { | ||
| gpr_log(GPR_INFO, "test_unparsable_target()"); |
There was a problem hiding this comment.
Were the log additions in this file intentional?
There was a problem hiding this comment.
Yes. It helps figure out where the problem is when a test hangs.
That having been said, a better approach here is to just use gtest instead, since the framework handles this logging for you. So I've made that change.
src/core/lib/surface/server.cc
Outdated
| if (server->channelz_server != nullptr && l->socket_uuid != 0) { | ||
| server->channelz_server->RemoveChildListenSocket(l->socket_uuid); | ||
| for (auto& listener : server->listeners) { | ||
| auto* channelz_listen_socket_node = |
There was a problem hiding this comment.
Can we avoid auto since this isn't just being used as a pointer but is also being dereff'ed below?
src/core/lib/surface/server.cc
Outdated
| server->listeners = l; | ||
| grpc_server* server, | ||
| grpc_core::OrphanablePtr<grpc_core::ServerListenerInterface> listener) { | ||
| auto* listen_socket_node = listener->channelz_listen_socket_node(); |
There was a problem hiding this comment.
Likewise about avoiding auto here
| static grpc_error* CreateWithAcceptor(grpc_server* server, const char* name, | ||
| grpc_channel_args* args); | ||
|
|
||
| // Do not instantiate directly. Use one of the factory methods above. |
There was a problem hiding this comment.
They need to be public for MakeOrphanable<> and OrphanablePtr<>::reset(). Internally, see go/totw/134 for recommendations on this.
| Chttp2ServerListener* listener_; | ||
| grpc_pollset* accepting_pollset_; | ||
| grpc_tcp_server_acceptor* acceptor_; |
There was a problem hiding this comment.
| Chttp2ServerListener* listener_; | |
| grpc_pollset* accepting_pollset_; | |
| grpc_tcp_server_acceptor* acceptor_; | |
| Chttp2ServerListener* const listener_; | |
| grpc_pollset* const accepting_pollset_; | |
| grpc_tcp_server_acceptor* const acceptor_; |
to emphasize a lack of change in these and that they don't need a default initializer
| grpc_timer timer_; | ||
| grpc_closure on_timeout_; | ||
| grpc_closure on_receive_settings_; | ||
| grpc_pollset_set* interested_parties_; |
There was a problem hiding this comment.
| grpc_pollset_set* interested_parties_; | |
| grpc_pollset_set* const interested_parties_; |
I think on this also since the PSS is created in the constructor and just gets things add/del over time
| static void DestroyListener(grpc_server* /*server*/, void* arg, | ||
| grpc_closure* destroy_done); | ||
|
|
||
| grpc_server* server_; |
There was a problem hiding this comment.
| grpc_server* server_; | |
| grpc_server* const server_; |
|
|
||
| grpc_server* server_; | ||
| grpc_tcp_server* tcp_server_; | ||
| grpc_channel_args* args_; |
There was a problem hiding this comment.
| grpc_channel_args* args_; | |
| grpc_channel_args* const args_; |
| &self->on_receive_settings_); | ||
| grpc_channel_args_destroy(args->args); | ||
| self->Ref().release(); // Held by OnTimeout(). | ||
| GRPC_CHTTP2_REF_TRANSPORT((grpc_chttp2_transport*)transport, |
There was a problem hiding this comment.
Why a C-style cast here for a reinterpret_cast above?
There was a problem hiding this comment.
This was pre-existing code, but I've changed it to use reinterpret_cast<>.
f39c2d4 to
492889f
Compare
|
Green! |
No description provided.