From dcabe420ccc3e97a0b56c0e3542447f37e728290 Mon Sep 17 00:00:00 2001 From: Ming-Chuan Date: Thu, 28 Oct 2021 15:52:21 +0800 Subject: [PATCH] [BinderTransport] Create client channel instead of direct channel (#27790) * [BinderTransport] Create client channel instead of direct channel In this commit we create a client channel instead of direct channel. BinderConnector is added to connect subchannel when the user actually make RPC call using the channel. BindToOnDeviceServerService() is not required anymore since now the actual connection is delay until the channel is used. * Regenerate projects. --- BUILD | 5 + CMakeLists.txt | 16 +++ build_autogenerated.yaml | 32 +++++ .../grpc/binder/cpp/exampleclient/native.cc | 15 +-- .../binder/client/binder_connector.cc | 121 ++++++++++++++++++ .../binder/client/binder_connector.h | 44 +++++++ .../transport/binder/client/channel_create.cc | 98 +++++++------- .../transport/binder/client/channel_create.h | 8 -- .../binder/client/channel_create_impl.cc | 61 +++++++-- .../binder/client/channel_create_impl.h | 12 +- .../binder/client/security_policy_setting.cc | 42 ++++++ .../binder/client/security_policy_setting.h | 50 ++++++++ .../binder/end2end/binder_server_test.cc | 7 +- 13 files changed, 423 insertions(+), 88 deletions(-) create mode 100644 src/core/ext/transport/binder/client/binder_connector.cc create mode 100644 src/core/ext/transport/binder/client/binder_connector.h create mode 100644 src/core/ext/transport/binder/client/security_policy_setting.cc create mode 100644 src/core/ext/transport/binder/client/security_policy_setting.h diff --git a/BUILD b/BUILD index 05882229b13..e091b7afb72 100644 --- a/BUILD +++ b/BUILD @@ -504,11 +504,13 @@ grpc_cc_library( grpc_cc_library( name = "grpc++_binder", srcs = [ + "src/core/ext/transport/binder/client/binder_connector.cc", "src/core/ext/transport/binder/client/channel_create.cc", "src/core/ext/transport/binder/client/channel_create_impl.cc", "src/core/ext/transport/binder/client/connection_id_generator.cc", "src/core/ext/transport/binder/client/endpoint_binder_pool.cc", "src/core/ext/transport/binder/client/jni_utils.cc", + "src/core/ext/transport/binder/client/security_policy_setting.cc", "src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc", "src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc", "src/core/ext/transport/binder/server/binder_server.cc", @@ -522,11 +524,13 @@ grpc_cc_library( "src/core/ext/transport/binder/wire_format/wire_writer.cc", ], hdrs = [ + "src/core/ext/transport/binder/client/binder_connector.h", "src/core/ext/transport/binder/client/channel_create.h", "src/core/ext/transport/binder/client/channel_create_impl.h", "src/core/ext/transport/binder/client/connection_id_generator.h", "src/core/ext/transport/binder/client/endpoint_binder_pool.h", "src/core/ext/transport/binder/client/jni_utils.h", + "src/core/ext/transport/binder/client/security_policy_setting.h", "src/core/ext/transport/binder/security_policy/internal_only_security_policy.h", "src/core/ext/transport/binder/security_policy/security_policy.h", "src/core/ext/transport/binder/security_policy/untrusted_security_policy.h", @@ -566,6 +570,7 @@ grpc_cc_library( "grpc++_base", "grpc++_internals", "grpc_base", + "grpc_client_channel", "grpc_codegen", "orphanable", "slice_refcount", diff --git a/CMakeLists.txt b/CMakeLists.txt index 472e9cf1a1b..b5f76dbd948 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8320,11 +8320,13 @@ add_executable(binder_server_test ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.grpc.pb.cc ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.pb.h ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.grpc.pb.h + src/core/ext/transport/binder/client/binder_connector.cc src/core/ext/transport/binder/client/channel_create.cc src/core/ext/transport/binder/client/channel_create_impl.cc src/core/ext/transport/binder/client/connection_id_generator.cc src/core/ext/transport/binder/client/endpoint_binder_pool.cc src/core/ext/transport/binder/client/jni_utils.cc + src/core/ext/transport/binder/client/security_policy_setting.cc src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc src/core/ext/transport/binder/server/binder_server.cc @@ -8374,11 +8376,13 @@ endif() if(gRPC_BUILD_TESTS) add_executable(binder_transport_test + src/core/ext/transport/binder/client/binder_connector.cc src/core/ext/transport/binder/client/channel_create.cc src/core/ext/transport/binder/client/channel_create_impl.cc src/core/ext/transport/binder/client/connection_id_generator.cc src/core/ext/transport/binder/client/endpoint_binder_pool.cc src/core/ext/transport/binder/client/jni_utils.cc + src/core/ext/transport/binder/client/security_policy_setting.cc src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc src/core/ext/transport/binder/server/binder_server.cc @@ -10026,11 +10030,13 @@ add_executable(end2end_binder_transport_test ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.grpc.pb.cc ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.pb.h ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.grpc.pb.h + src/core/ext/transport/binder/client/binder_connector.cc src/core/ext/transport/binder/client/channel_create.cc src/core/ext/transport/binder/client/channel_create_impl.cc src/core/ext/transport/binder/client/connection_id_generator.cc src/core/ext/transport/binder/client/endpoint_binder_pool.cc src/core/ext/transport/binder/client/jni_utils.cc + src/core/ext/transport/binder/client/security_policy_setting.cc src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc src/core/ext/transport/binder/server/binder_server.cc @@ -10135,11 +10141,13 @@ endif() if(gRPC_BUILD_TESTS) add_executable(endpoint_binder_pool_test + src/core/ext/transport/binder/client/binder_connector.cc src/core/ext/transport/binder/client/channel_create.cc src/core/ext/transport/binder/client/channel_create_impl.cc src/core/ext/transport/binder/client/connection_id_generator.cc src/core/ext/transport/binder/client/endpoint_binder_pool.cc src/core/ext/transport/binder/client/jni_utils.cc + src/core/ext/transport/binder/client/security_policy_setting.cc src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc src/core/ext/transport/binder/server/binder_server.cc @@ -10552,11 +10560,13 @@ endif() if(gRPC_BUILD_TESTS) add_executable(fake_binder_test + src/core/ext/transport/binder/client/binder_connector.cc src/core/ext/transport/binder/client/channel_create.cc src/core/ext/transport/binder/client/channel_create_impl.cc src/core/ext/transport/binder/client/connection_id_generator.cc src/core/ext/transport/binder/client/endpoint_binder_pool.cc src/core/ext/transport/binder/client/jni_utils.cc + src/core/ext/transport/binder/client/security_policy_setting.cc src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc src/core/ext/transport/binder/server/binder_server.cc @@ -16187,11 +16197,13 @@ endif() if(gRPC_BUILD_TESTS) add_executable(transport_stream_receiver_test + src/core/ext/transport/binder/client/binder_connector.cc src/core/ext/transport/binder/client/channel_create.cc src/core/ext/transport/binder/client/channel_create_impl.cc src/core/ext/transport/binder/client/connection_id_generator.cc src/core/ext/transport/binder/client/endpoint_binder_pool.cc src/core/ext/transport/binder/client/jni_utils.cc + src/core/ext/transport/binder/client/security_policy_setting.cc src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc src/core/ext/transport/binder/server/binder_server.cc @@ -16503,11 +16515,13 @@ endif() if(gRPC_BUILD_TESTS) add_executable(wire_reader_test + src/core/ext/transport/binder/client/binder_connector.cc src/core/ext/transport/binder/client/channel_create.cc src/core/ext/transport/binder/client/channel_create_impl.cc src/core/ext/transport/binder/client/connection_id_generator.cc src/core/ext/transport/binder/client/endpoint_binder_pool.cc src/core/ext/transport/binder/client/jni_utils.cc + src/core/ext/transport/binder/client/security_policy_setting.cc src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc src/core/ext/transport/binder/server/binder_server.cc @@ -16603,11 +16617,13 @@ endif() if(gRPC_BUILD_TESTS) add_executable(wire_writer_test + src/core/ext/transport/binder/client/binder_connector.cc src/core/ext/transport/binder/client/channel_create.cc src/core/ext/transport/binder/client/channel_create_impl.cc src/core/ext/transport/binder/client/connection_id_generator.cc src/core/ext/transport/binder/client/endpoint_binder_pool.cc src/core/ext/transport/binder/client/jni_utils.cc + src/core/ext/transport/binder/client/security_policy_setting.cc src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc src/core/ext/transport/binder/server/binder_server.cc diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index e32068b7118..241ef2a9eb9 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -4619,11 +4619,13 @@ targets: build: test language: c++ headers: + - src/core/ext/transport/binder/client/binder_connector.h - src/core/ext/transport/binder/client/channel_create.h - src/core/ext/transport/binder/client/channel_create_impl.h - src/core/ext/transport/binder/client/connection_id_generator.h - src/core/ext/transport/binder/client/endpoint_binder_pool.h - src/core/ext/transport/binder/client/jni_utils.h + - src/core/ext/transport/binder/client/security_policy_setting.h - src/core/ext/transport/binder/security_policy/internal_only_security_policy.h - src/core/ext/transport/binder/security_policy/security_policy.h - src/core/ext/transport/binder/security_policy/untrusted_security_policy.h @@ -4646,11 +4648,13 @@ targets: - src/proto/grpc/testing/echo.proto - src/proto/grpc/testing/echo_messages.proto - src/proto/grpc/testing/simple_messages.proto + - src/core/ext/transport/binder/client/binder_connector.cc - src/core/ext/transport/binder/client/channel_create.cc - src/core/ext/transport/binder/client/channel_create_impl.cc - src/core/ext/transport/binder/client/connection_id_generator.cc - src/core/ext/transport/binder/client/endpoint_binder_pool.cc - src/core/ext/transport/binder/client/jni_utils.cc + - src/core/ext/transport/binder/client/security_policy_setting.cc - src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc - src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc - src/core/ext/transport/binder/server/binder_server.cc @@ -4673,11 +4677,13 @@ targets: build: test language: c++ headers: + - src/core/ext/transport/binder/client/binder_connector.h - src/core/ext/transport/binder/client/channel_create.h - src/core/ext/transport/binder/client/channel_create_impl.h - src/core/ext/transport/binder/client/connection_id_generator.h - src/core/ext/transport/binder/client/endpoint_binder_pool.h - src/core/ext/transport/binder/client/jni_utils.h + - src/core/ext/transport/binder/client/security_policy_setting.h - src/core/ext/transport/binder/security_policy/internal_only_security_policy.h - src/core/ext/transport/binder/security_policy/security_policy.h - src/core/ext/transport/binder/security_policy/untrusted_security_policy.h @@ -4707,11 +4713,13 @@ targets: - src/cpp/thread_manager/thread_manager.h - test/core/transport/binder/mock_objects.h src: + - src/core/ext/transport/binder/client/binder_connector.cc - src/core/ext/transport/binder/client/channel_create.cc - src/core/ext/transport/binder/client/channel_create_impl.cc - src/core/ext/transport/binder/client/connection_id_generator.cc - src/core/ext/transport/binder/client/endpoint_binder_pool.cc - src/core/ext/transport/binder/client/jni_utils.cc + - src/core/ext/transport/binder/client/security_policy_setting.cc - src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc - src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc - src/core/ext/transport/binder/server/binder_server.cc @@ -5456,11 +5464,13 @@ targets: build: test language: c++ headers: + - src/core/ext/transport/binder/client/binder_connector.h - src/core/ext/transport/binder/client/channel_create.h - src/core/ext/transport/binder/client/channel_create_impl.h - src/core/ext/transport/binder/client/connection_id_generator.h - src/core/ext/transport/binder/client/endpoint_binder_pool.h - src/core/ext/transport/binder/client/jni_utils.h + - src/core/ext/transport/binder/client/security_policy_setting.h - src/core/ext/transport/binder/security_policy/internal_only_security_policy.h - src/core/ext/transport/binder/security_policy/security_policy.h - src/core/ext/transport/binder/security_policy/untrusted_security_policy.h @@ -5484,11 +5494,13 @@ targets: - src/proto/grpc/testing/echo.proto - src/proto/grpc/testing/echo_messages.proto - src/proto/grpc/testing/simple_messages.proto + - src/core/ext/transport/binder/client/binder_connector.cc - src/core/ext/transport/binder/client/channel_create.cc - src/core/ext/transport/binder/client/channel_create_impl.cc - src/core/ext/transport/binder/client/connection_id_generator.cc - src/core/ext/transport/binder/client/endpoint_binder_pool.cc - src/core/ext/transport/binder/client/jni_utils.cc + - src/core/ext/transport/binder/client/security_policy_setting.cc - src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc - src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc - src/core/ext/transport/binder/server/binder_server.cc @@ -5531,11 +5543,13 @@ targets: build: test language: c++ headers: + - src/core/ext/transport/binder/client/binder_connector.h - src/core/ext/transport/binder/client/channel_create.h - src/core/ext/transport/binder/client/channel_create_impl.h - src/core/ext/transport/binder/client/connection_id_generator.h - src/core/ext/transport/binder/client/endpoint_binder_pool.h - src/core/ext/transport/binder/client/jni_utils.h + - src/core/ext/transport/binder/client/security_policy_setting.h - src/core/ext/transport/binder/security_policy/internal_only_security_policy.h - src/core/ext/transport/binder/security_policy/security_policy.h - src/core/ext/transport/binder/security_policy/untrusted_security_policy.h @@ -5565,11 +5579,13 @@ targets: - src/cpp/thread_manager/thread_manager.h - test/core/transport/binder/mock_objects.h src: + - src/core/ext/transport/binder/client/binder_connector.cc - src/core/ext/transport/binder/client/channel_create.cc - src/core/ext/transport/binder/client/channel_create_impl.cc - src/core/ext/transport/binder/client/connection_id_generator.cc - src/core/ext/transport/binder/client/endpoint_binder_pool.cc - src/core/ext/transport/binder/client/jni_utils.cc + - src/core/ext/transport/binder/client/security_policy_setting.cc - src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc - src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc - src/core/ext/transport/binder/server/binder_server.cc @@ -5763,11 +5779,13 @@ targets: build: test language: c++ headers: + - src/core/ext/transport/binder/client/binder_connector.h - src/core/ext/transport/binder/client/channel_create.h - src/core/ext/transport/binder/client/channel_create_impl.h - src/core/ext/transport/binder/client/connection_id_generator.h - src/core/ext/transport/binder/client/endpoint_binder_pool.h - src/core/ext/transport/binder/client/jni_utils.h + - src/core/ext/transport/binder/client/security_policy_setting.h - src/core/ext/transport/binder/security_policy/internal_only_security_policy.h - src/core/ext/transport/binder/security_policy/security_policy.h - src/core/ext/transport/binder/security_policy/untrusted_security_policy.h @@ -5797,11 +5815,13 @@ targets: - src/cpp/thread_manager/thread_manager.h - test/core/transport/binder/end2end/fake_binder.h src: + - src/core/ext/transport/binder/client/binder_connector.cc - src/core/ext/transport/binder/client/channel_create.cc - src/core/ext/transport/binder/client/channel_create_impl.cc - src/core/ext/transport/binder/client/connection_id_generator.cc - src/core/ext/transport/binder/client/endpoint_binder_pool.cc - src/core/ext/transport/binder/client/jni_utils.cc + - src/core/ext/transport/binder/client/security_policy_setting.cc - src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc - src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc - src/core/ext/transport/binder/server/binder_server.cc @@ -8148,11 +8168,13 @@ targets: build: test language: c++ headers: + - src/core/ext/transport/binder/client/binder_connector.h - src/core/ext/transport/binder/client/channel_create.h - src/core/ext/transport/binder/client/channel_create_impl.h - src/core/ext/transport/binder/client/connection_id_generator.h - src/core/ext/transport/binder/client/endpoint_binder_pool.h - src/core/ext/transport/binder/client/jni_utils.h + - src/core/ext/transport/binder/client/security_policy_setting.h - src/core/ext/transport/binder/security_policy/internal_only_security_policy.h - src/core/ext/transport/binder/security_policy/security_policy.h - src/core/ext/transport/binder/security_policy/untrusted_security_policy.h @@ -8181,11 +8203,13 @@ targets: - src/cpp/server/thread_pool_interface.h - src/cpp/thread_manager/thread_manager.h src: + - src/core/ext/transport/binder/client/binder_connector.cc - src/core/ext/transport/binder/client/channel_create.cc - src/core/ext/transport/binder/client/channel_create_impl.cc - src/core/ext/transport/binder/client/connection_id_generator.cc - src/core/ext/transport/binder/client/endpoint_binder_pool.cc - src/core/ext/transport/binder/client/jni_utils.cc + - src/core/ext/transport/binder/client/security_policy_setting.cc - src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc - src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc - src/core/ext/transport/binder/server/binder_server.cc @@ -8340,11 +8364,13 @@ targets: build: test language: c++ headers: + - src/core/ext/transport/binder/client/binder_connector.h - src/core/ext/transport/binder/client/channel_create.h - src/core/ext/transport/binder/client/channel_create_impl.h - src/core/ext/transport/binder/client/connection_id_generator.h - src/core/ext/transport/binder/client/endpoint_binder_pool.h - src/core/ext/transport/binder/client/jni_utils.h + - src/core/ext/transport/binder/client/security_policy_setting.h - src/core/ext/transport/binder/security_policy/internal_only_security_policy.h - src/core/ext/transport/binder/security_policy/security_policy.h - src/core/ext/transport/binder/security_policy/untrusted_security_policy.h @@ -8374,11 +8400,13 @@ targets: - src/cpp/thread_manager/thread_manager.h - test/core/transport/binder/mock_objects.h src: + - src/core/ext/transport/binder/client/binder_connector.cc - src/core/ext/transport/binder/client/channel_create.cc - src/core/ext/transport/binder/client/channel_create_impl.cc - src/core/ext/transport/binder/client/connection_id_generator.cc - src/core/ext/transport/binder/client/endpoint_binder_pool.cc - src/core/ext/transport/binder/client/jni_utils.cc + - src/core/ext/transport/binder/client/security_policy_setting.cc - src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc - src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc - src/core/ext/transport/binder/server/binder_server.cc @@ -8448,11 +8476,13 @@ targets: build: test language: c++ headers: + - src/core/ext/transport/binder/client/binder_connector.h - src/core/ext/transport/binder/client/channel_create.h - src/core/ext/transport/binder/client/channel_create_impl.h - src/core/ext/transport/binder/client/connection_id_generator.h - src/core/ext/transport/binder/client/endpoint_binder_pool.h - src/core/ext/transport/binder/client/jni_utils.h + - src/core/ext/transport/binder/client/security_policy_setting.h - src/core/ext/transport/binder/security_policy/internal_only_security_policy.h - src/core/ext/transport/binder/security_policy/security_policy.h - src/core/ext/transport/binder/security_policy/untrusted_security_policy.h @@ -8482,11 +8512,13 @@ targets: - src/cpp/thread_manager/thread_manager.h - test/core/transport/binder/mock_objects.h src: + - src/core/ext/transport/binder/client/binder_connector.cc - src/core/ext/transport/binder/client/channel_create.cc - src/core/ext/transport/binder/client/channel_create_impl.cc - src/core/ext/transport/binder/client/connection_id_generator.cc - src/core/ext/transport/binder/client/endpoint_binder_pool.cc - src/core/ext/transport/binder/client/jni_utils.cc + - src/core/ext/transport/binder/client/security_policy_setting.cc - src/core/ext/transport/binder/security_policy/internal_only_security_policy.cc - src/core/ext/transport/binder/security_policy/untrusted_security_policy.cc - src/core/ext/transport/binder/server/binder_server.cc diff --git a/examples/android/binder/java/io/grpc/binder/cpp/exampleclient/native.cc b/examples/android/binder/java/io/grpc/binder/cpp/exampleclient/native.cc index effdf2e2ea5..435e541ee9d 100644 --- a/examples/android/binder/java/io/grpc/binder/cpp/exampleclient/native.cc +++ b/examples/android/binder/java/io/grpc/binder/cpp/exampleclient/native.cc @@ -25,20 +25,17 @@ extern "C" JNIEXPORT jstring JNICALL Java_io_grpc_binder_cpp_exampleclient_ButtonPressHandler_native_1entry( JNIEnv* env, jobject /*this*/, jobject application) { static bool first = true; - __android_log_print(ANDROID_LOG_INFO, "DemoClient", "Line number %d", - __LINE__); + static std::shared_ptr channel; if (first) { first = false; - grpc::experimental::BindToOnDeviceServerService( - env, application, "io.grpc.binder.cpp.exampleserver", - "io.grpc.binder.cpp.exampleserver.ExportedEndpointService"); - return env->NewStringUTF("Clicked 1 time"); - } else { // TODO(mingcl): Use same signature security after it become available - auto channel = grpc::experimental::CreateBinderChannel( - env, application, "", "", + channel = grpc::experimental::CreateBinderChannel( + env, application, "io.grpc.binder.cpp.exampleserver", + "io.grpc.binder.cpp.exampleserver.ExportedEndpointService", std::make_shared< grpc::experimental::binder::UntrustedSecurityPolicy>()); + return env->NewStringUTF("Clicked 1 time"); + } else { auto stub = helloworld::Greeter::NewStub(channel); grpc::ClientContext context; helloworld::HelloRequest request; diff --git a/src/core/ext/transport/binder/client/binder_connector.cc b/src/core/ext/transport/binder/client/binder_connector.cc new file mode 100644 index 00000000000..f01c3f7722c --- /dev/null +++ b/src/core/ext/transport/binder/client/binder_connector.cc @@ -0,0 +1,121 @@ +// Copyright 2021 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 + +#include "src/core/ext/transport/binder/client/binder_connector.h" + +#include "src/core/lib/iomgr/port.h" + +#ifdef GRPC_HAVE_UNIX_SOCKET +#include +#endif + +#include +#include + +#include "src/core/ext/filters/client_channel/connector.h" +#include "src/core/ext/filters/client_channel/subchannel.h" +#include "src/core/ext/transport/binder/client/endpoint_binder_pool.h" +#include "src/core/ext/transport/binder/client/security_policy_setting.h" +#include "src/core/ext/transport/binder/security_policy/untrusted_security_policy.h" +#include "src/core/ext/transport/binder/transport/binder_transport.h" +#include "src/core/ext/transport/binder/wire_format/binder.h" + +namespace { + +// TODO(mingcl): Currently this does no error handling and assumes the +// connection always succeeds in reasonable amount of time. +class BinderConnector : public grpc_core::SubchannelConnector { + public: + BinderConnector() {} + ~BinderConnector() override {} + void Connect(const Args& args, Result* result, + grpc_closure* notify) override { +#ifdef GRPC_HAVE_UNIX_SOCKET + { + struct sockaddr_un* un = + reinterpret_cast(args.address->addr); + // length of identifier, including null terminator + size_t id_length = args.address->len - sizeof(un->sun_family); + // The c-style string at least will have a null terminator, and the + // connection id itself should not be empty + GPR_ASSERT(id_length >= 2); + // Make sure there is null terminator at the expected location before + // reading from it + GPR_ASSERT(un->sun_path[id_length - 1] == '\0'); + conn_id_ = un->sun_path; + } +#else + GPR_ASSERT(0); +#endif + gpr_log(GPR_ERROR, "conn_id_ = %s", conn_id_.c_str()); + + args_ = args; + GPR_ASSERT(notify_ == nullptr); + notify_ = notify; + result_ = result; + + Ref().release(); // Ref held by the following callback + + grpc_binder::GetEndpointBinderPool()->GetEndpointBinder( + conn_id_, + std::bind(&BinderConnector::OnConnected, this, std::placeholders::_1)); + } + + void OnConnected(std::unique_ptr endpoint_binder) { + GPR_ASSERT(endpoint_binder != nullptr); + grpc_transport* transport = grpc_create_binder_transport_client( + std::move(endpoint_binder), + grpc_binder::GetSecurityPolicySetting()->Get(conn_id_)); + GPR_ASSERT(transport != nullptr); + result_->channel_args = grpc_channel_args_copy(args_.channel_args); + result_->transport = transport; + + grpc_core::ExecCtx::Run(DEBUG_LOCATION, notify_, GRPC_ERROR_NONE); + + Unref(); // Was referenced in BinderConnector::Connect + } + void Shutdown(grpc_error_handle error) override { (void)error; } + + private: + Args args_; + grpc_closure* notify_ = nullptr; + Result* result_ = nullptr; + + std::string conn_id_; +}; + +} // namespace + +namespace grpc_core { + +grpc_core::RefCountedPtr +BinderClientChannelFactory::CreateSubchannel( + const grpc_resolved_address& address, const grpc_channel_args* args) { + gpr_log(GPR_ERROR, "BinderClientChannelFactory::CreateSubchannel called"); + grpc_arg default_authority_arg = grpc_channel_arg_string_create( + const_cast(GRPC_ARG_DEFAULT_AUTHORITY), + const_cast("binder.authority")); + grpc_channel_args* new_args = + grpc_channel_args_copy_and_add(args, &default_authority_arg, 1); + + grpc_core::RefCountedPtr s = + grpc_core::Subchannel::Create( + grpc_core::MakeOrphanable(), address, new_args); + + return s; +} + +} // namespace grpc_core diff --git a/src/core/ext/transport/binder/client/binder_connector.h b/src/core/ext/transport/binder/client/binder_connector.h new file mode 100644 index 00000000000..a5fd6f0f7ed --- /dev/null +++ b/src/core/ext/transport/binder/client/binder_connector.h @@ -0,0 +1,44 @@ +// Copyright 2021 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. + +#ifndef GRPC_CORE_EXT_TRANSPORT_BINDER_CLIENT_BINDER_CONNECTOR_H +#define GRPC_CORE_EXT_TRANSPORT_BINDER_CLIENT_BINDER_CONNECTOR_H + +#include + +#include +#include + +#include "absl/strings/string_view.h" +#include "absl/strings/strip.h" + +#include +#include +#include + +#include "src/core/ext/filters/client_channel/client_channel.h" +#include "src/core/ext/filters/client_channel/client_channel_factory.h" + +namespace grpc_core { + +class BinderClientChannelFactory : public grpc_core::ClientChannelFactory { + public: + grpc_core::RefCountedPtr CreateSubchannel( + const grpc_resolved_address& address, + const grpc_channel_args* args) override; +}; + +} // namespace grpc_core + +#endif // GRPC_CORE_EXT_TRANSPORT_BINDER_CLIENT_BINDER_CONNECTOR_H diff --git a/src/core/ext/transport/binder/client/channel_create.cc b/src/core/ext/transport/binder/client/channel_create.cc index c6392d93acd..7280b1dac62 100644 --- a/src/core/ext/transport/binder/client/channel_create.cc +++ b/src/core/ext/transport/binder/client/channel_create.cc @@ -43,13 +43,16 @@ #include #include +#include "src/core/ext/filters/client_channel/client_channel.h" #include "src/core/ext/transport/binder/client/channel_create_impl.h" #include "src/core/ext/transport/binder/client/connection_id_generator.h" #include "src/core/ext/transport/binder/client/endpoint_binder_pool.h" #include "src/core/ext/transport/binder/client/jni_utils.h" +#include "src/core/ext/transport/binder/client/security_policy_setting.h" #include "src/core/ext/transport/binder/transport/binder_transport.h" #include "src/core/ext/transport/binder/wire_format/binder.h" #include "src/core/ext/transport/binder/wire_format/binder_android.h" +#include "src/core/lib/channel/channel_args.h" #include "src/core/lib/surface/channel.h" #include "src/core/lib/transport/transport.h" #include "src/cpp/client/create_channel_internal.h" @@ -57,42 +60,6 @@ namespace grpc { namespace experimental { -namespace { -// TODO(mingcl): To support multiple binder transport connection at the same -// time, we will need to generate unique connection id for each connection. -// For now we just use a single connection id globally. This will be fixed after -// we drop the BindToOnDeviceServerService interface -std::string g_connection_id; -} // namespace - -void BindToOnDeviceServerService(void* jni_env_void, jobject application, - absl::string_view package_name, - absl::string_view class_name) { - // Init gRPC library first so gpr_log works - grpc::internal::GrpcLibrary init_lib; - init_lib.init(); - - JNIEnv* jni_env = static_cast(jni_env_void); - - { - GPR_ASSERT(g_connection_id == ""); - g_connection_id = grpc_binder::GetConnectionIdGenerator()->Generate( - std::string(package_name), std::string(class_name)); - GPR_ASSERT(g_connection_id != ""); - } - - // clang-format off - grpc_binder::CallStaticJavaMethod(jni_env, - "io/grpc/binder/cpp/NativeConnectionHelper", - "tryEstablishConnection", - "(Landroid/content/Context;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V", - application, std::string(package_name), std::string(class_name), g_connection_id); - // clang-format on -} - -// BindToOndeviceServerService need to be called before this, in a different -// task (due to Android API design). (Reference: -// https://stackoverflow.com/a/3055749) std::shared_ptr CreateBinderChannel( void* jni_env_void, jobject application, absl::string_view package_name, absl::string_view class_name, @@ -103,33 +70,58 @@ std::shared_ptr CreateBinderChannel( ChannelArguments()); } -// BindToOndeviceServerService need to be called before this, in a different -// task (due to Android API design). (Reference: -// https://stackoverflow.com/a/3055749) std::shared_ptr CreateCustomBinderChannel( - void*, jobject /*application*/, absl::string_view /*package_name*/, - absl::string_view /*class_name*/, + void* jni_env_void, jobject application, absl::string_view package_name, + absl::string_view class_name, std::shared_ptr security_policy, const ChannelArguments& args) { + grpc::internal::GrpcLibrary init_lib; + init_lib.init(); + + GPR_ASSERT(jni_env_void != nullptr); GPR_ASSERT(security_policy != nullptr); - std::unique_ptr endpoint_binder; - grpc_binder::GetEndpointBinderPool()->GetEndpointBinder( - g_connection_id, [&](std::unique_ptr e) { - endpoint_binder = std::move(e); - }); - // This assumes the above callback will be called immediately before - // `GetEndpointBinder` returns - GPR_ASSERT(endpoint_binder != nullptr); + std::string connection_id = grpc_binder::GetConnectionIdGenerator()->Generate( + std::string(package_name), std::string(class_name)); + // After invoking this Java method, Java code will put endpoint binder into + // `EndpointBinderPool` after the connection succeeds + // TODO(mingcl): Consider if we want to delay the connection establishment + // until SubchannelConnector start establishing connection. For now we don't + // see any benifits doing that. + // clang-format off + grpc_binder::CallStaticJavaMethod(static_cast(jni_env_void), + "io/grpc/binder/cpp/NativeConnectionHelper", + "tryEstablishConnection", + "(Landroid/content/Context;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V", + application, std::string(package_name), std::string(class_name), connection_id); + // clang-format on + + // Set server URI to a URI that contains connection id. The URI will be used + // by subchannel connector to obtain correct endpoint binder from + // `EndpointBinderPool`. grpc_channel_args channel_args; args.SetChannelArgs(&channel_args); - return CreateChannelInternal( - "", - ::grpc::internal::CreateChannelFromBinderImpl( - std::move(endpoint_binder), security_policy, &channel_args), + grpc_channel_args* new_args; + { + grpc_arg server_uri_arg = grpc_channel_arg_string_create( + const_cast(GRPC_ARG_SERVER_URI), + const_cast(("binder:" + connection_id).c_str())); + const char* to_remove[] = {GRPC_ARG_SERVER_URI}; + new_args = grpc_channel_args_copy_and_add_and_remove( + &channel_args, to_remove, 1, &server_uri_arg, 1); + } + + grpc_binder::GetSecurityPolicySetting()->Set(connection_id, security_policy); + + auto channel = CreateChannelInternal( + "", ::grpc::internal::CreateClientBinderChannelImpl(new_args), std::vector< std::unique_ptr>()); + + grpc_channel_args_destroy(new_args); + + return channel; } } // namespace experimental diff --git a/src/core/ext/transport/binder/client/channel_create.h b/src/core/ext/transport/binder/client/channel_create.h index 1a81f7a2390..3ff873a372f 100644 --- a/src/core/ext/transport/binder/client/channel_create.h +++ b/src/core/ext/transport/binder/client/channel_create.h @@ -32,14 +32,6 @@ namespace grpc { namespace experimental { -// This need be called before calling CreateBinderChannel, and the thread need -// to be free before invoking CreateBinderChannel. -// TODO(mingcl): This method will be removed after we start creating client -// channel instead of direct channel -void BindToOnDeviceServerService(void* jni_env_void, jobject application, - absl::string_view /*package_name*/, - absl::string_view /*class_name*/); - // Need to be invoked after BindToOnDeviceServerService // Create a new Channel from server package name and service class name std::shared_ptr CreateBinderChannel( diff --git a/src/core/ext/transport/binder/client/channel_create_impl.cc b/src/core/ext/transport/binder/client/channel_create_impl.cc index ab88c1b1d82..d0a01e31ff1 100644 --- a/src/core/ext/transport/binder/client/channel_create_impl.cc +++ b/src/core/ext/transport/binder/client/channel_create_impl.cc @@ -19,44 +19,83 @@ #include #include +#include "src/core/ext/transport/binder/client/binder_connector.h" #include "src/core/ext/transport/binder/transport/binder_transport.h" #include "src/core/ext/transport/binder/wire_format/binder.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/surface/api_trace.h" #include "src/core/lib/surface/channel.h" +namespace { + +grpc_core::BinderClientChannelFactory* g_factory; +gpr_once g_factory_once = GPR_ONCE_INIT; + +void FactoryInit() { g_factory = new grpc_core::BinderClientChannelFactory(); } +} // namespace + namespace grpc { namespace internal { -grpc_channel* CreateChannelFromBinderImpl( +grpc_channel* CreateDirectBinderChannelImplForTesting( std::unique_ptr endpoint_binder, - std::shared_ptr security_policy, - const grpc_channel_args* args) { + const grpc_channel_args* args, + std::shared_ptr + security_policy) { grpc_core::ExecCtx exec_ctx; - GRPC_API_TRACE("grpc_channel_create_from_binder(target=%p, args=%p)", 2, - ((void*)1234, args)); grpc_transport* transport = grpc_create_binder_transport_client( std::move(endpoint_binder), security_policy); - GPR_ASSERT(transport); - - // TODO(b/192207753): check binder alive and ping binder + GPR_ASSERT(transport != nullptr); - // TODO(b/192207758): Figure out if we are required to set authority here grpc_arg default_authority_arg = grpc_channel_arg_string_create( const_cast(GRPC_ARG_DEFAULT_AUTHORITY), - const_cast("test.authority")); + const_cast("binder.authority")); grpc_channel_args* final_args = grpc_channel_args_copy_and_add(args, &default_authority_arg, 1); grpc_error_handle error = GRPC_ERROR_NONE; grpc_channel* channel = grpc_channel_create( "binder_target_placeholder", final_args, GRPC_CLIENT_DIRECT_CHANNEL, transport, nullptr, 0, &error); - // TODO(mingcl): Handle error properly GPR_ASSERT(error == GRPC_ERROR_NONE); grpc_channel_args_destroy(final_args); return channel; } +grpc_channel* CreateClientBinderChannelImpl(const grpc_channel_args* args) { + grpc_core::ExecCtx exec_ctx; + + gpr_once_init(&g_factory_once, FactoryInit); + + // Set channel factory argument + grpc_arg channel_factory_arg = + grpc_core::ClientChannelFactory::CreateChannelArg(g_factory); + const char* arg_to_remove = channel_factory_arg.key; + grpc_channel_args* new_args = grpc_channel_args_copy_and_add_and_remove( + args, &arg_to_remove, 1, &channel_factory_arg, 1); + + grpc_error_handle error = GRPC_ERROR_NONE; + + grpc_channel* channel = + grpc_channel_create("binder_channel_target_placeholder", new_args, + GRPC_CLIENT_CHANNEL, nullptr, nullptr, 0, &error); + + // Clean up. + grpc_channel_args_destroy(new_args); + if (channel == nullptr) { + intptr_t integer; + grpc_status_code status = GRPC_STATUS_INTERNAL; + if (grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &integer)) { + status = static_cast(integer); + } + GRPC_ERROR_UNREF(error); + channel = grpc_lame_client_channel_create( + "binder_channel_target_placeholder", status, + "Failed to create binder channel"); + } + + return channel; +} + } // namespace internal } // namespace grpc diff --git a/src/core/ext/transport/binder/client/channel_create_impl.h b/src/core/ext/transport/binder/client/channel_create_impl.h index 94201a6ef65..5cd165e4703 100644 --- a/src/core/ext/transport/binder/client/channel_create_impl.h +++ b/src/core/ext/transport/binder/client/channel_create_impl.h @@ -24,10 +24,16 @@ namespace grpc { namespace internal { -grpc_channel* CreateChannelFromBinderImpl( +// Creates a GRPC_CLIENT_DIRECT_CHANNEL channel from endpoint binder +// At this moment this is only used for testing. +grpc_channel* CreateDirectBinderChannelImplForTesting( std::unique_ptr endpoint_binder, - std::shared_ptr security_policy, - const grpc_channel_args* args); + const grpc_channel_args* args, + std::shared_ptr + security_policy); + +// Creates a GRPC_CLIENT_CHANNEL channel +grpc_channel* CreateClientBinderChannelImpl(const grpc_channel_args* args); } // namespace internal } // namespace grpc diff --git a/src/core/ext/transport/binder/client/security_policy_setting.cc b/src/core/ext/transport/binder/client/security_policy_setting.cc new file mode 100644 index 00000000000..7d9f5974945 --- /dev/null +++ b/src/core/ext/transport/binder/client/security_policy_setting.cc @@ -0,0 +1,42 @@ +// Copyright 2021 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 + +#include "src/core/ext/transport/binder/client/security_policy_setting.h" + +namespace grpc_binder { + +void SecurityPolicySetting::Set( + absl::string_view connection_id, + std::shared_ptr + security_policy) { + grpc_core::MutexLock l(&m_); + GPR_ASSERT(security_policy_map_.count(std::string(connection_id)) == 0); + security_policy_map_[std::string(connection_id)] = security_policy; +} + +std::shared_ptr +SecurityPolicySetting::Get(absl::string_view connection_id) { + grpc_core::MutexLock l(&m_); + GPR_ASSERT(security_policy_map_.count(std::string(connection_id)) != 0); + return security_policy_map_[std::string(connection_id)]; +} + +SecurityPolicySetting* GetSecurityPolicySetting() { + static SecurityPolicySetting* s = new SecurityPolicySetting(); + return s; +} + +} // namespace grpc_binder diff --git a/src/core/ext/transport/binder/client/security_policy_setting.h b/src/core/ext/transport/binder/client/security_policy_setting.h new file mode 100644 index 00000000000..a35975de21c --- /dev/null +++ b/src/core/ext/transport/binder/client/security_policy_setting.h @@ -0,0 +1,50 @@ +// Copyright 2021 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. + +#ifndef GRPC_CORE_EXT_TRANSPORT_BINDER_CLIENT_SECURITY_POLICY_SETTING_H +#define GRPC_CORE_EXT_TRANSPORT_BINDER_CLIENT_SECURITY_POLICY_SETTING_H + +#include + +#include "absl/container/flat_hash_map.h" +#include "absl/strings/string_view.h" + +#include "src/core/ext/transport/binder/security_policy/security_policy.h" +#include "src/core/lib/gprpp/sync.h" + +namespace grpc_binder { + +// A singleton class for setting security setting for each connection. This is +// required because we cannot pass security policy shared pointers around using +// gRPC arguments, we can only pass connection_id around as part of URI +class SecurityPolicySetting { + public: + void Set(absl::string_view connection_id, + std::shared_ptr + security_policy); + std::shared_ptr Get( + absl::string_view connection_id); + + private: + grpc_core::Mutex m_; + absl::flat_hash_map< + std::string, std::shared_ptr> + security_policy_map_ ABSL_GUARDED_BY(m_); +}; + +SecurityPolicySetting* GetSecurityPolicySetting(); + +} // namespace grpc_binder + +#endif // GRPC_CORE_EXT_TRANSPORT_BINDER_CLIENT_SECURITY_POLICY_SETTING_H diff --git a/test/core/transport/binder/end2end/binder_server_test.cc b/test/core/transport/binder/end2end/binder_server_test.cc index 67fc635b733..52383928951 100644 --- a/test/core/transport/binder/end2end/binder_server_test.cc +++ b/test/core/transport/binder/end2end/binder_server_test.cc @@ -74,11 +74,10 @@ std::shared_ptr CreateBinderChannel( return grpc::CreateChannelInternal( "", - grpc::internal::CreateChannelFromBinderImpl( - std::move(endpoint_binder), + grpc::internal::CreateDirectBinderChannelImplForTesting( + std::move(endpoint_binder), nullptr, std::make_shared< - grpc::experimental::binder::UntrustedSecurityPolicy>(), - nullptr), + grpc::experimental::binder::UntrustedSecurityPolicy>()), std::vector>()); }