From 071aff284f0ba700e1296688ed2b52dddfce051e Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Tue, 23 Feb 2016 18:31:27 -0800 Subject: [PATCH 01/60] Accept instance name as argument --- tools/gce/create_linux_worker.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/gce/create_linux_worker.sh b/tools/gce/create_linux_worker.sh index 2a9e77ab173..399e3ec4e43 100755 --- a/tools/gce/create_linux_worker.sh +++ b/tools/gce/create_linux_worker.sh @@ -37,7 +37,7 @@ cd $(dirname $0) CLOUD_PROJECT=grpc-testing ZONE=us-central1-a -INSTANCE_NAME=grpc-jenkins-worker1 +INSTANCE_NAME="${1:-grpc-jenkins-worker1}" gcloud compute instances create $INSTANCE_NAME \ --project="$CLOUD_PROJECT" \ From 02eda41d215d2e49f5626e0ecd2c2d16e9c9a1f2 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 25 Feb 2016 19:43:57 -0800 Subject: [PATCH 02/60] Do stream operations in a more idiomatic way: Write, WritesDone, Read until fail, Finish --- test/cpp/qps/driver.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index 1c7fdf87960..18a9ae4d256 100644 --- a/test/cpp/qps/driver.cc +++ b/test/cpp/qps/driver.cc @@ -351,9 +351,11 @@ std::unique_ptr RunScenario( gpr_log(GPR_INFO, "Finishing"); for (auto server = &servers[0]; server != &servers[num_servers]; server++) { GPR_ASSERT(server->stream->Write(server_mark)); + GPR_ASSERT(server->stream->WritesDone()); } for (auto client = &clients[0]; client != &clients[num_clients]; client++) { GPR_ASSERT(client->stream->Write(client_mark)); + GPR_ASSERT(client->stream->WritesDone()); } for (auto server = &servers[0]; server != &servers[num_servers]; server++) { GPR_ASSERT(server->stream->Read(&server_status)); @@ -361,6 +363,7 @@ std::unique_ptr RunScenario( result->server_resources.emplace_back( stats.time_elapsed(), stats.time_user(), stats.time_system(), server_status.cores()); + GPR_ASSERT(!server->stream->Read(&server_status)); } for (auto client = &clients[0]; client != &clients[num_clients]; client++) { GPR_ASSERT(client->stream->Read(&client_status)); @@ -368,14 +371,13 @@ std::unique_ptr RunScenario( result->latencies.MergeProto(stats.latencies()); result->client_resources.emplace_back( stats.time_elapsed(), stats.time_user(), stats.time_system(), -1); + GPR_ASSERT(!client->stream->Read(&client_status)); } for (auto client = &clients[0]; client != &clients[num_clients]; client++) { - GPR_ASSERT(client->stream->WritesDone()); GPR_ASSERT(client->stream->Finish().ok()); } for (auto server = &servers[0]; server != &servers[num_servers]; server++) { - GPR_ASSERT(server->stream->WritesDone()); GPR_ASSERT(server->stream->Finish().ok()); } delete[] clients; From e7042b5ddf4f72dbba79e53621f28ba7d3cd9df4 Mon Sep 17 00:00:00 2001 From: vjpai Date: Thu, 25 Feb 2016 20:28:05 -0800 Subject: [PATCH 03/60] Simplify Mark processing code --- test/cpp/qps/client.h | 35 +++++++++-------------------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/test/cpp/qps/client.h b/test/cpp/qps/client.h index 2dc83f0f292..6a822e35c78 100644 --- a/test/cpp/qps/client.h +++ b/test/cpp/qps/client.h @@ -123,15 +123,13 @@ class Client { if (reset) { Histogram* to_merge = new Histogram[threads_.size()]; for (size_t i = 0; i < threads_.size(); i++) { - threads_[i]->BeginSwap(&to_merge[i]); - } - std::unique_ptr timer(new UsageTimer); - timer_.swap(timer); - for (size_t i = 0; i < threads_.size(); i++) { - threads_[i]->EndSwap(); + threads_[i]->Swap(&to_merge[i]); latencies.Merge(to_merge[i]); } delete[] to_merge; + + std::unique_ptr timer(new UsageTimer); + timer_.swap(timer); timer_result = timer->Mark(); } else { // merge snapshots of each thread histogram @@ -227,7 +225,6 @@ class Client { public: Thread(Client* client, size_t idx) : done_(false), - new_stats_(nullptr), client_(client), idx_(idx), impl_(&Thread::ThreadFunc, this) {} @@ -240,16 +237,9 @@ class Client { impl_.join(); } - void BeginSwap(Histogram* n) { + void Swap(Histogram* n) { std::lock_guard g(mu_); - new_stats_ = n; - } - - void EndSwap() { - std::unique_lock g(mu_); - while (new_stats_ != nullptr) { - cv_.wait(g); - }; + n->Swap(&histogram_); } void MergeStatsInto(Histogram* hist) { @@ -263,10 +253,11 @@ class Client { void ThreadFunc() { for (;;) { + // lock since the thread should only be doing one thing at a time + std::lock_guard g(mu_); // run the loop body const bool thread_still_ok = client_->ThreadFunc(&histogram_, idx_); - // lock, see if we're done - std::lock_guard g(mu_); + // see if we're done if (!thread_still_ok) { gpr_log(GPR_ERROR, "Finishing client thread due to RPC error"); done_ = true; @@ -274,19 +265,11 @@ class Client { if (done_) { return; } - // check if we're resetting stats, swap out the histogram if so - if (new_stats_) { - new_stats_->Swap(&histogram_); - new_stats_ = nullptr; - cv_.notify_one(); - } } } std::mutex mu_; - std::condition_variable cv_; bool done_; - Histogram* new_stats_; Histogram histogram_; Client* client_; const size_t idx_; From 762ed4368afa513ef468eac4cf16e4c91ff9b61a Mon Sep 17 00:00:00 2001 From: Chris Bacon Date: Fri, 26 Feb 2016 12:44:10 +0000 Subject: [PATCH 04/60] Support for coreclr Use .ToLowerInvariant, which is supported by coreclr --- src/csharp/Grpc.Core/Metadata.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/csharp/Grpc.Core/Metadata.cs b/src/csharp/Grpc.Core/Metadata.cs index aa22f840d6c..52cef96f405 100644 --- a/src/csharp/Grpc.Core/Metadata.cs +++ b/src/csharp/Grpc.Core/Metadata.cs @@ -323,7 +323,7 @@ namespace Grpc.Core private static string NormalizeKey(string key) { - var normalized = GrpcPreconditions.CheckNotNull(key, "key").ToLower(CultureInfo.InvariantCulture); + var normalized = GrpcPreconditions.CheckNotNull(key, "key").ToLowerInvariant(); GrpcPreconditions.CheckArgument(ValidKeyRegex.IsMatch(normalized), "Metadata entry key not valid. Keys can only contain lowercase alphanumeric characters, underscores and hyphens."); return normalized; From 60a7e3336df1554891bae57b3f8d6e7304f6e039 Mon Sep 17 00:00:00 2001 From: vjpai Date: Fri, 26 Feb 2016 10:30:17 -0800 Subject: [PATCH 05/60] Finish clients fully before finishing servers --- test/cpp/qps/driver.cc | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index 18a9ae4d256..bc8780f74d5 100644 --- a/test/cpp/qps/driver.cc +++ b/test/cpp/qps/driver.cc @@ -348,23 +348,11 @@ std::unique_ptr RunScenario( std::unique_ptr result(new ScenarioResult); result->client_config = result_client_config; result->server_config = result_server_config; - gpr_log(GPR_INFO, "Finishing"); - for (auto server = &servers[0]; server != &servers[num_servers]; server++) { - GPR_ASSERT(server->stream->Write(server_mark)); - GPR_ASSERT(server->stream->WritesDone()); - } + gpr_log(GPR_INFO, "Finishing clients"); for (auto client = &clients[0]; client != &clients[num_clients]; client++) { GPR_ASSERT(client->stream->Write(client_mark)); GPR_ASSERT(client->stream->WritesDone()); } - for (auto server = &servers[0]; server != &servers[num_servers]; server++) { - GPR_ASSERT(server->stream->Read(&server_status)); - const auto& stats = server_status.stats(); - result->server_resources.emplace_back( - stats.time_elapsed(), stats.time_user(), stats.time_system(), - server_status.cores()); - GPR_ASSERT(!server->stream->Read(&server_status)); - } for (auto client = &clients[0]; client != &clients[num_clients]; client++) { GPR_ASSERT(client->stream->Read(&client_status)); const auto& stats = client_status.stats(); @@ -373,14 +361,28 @@ std::unique_ptr RunScenario( stats.time_elapsed(), stats.time_user(), stats.time_system(), -1); GPR_ASSERT(!client->stream->Read(&client_status)); } - for (auto client = &clients[0]; client != &clients[num_clients]; client++) { GPR_ASSERT(client->stream->Finish().ok()); } + delete[] clients; + + gpr_log(GPR_INFO, "Finishing servers"); + for (auto server = &servers[0]; server != &servers[num_servers]; server++) { + GPR_ASSERT(server->stream->Write(server_mark)); + GPR_ASSERT(server->stream->WritesDone()); + } + for (auto server = &servers[0]; server != &servers[num_servers]; server++) { + GPR_ASSERT(server->stream->Read(&server_status)); + const auto& stats = server_status.stats(); + result->server_resources.emplace_back( + stats.time_elapsed(), stats.time_user(), stats.time_system(), + server_status.cores()); + GPR_ASSERT(!server->stream->Read(&server_status)); + } for (auto server = &servers[0]; server != &servers[num_servers]; server++) { GPR_ASSERT(server->stream->Finish().ok()); } - delete[] clients; + delete[] servers; return result; } From c47c9497fa6518205a598c2bf74517df09541fed Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Fri, 26 Feb 2016 14:39:59 -0800 Subject: [PATCH 06/60] clang-format --- test/cpp/qps/client.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cpp/qps/client.h b/test/cpp/qps/client.h index 6a822e35c78..92e77eed9b2 100644 --- a/test/cpp/qps/client.h +++ b/test/cpp/qps/client.h @@ -253,7 +253,7 @@ class Client { void ThreadFunc() { for (;;) { - // lock since the thread should only be doing one thing at a time + // lock since the thread should only be doing one thing at a time std::lock_guard g(mu_); // run the loop body const bool thread_still_ok = client_->ThreadFunc(&histogram_, idx_); From ebf1fcff7cf3ba0ad515fccab5dcf0ab75c5bd31 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 13:53:49 -0800 Subject: [PATCH 07/60] Update base INSTALL to markdown and remove outdated content --- CONTRIBUTING.md | 2 +- INSTALL | 225 ------------------------------ INSTALL.md | 45 ++++++ README.md | 2 +- examples/cpp/README.md | 2 +- examples/cpp/cpptutorial.md | 2 +- examples/cpp/helloworld/README.md | 2 +- 7 files changed, 50 insertions(+), 230 deletions(-) delete mode 100644 INSTALL create mode 100644 INSTALL.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9423c46547a..03d2e276a82 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,7 +13,7 @@ In order to protect both you and ourselves, you will need to sign the ### Technical requirements You will need several tools to work with this repository. In addition to all of -the packages described in the [INSTALL](INSTALL) file, you will also need +the packages described in the [INSTALL](INSTALL.md) file, you will also need python, and the mako template renderer. To install the latter, using pip, one should simply be able to do `pip install mako`. diff --git a/INSTALL b/INSTALL deleted file mode 100644 index a0df57dcd35..00000000000 --- a/INSTALL +++ /dev/null @@ -1,225 +0,0 @@ -These instructions only cover building grpc C and C++ libraries under -typical unix systems. If you need more information, please try grpc's -wiki pages: - - https://github.com/google/grpc/wiki - - -************************* -* If you are in a hurry * -************************* - -On Linux (Debian): - - Note: you will need to add the Debian 'jessie-backports' distribution to your sources - file first. - - Add the following line to your `/etc/apt/sources.list` file: - - deb http://http.debian.net/debian jessie-backports main - - Install the gRPC library: - - $ [sudo] apt-get install libgrpc-dev - -OR - - $ git clone https://github.com/grpc/grpc.git - $ cd grpc - $ git submodule update --init - $ make - $ [sudo] make install - -You don't need anything else than GNU Make, gcc and autotools. Under a Debian -or Ubuntu system, this should boil down to the following packages: - - $ [sudo] apt-get install build-essential autoconf libtool - -Building the python wrapper requires the following: - - $ [sudo] apt-get install python-all-dev python-virtualenv - -If you want to install in a different directory than the default /usr/lib, you can -override it on the command line: - - $ [sudo] make install prefix=/opt - - -******************************* -* More detailled instructions * -******************************* - -Setting up dependencies -======================= - -Dependencies to compile the libraries -------------------------------------- - -grpc libraries have few external dependencies. If you need to compile and -install them, they are present in the third_party directory if you have -cloned the github repository recursively. If you didn't clone recursively, -you can still get them later by running the following command: - - $ git submodule update --init - -Note that the Makefile makes it much easier for you to compile from sources -if you were to clone recursively our git repository: it will automatically -compile zlib and OpenSSL, which are core requirements for grpc. Note this -creates grpc libraries that will have zlib and OpenSSL built-in inside of them, -which significantly increases the libraries' size. - -In order to decrease that size, you can manually install zlib and OpenSSL on -your system, so that the Makefile can use them instead. - -Under a Debian or Ubuntu system, one can acquire the development package -for zlib this way: - - # apt-get install zlib1g-dev - -To the best of our knowledge, no distribution has an OpenSSL package that -supports ALPN yet, so you would still have to depend on installing from source -for that particular dependency if you want to reduce the libraries' size. - -The recommended version of OpenSSL that provides ALPN support is available -at this URL: - - https://www.openssl.org/source/openssl-1.0.2.tar.gz - - -Dependencies to compile and run the tests ------------------------------------------ - -Compiling and running grpc plain-C tests dont't require any more dependency. - - -Compiling and running grpc C++ tests depend on protobuf 3.0.0, gtest and -gflags. Although gflags is provided in third_party, you will need to manually -install that dependency on your system to run these tests. - -Under a Debian or Ubuntu system, you can install the gtests and gflags packages -using apt-get: - - # apt-get install libgflags-dev libgtest-dev - -However, protobuf 3.0.0 isn't in a debian package yet, but the Makefile will -automatically try and compile the one present in third_party if you cloned the -repository recursively, and that it detects your system is lacking it. - -Compiling and installing protobuf 3.0.0 requires a few more dependencies in -itself, notably the autoconf suite. If you have apt-get, you can install -these dependencies this way: - - # apt-get install autoconf libtool - -If you want to run the tests using one of the sanitized configurations, you -will need clang and its instrumented libc++: - - # apt-get install clang libc++-dev - -Mac-specific notes: -------------------- - -For a Mac system, git is not available by default. You will first need to -install Xcode from the Mac AppStore and then run the following command from a -terminal: - - $ sudo xcode-select --install - -You should also install "port" following the instructions at -https://www.macports.org . This will reside in /opt/local/bin/port for -most Mac installations. Do the "git submodule" command listed above. - -Then execute the following for all the needed build dependencies - - $ sudo /opt/local/bin/port install autoconf automake libtool gflags cmake - $ mkdir ~/gtest-svn - $ svn checkout http://googletest.googlecode.com/svn/trunk/ gtest-svn - $ mkdir mybuild - $ cd mybuild - $ cmake ../gtest-svn - $ make - $ make gtest.a gtest_main.a - $ sudo cp libgtest.a libgtest_main.a /opt/local/lib - $ sudo mkdir /opt/local/include/gtest - $ sudo cp -pr ../gtest-svn/include/gtest /opt/local/include/gtest - -We will also need to make openssl and install it appropriately - - $ cd - $ cd third_party/openssl - $ ./config - $ sudo make install - $ cd ../../ - -If you are going to make changes and need to regenerate the projects file, -you will need to install certain modules for python. - - $ sudo easy_install simplejson mako - -Mingw-specific notes: ---------------------- - -While gRPC compiles properly under mingw, some more preparation work is needed. -The recommendation is to use msys2. The installation instructions are available -at that address: http://msys2.github.io/ - -Once this is installed, make sure you are using the following: MinGW-w64 Win64. -You'll be required to install a few more packages: - - $ pacman -S make mingw-w64-x86_64-gcc mingw-w64-x86_64-zlib autoconf automake libtool - -Please also install OpenSSL from that website: - - http://slproweb.com/products/Win32OpenSSL.html - -The package Win64 OpenSSL v1.0.2a should do. At that point you should be able -to compile gRPC with the following: - - $ export LDFLAGS="-L/mingw64/lib -L/c/OpenSSL-Win64" - $ export CPPFLAGS="-I/mingw64/include -I/c/OpenSSL-Win64/include" - $ make - -A word on OpenSSL ------------------ - -Secure HTTP2 requires the TLS extension ALPN (see rfc 7301 and -http://http2.github.io/http2-spec/ section 3.3). Our HTTP2 implementation -relies on OpenSSL's implementation. OpenSSL 1.0.2 is the first released version -of OpenSSL that has ALPN support, and this explains our dependency on it. - -Note that the Makefile supports compiling only the unsecure elements of grpc, -and if you do not have OpenSSL and do not want it, you can still proceed -with installing only the elements you require. However, we strongly recommend -the use of encryption for all network traffic, and discourage the use of grpc -without TLS. - - -Compiling -========= - -If you have all the dependencies mentioned above, you should simply be able -to go ahead and run "make" to compile grpc's C and C++ libraries: - - $ make - - -Testing -======= - -To build and run the tests, you can run the command: - - $ make test - -If you want to be able to run them in parallel, and get better output, you can -also use the python tool we have written: - - $ ./tools/run_tests/run_tests.py - - -Installing -========== - -Once everything is compiled, you should be able to install grpc C and C++ -libraries and headers: - - # make install diff --git a/INSTALL.md b/INSTALL.md new file mode 100644 index 00000000000..aa809f23c8f --- /dev/null +++ b/INSTALL.md @@ -0,0 +1,45 @@ +#If you are in a hurry + +For language-specific installation instructions, please refer to these +documents + + * [C++](examples/cpp) + * [C#](src/csharp): NuGet package `Grpc` + * [Go](https://github.com/grpc/grpc-go): `go get google.golang.org/grpc` + * [Java](https://github.com/grpc/grpc-java) + * [Node](src/node): `npm install grpc` + * [Objective-C](src/objective-c) + * [PHP](src/php): `pecl install grpc-beta` + * [Python](src/python/grpcio): `pip install grpcio` + * [Ruby](src/ruby): `gem install grpc` + + +#Pre-requisites + +##Linux + +```sh + $ [sudo] apt-get install build-essential autoconf libtool +``` + +##Mac OSX + +For a Mac system, git is not available by default. You will first need to +install Xcode from the Mac AppStore and then run the following command from a +terminal: + +```sh + $ [sudo] xcode-select --install +``` + +#Build from Source + +This is for compiling just the gRPC C Core library. + +```sh + $ git clone https://github.com/grpc/grpc.git + $ cd grpc + $ git submodule update --init + $ make + $ [sudo] make install +``` diff --git a/README.md b/README.md index 033e09b91b7..abb4905392c 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ You can find more detailed documentation and examples in the [doc](doc) and [exa #Installation -See [grpc/INSTALL](INSTALL) for installation instructions for various platforms. +See [INSTALL](INSTALL.md) for installation instructions for various platforms. #Repository Structure & Status diff --git a/examples/cpp/README.md b/examples/cpp/README.md index 85c495099b7..979ba55dbb5 100644 --- a/examples/cpp/README.md +++ b/examples/cpp/README.md @@ -2,7 +2,7 @@ ## Installation -To install gRPC on your system, follow the instructions [here](../../INSTALL). +To install gRPC on your system, follow the instructions [here](../../INSTALL.md). ## Hello C++ gRPC! diff --git a/examples/cpp/cpptutorial.md b/examples/cpp/cpptutorial.md index cd1cddb1115..ef9ca99c0fe 100644 --- a/examples/cpp/cpptutorial.md +++ b/examples/cpp/cpptutorial.md @@ -91,7 +91,7 @@ message Point { Next we need to generate the gRPC client and server interfaces from our .proto service definition. We do this using the protocol buffer compiler `protoc` with a special gRPC C++ plugin. -For simplicity, we've provided a [makefile](route_guide/Makefile) that runs `protoc` for you with the appropriate plugin, input, and output (if you want to run this yourself, make sure you've installed protoc and followed the gRPC code [installation instructions](../../INSTALL) first): +For simplicity, we've provided a [makefile](route_guide/Makefile) that runs `protoc` for you with the appropriate plugin, input, and output (if you want to run this yourself, make sure you've installed protoc and followed the gRPC code [installation instructions](../../INSTALL.md) first): ```shell $ make route_guide.grpc.pb.cc route_guide.pb.cc diff --git a/examples/cpp/helloworld/README.md b/examples/cpp/helloworld/README.md index 90f3d6b1475..8e11f7cdf3e 100644 --- a/examples/cpp/helloworld/README.md +++ b/examples/cpp/helloworld/README.md @@ -2,7 +2,7 @@ ### Install gRPC Make sure you have installed gRPC on your system. Follow the instructions here: -[https://github.com/grpc/grpc/blob/master/INSTALL](../../../INSTALL). +[https://github.com/grpc/grpc/blob/master/INSTALL](../../../INSTALL.md). ### Get the tutorial source code From 1279bd7934e3c5522b0b6ee6dffcb5b944bf752d Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 14:49:03 -0800 Subject: [PATCH 08/60] bit of text change --- INSTALL.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index aa809f23c8f..d9411db021d 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -1,7 +1,7 @@ #If you are in a hurry -For language-specific installation instructions, please refer to these -documents +For language-specific installation instructions for gRPC runtime, please +refer to these documents * [C++](examples/cpp) * [C#](src/csharp): NuGet package `Grpc` @@ -34,7 +34,8 @@ terminal: #Build from Source -This is for compiling just the gRPC C Core library. +For developers who are interested to contribute, here is how to compile the +gRPC C Core library. ```sh $ git clone https://github.com/grpc/grpc.git From f2f7d57c01037993220132817a2897777288f3ca Mon Sep 17 00:00:00 2001 From: Dan Born Date: Thu, 3 Mar 2016 17:26:12 -0800 Subject: [PATCH 09/60] Allow use of alternative credential types. --- src/proto/grpc/testing/echo_messages.proto | 1 + test/cpp/end2end/end2end_test.cc | 33 ++++++++++++++-------- test/cpp/end2end/test_service_impl.cc | 10 ++++--- test/cpp/util/test_credentials_provider.h | 5 +++- 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/proto/grpc/testing/echo_messages.proto b/src/proto/grpc/testing/echo_messages.proto index d05a35548d1..5ce0a1fd642 100644 --- a/src/proto/grpc/testing/echo_messages.proto +++ b/src/proto/grpc/testing/echo_messages.proto @@ -42,6 +42,7 @@ message RequestParams { bool echo_peer = 7; string expected_client_identity = 8; // will force check_auth_context. bool skip_cancelled_check = 9; + string expected_transport_security_type = 10; } message EchoRequest { diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index dc2c4f6426f..8131a14ff76 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -59,6 +59,7 @@ using grpc::testing::EchoRequest; using grpc::testing::EchoResponse; +using grpc::testing::kTlsCredentialsType; using std::chrono::system_clock; namespace grpc { @@ -1194,6 +1195,8 @@ TEST_P(SecureEnd2endTest, BlockingAuthMetadataPluginAndProcessorSuccess) { request.mutable_param()->set_echo_metadata(true); request.mutable_param()->set_expected_client_identity( TestAuthMetadataProcessor::kGoodGuy); + request.mutable_param()->set_expected_transport_security_type( + GetParam().credentials_type); Status s = stub_->Echo(&context, request, &response); EXPECT_EQ(request.message(), response.message()); @@ -1301,6 +1304,8 @@ TEST_P(SecureEnd2endTest, NonBlockingAuthMetadataPluginAndProcessorSuccess) { request.mutable_param()->set_echo_metadata(true); request.mutable_param()->set_expected_client_identity( TestAuthMetadataProcessor::kGoodGuy); + request.mutable_param()->set_expected_transport_security_type( + GetParam().credentials_type); Status s = stub_->Echo(&context, request, &response); EXPECT_EQ(request.message(), response.message()); @@ -1349,25 +1354,29 @@ TEST_P(SecureEnd2endTest, ClientAuthContext) { EchoRequest request; EchoResponse response; request.set_message("Hello"); - request.mutable_param()->set_check_auth_context(true); - + request.mutable_param()->set_check_auth_context(GetParam().credentials_type == + kTlsCredentialsType); + request.mutable_param()->set_expected_transport_security_type( + GetParam().credentials_type); ClientContext context; Status s = stub_->Echo(&context, request, &response); EXPECT_EQ(response.message(), request.message()); EXPECT_TRUE(s.ok()); std::shared_ptr auth_ctx = context.auth_context(); - std::vector ssl = + std::vector tst = auth_ctx->FindPropertyValues("transport_security_type"); - EXPECT_EQ(1u, ssl.size()); - EXPECT_EQ("ssl", ToString(ssl[0])); - EXPECT_EQ("x509_subject_alternative_name", - auth_ctx->GetPeerIdentityPropertyName()); - EXPECT_EQ(3u, auth_ctx->GetPeerIdentity().size()); - EXPECT_EQ("*.test.google.fr", ToString(auth_ctx->GetPeerIdentity()[0])); - EXPECT_EQ("waterzooi.test.google.be", - ToString(auth_ctx->GetPeerIdentity()[1])); - EXPECT_EQ("*.test.youtube.com", ToString(auth_ctx->GetPeerIdentity()[2])); + EXPECT_EQ(1u, tst.size()); + EXPECT_EQ(GetParam().credentials_type, ToString(tst[0])); + if (GetParam().credentials_type == kTlsCredentialsType) { + EXPECT_EQ("x509_subject_alternative_name", + auth_ctx->GetPeerIdentityPropertyName()); + EXPECT_EQ(3u, auth_ctx->GetPeerIdentity().size()); + EXPECT_EQ("*.test.google.fr", ToString(auth_ctx->GetPeerIdentity()[0])); + EXPECT_EQ("waterzooi.test.google.be", + ToString(auth_ctx->GetPeerIdentity()[1])); + EXPECT_EQ("*.test.youtube.com", ToString(auth_ctx->GetPeerIdentity()[2])); + } } std::vector CreateTestScenarios(bool use_proxy, diff --git a/test/cpp/end2end/test_service_impl.cc b/test/cpp/end2end/test_service_impl.cc index 7c3e514effa..698d4678c87 100644 --- a/test/cpp/end2end/test_service_impl.cc +++ b/test/cpp/end2end/test_service_impl.cc @@ -63,13 +63,14 @@ void MaybeEchoDeadline(ServerContext* context, const EchoRequest* request, } void CheckServerAuthContext(const ServerContext* context, + const grpc::string& expected_transport_security_type, const grpc::string& expected_client_identity) { std::shared_ptr auth_ctx = context->auth_context(); - std::vector ssl = + std::vector tst = auth_ctx->FindPropertyValues("transport_security_type"); - EXPECT_EQ(1u, ssl.size()); - EXPECT_EQ("ssl", ToString(ssl[0])); - if (expected_client_identity.length() == 0) { + EXPECT_EQ(1u, tst.size()); + EXPECT_EQ(expected_transport_security_type, ToString(tst[0])); + if (expected_client_identity.empty()) { EXPECT_TRUE(auth_ctx->GetPeerIdentityPropertyName().empty()); EXPECT_TRUE(auth_ctx->GetPeerIdentity().empty()); EXPECT_FALSE(auth_ctx->IsPeerAuthenticated()); @@ -139,6 +140,7 @@ Status TestServiceImpl::Echo(ServerContext* context, const EchoRequest* request, (request->param().expected_client_identity().length() > 0 || request->param().check_auth_context())) { CheckServerAuthContext(context, + request->param().expected_transport_security_type(), request->param().expected_client_identity()); } if (request->has_param() && request->param().response_message_length() > 0) { diff --git a/test/cpp/util/test_credentials_provider.h b/test/cpp/util/test_credentials_provider.h index 50fadb53a24..1fb311e556e 100644 --- a/test/cpp/util/test_credentials_provider.h +++ b/test/cpp/util/test_credentials_provider.h @@ -44,7 +44,10 @@ namespace grpc { namespace testing { const char kInsecureCredentialsType[] = "INSECURE_CREDENTIALS"; -const char kTlsCredentialsType[] = "TLS_CREDENTIALS"; + +// For real credentials, like tls/ssl, this name should match the AuthContext +// property "transport_security_type". +const char kTlsCredentialsType[] = "ssl"; // Provide test credentials of a particular type. class CredentialTypeProvider { From 51221f1e7e65231539614f9b020c331f234663a5 Mon Sep 17 00:00:00 2001 From: Dan Born Date: Thu, 3 Mar 2016 17:44:50 -0800 Subject: [PATCH 10/60] clang reformat. --- test/cpp/end2end/test_service_impl.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/cpp/end2end/test_service_impl.cc b/test/cpp/end2end/test_service_impl.cc index 698d4678c87..fe29c4afe9e 100644 --- a/test/cpp/end2end/test_service_impl.cc +++ b/test/cpp/end2end/test_service_impl.cc @@ -62,9 +62,10 @@ void MaybeEchoDeadline(ServerContext* context, const EchoRequest* request, } } -void CheckServerAuthContext(const ServerContext* context, - const grpc::string& expected_transport_security_type, - const grpc::string& expected_client_identity) { +void CheckServerAuthContext( + const ServerContext* context, + const grpc::string& expected_transport_security_type, + const grpc::string& expected_client_identity) { std::shared_ptr auth_ctx = context->auth_context(); std::vector tst = auth_ctx->FindPropertyValues("transport_security_type"); From 93744fccb784c2cab8a9f008348f1e87cc1293e5 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 20:41:38 -0800 Subject: [PATCH 11/60] update PHP composer.json files --- composer.json | 11 +++++++++-- src/php/composer.json | 6 ++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 61f81e02bf4..9ba45fd3a36 100644 --- a/composer.json +++ b/composer.json @@ -2,13 +2,20 @@ "name": "grpc/grpc", "type": "library", "description": "gRPC library for PHP", - "version": "0.6.0", + "version": "0.13.0", "keywords": ["rpc"], "homepage": "http://grpc.io", "license": "BSD-3-Clause", + "repositories": [ + { + "type": "vcs", + "url": "https://github.com/stanley-cheung/Protobuf-PHP" + } + ], "require": { "php": ">=5.5.0", - "google/auth": "dev-master" + "datto/protobuf-php": "dev-master", + "google/auth": "v0.7" }, "autoload": { "psr-4": { diff --git a/src/php/composer.json b/src/php/composer.json index 1d41f847ac0..59f846fdcc6 100644 --- a/src/php/composer.json +++ b/src/php/composer.json @@ -1,7 +1,9 @@ { "name": "grpc/grpc", + "type": "library", "description": "gRPC library for PHP", - "version": "0.6.0", + "version": "0.13.0", + "keywords": ["rpc"], "homepage": "http://grpc.io", "license": "BSD-3-Clause", "repositories": [ @@ -13,7 +15,7 @@ "require": { "php": ">=5.5.0", "datto/protobuf-php": "dev-master", - "google/auth": "dev-master" + "google/auth": "v0.7" }, "autoload": { "psr-4": { From b0d1567e8ea9c4ed528e63df9a8649c953b4e349 Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Mon, 7 Mar 2016 10:51:02 -0800 Subject: [PATCH 12/60] Revert "Revert "Properly integrate async API with server-side cancellations."" --- .../grpc++/impl/codegen/completion_queue.h | 1 + include/grpc++/impl/codegen/server_context.h | 3 + src/cpp/server/server_context.cc | 26 +- test/cpp/end2end/async_end2end_test.cc | 232 +++++++++++++----- 4 files changed, 198 insertions(+), 64 deletions(-) diff --git a/include/grpc++/impl/codegen/completion_queue.h b/include/grpc++/impl/codegen/completion_queue.h index 102831e1c9b..928ab2db317 100644 --- a/include/grpc++/impl/codegen/completion_queue.h +++ b/include/grpc++/impl/codegen/completion_queue.h @@ -184,6 +184,7 @@ class CompletionQueue : private GrpcLibrary { bool Pluck(CompletionQueueTag* tag); /// Performs a single polling pluck on \a tag. + /// \warning Must not be mixed with calls to \a Next. void TryPluck(CompletionQueueTag* tag); grpc_completion_queue* cq_; // owned diff --git a/include/grpc++/impl/codegen/server_context.h b/include/grpc++/impl/codegen/server_context.h index ad08b8210d6..91ebe574b14 100644 --- a/include/grpc++/impl/codegen/server_context.h +++ b/include/grpc++/impl/codegen/server_context.h @@ -103,6 +103,9 @@ class ServerContext { void AddInitialMetadata(const grpc::string& key, const grpc::string& value); void AddTrailingMetadata(const grpc::string& key, const grpc::string& value); + // IsCancelled is always safe to call when using sync API + // When using async API, it is only safe to call IsCancelled after + // the AsyncNotifyWhenDone tag has been delivered bool IsCancelled() const; // Cancel the Call from the server. This is a best-effort API and depending on diff --git a/src/cpp/server/server_context.cc b/src/cpp/server/server_context.cc index e205a1969b3..eb49b210379 100644 --- a/src/cpp/server/server_context.cc +++ b/src/cpp/server/server_context.cc @@ -62,7 +62,11 @@ class ServerContext::CompletionOp GRPC_FINAL : public CallOpSetInterface { void FillOps(grpc_op* ops, size_t* nops) GRPC_OVERRIDE; bool FinalizeResult(void** tag, bool* status) GRPC_OVERRIDE; - bool CheckCancelled(CompletionQueue* cq); + bool CheckCancelled(CompletionQueue* cq) { + cq->TryPluck(this); + return CheckCancelledNoPluck(); + } + bool CheckCancelledAsync() { return CheckCancelledNoPluck(); } void set_tag(void* tag) { has_tag_ = true; @@ -72,6 +76,11 @@ class ServerContext::CompletionOp GRPC_FINAL : public CallOpSetInterface { void Unref(); private: + bool CheckCancelledNoPluck() { + grpc::lock_guard g(mu_); + return finalized_ ? (cancelled_ != 0) : false; + } + bool has_tag_; void* tag_; grpc::mutex mu_; @@ -88,12 +97,6 @@ void ServerContext::CompletionOp::Unref() { } } -bool ServerContext::CompletionOp::CheckCancelled(CompletionQueue* cq) { - cq->TryPluck(this); - grpc::lock_guard g(mu_); - return finalized_ ? cancelled_ != 0 : false; -} - void ServerContext::CompletionOp::FillOps(grpc_op* ops, size_t* nops) { ops->op = GRPC_OP_RECV_CLOSE_ON_SERVER; ops->data.recv_close_on_server.cancelled = &cancelled_; @@ -182,7 +185,14 @@ void ServerContext::TryCancel() const { } bool ServerContext::IsCancelled() const { - return completion_op_ && completion_op_->CheckCancelled(cq_); + if (has_notify_when_done_tag_) { + // when using async API, but the result is only valid + // if the tag has already been delivered at the completion queue + return completion_op_ && completion_op_->CheckCancelledAsync(); + } else { + // when using sync API + return completion_op_ && completion_op_->CheckCancelled(cq_); + } } void ServerContext::set_compression_level(grpc_compression_level level) { diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index 9ca3bf98f85..dc8c2bb6e5b 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -68,6 +68,7 @@ namespace testing { namespace { void* tag(int i) { return (void*)(intptr_t)i; } +int detag(void* p) { return static_cast(reinterpret_cast(p)); } #ifdef GPR_POSIX_SOCKET static int maybe_assert_non_blocking_poll(struct pollfd* pfds, nfds_t nfds, @@ -106,37 +107,50 @@ class PollingOverrider { class Verifier { public: explicit Verifier(bool spin) : spin_(spin) {} + // Expect sets the expected ok value for a specific tag Verifier& Expect(int i, bool expect_ok) { expectations_[tag(i)] = expect_ok; return *this; } + // Next waits for 1 async tag to complete, checks its + // expectations, and returns the tag + int Next(CompletionQueue* cq, bool ignore_ok) { + bool ok; + void* got_tag; + if (spin_) { + for (;;) { + auto r = cq->AsyncNext(&got_tag, &ok, gpr_time_0(GPR_CLOCK_REALTIME)); + if (r == CompletionQueue::TIMEOUT) continue; + if (r == CompletionQueue::GOT_EVENT) break; + gpr_log(GPR_ERROR, "unexpected result from AsyncNext"); + abort(); + } + } else { + EXPECT_TRUE(cq->Next(&got_tag, &ok)); + } + auto it = expectations_.find(got_tag); + EXPECT_TRUE(it != expectations_.end()); + if (!ignore_ok) { + EXPECT_EQ(it->second, ok); + } + expectations_.erase(it); + return detag(got_tag); + } + + // Verify keeps calling Next until all currently set + // expected tags are complete void Verify(CompletionQueue* cq) { Verify(cq, false); } + // This version of Verify allows optionally ignoring the + // outcome of the expectation void Verify(CompletionQueue* cq, bool ignore_ok) { GPR_ASSERT(!expectations_.empty()); while (!expectations_.empty()) { - bool ok; - void* got_tag; - if (spin_) { - for (;;) { - auto r = cq->AsyncNext(&got_tag, &ok, gpr_time_0(GPR_CLOCK_REALTIME)); - if (r == CompletionQueue::TIMEOUT) continue; - if (r == CompletionQueue::GOT_EVENT) break; - gpr_log(GPR_ERROR, "unexpected result from AsyncNext"); - abort(); - } - } else { - EXPECT_TRUE(cq->Next(&got_tag, &ok)); - } - auto it = expectations_.find(got_tag); - EXPECT_TRUE(it != expectations_.end()); - if (!ignore_ok) { - EXPECT_EQ(it->second, ok); - } - expectations_.erase(it); + Next(cq, ignore_ok); } } + // This version of Verify stops after a certain deadline void Verify(CompletionQueue* cq, std::chrono::system_clock::time_point deadline) { if (expectations_.empty()) { @@ -793,7 +807,8 @@ TEST_P(AsyncEnd2endTest, UnimplementedRpc) { } // This class is for testing scenarios where RPCs are cancelled on the server -// by calling ServerContext::TryCancel() +// by calling ServerContext::TryCancel(). Server uses AsyncNotifyWhenDone +// API to check for cancellation class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { protected: typedef enum { @@ -803,13 +818,6 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { CANCEL_AFTER_PROCESSING } ServerTryCancelRequestPhase; - void ServerTryCancel(ServerContext* context) { - EXPECT_FALSE(context->IsCancelled()); - context->TryCancel(); - gpr_log(GPR_INFO, "Server called TryCancel()"); - EXPECT_TRUE(context->IsCancelled()); - } - // Helper for testing client-streaming RPCs which are cancelled on the server. // Depending on the value of server_try_cancel parameter, this will test one // of the following three scenarios: @@ -843,6 +851,7 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { // On the server, request to be notified of 'RequestStream' calls // and receive the 'RequestStream' call just made by the client + srv_ctx.AsyncNotifyWhenDone(tag(11)); service_.RequestRequestStream(&srv_ctx, &srv_stream, cq_.get(), cq_.get(), tag(2)); Verifier(GetParam()).Expect(2, true).Verify(cq_.get()); @@ -858,9 +867,12 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { bool expected_server_cq_result = true; bool ignore_cq_result = false; + bool want_done_tag = false; if (server_try_cancel == CANCEL_BEFORE_PROCESSING) { - ServerTryCancel(&srv_ctx); + srv_ctx.TryCancel(); + Verifier(GetParam()).Expect(11, true).Verify(cq_.get()); + EXPECT_TRUE(srv_ctx.IsCancelled()); // Since cancellation is done before server reads any results, we know // for sure that all cq results will return false from this point forward @@ -868,22 +880,39 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { } std::thread* server_try_cancel_thd = NULL; + + auto verif = Verifier(GetParam()); + if (server_try_cancel == CANCEL_DURING_PROCESSING) { - server_try_cancel_thd = new std::thread( - &AsyncEnd2endServerTryCancelTest::ServerTryCancel, this, &srv_ctx); + server_try_cancel_thd = + new std::thread(&ServerContext::TryCancel, &srv_ctx); // Server will cancel the RPC in a parallel thread while reading the // requests from the client. Since the cancellation can happen at anytime, // some of the cq results (i.e those until cancellation) might be true but // its non deterministic. So better to ignore the cq results ignore_cq_result = true; + // Expect that we might possibly see the done tag that + // indicates cancellation completion in this case + want_done_tag = true; + verif.Expect(11, true); } // Server reads 3 messages (tags 6, 7 and 8) + // But if want_done_tag is true, we might also see tag 11 for (int tag_idx = 6; tag_idx <= 8; tag_idx++) { srv_stream.Read(&recv_request, tag(tag_idx)); - Verifier(GetParam()) - .Expect(tag_idx, expected_server_cq_result) - .Verify(cq_.get(), ignore_cq_result); + // Note that we'll add something to the verifier and verify that + // something was seen, but it might be tag 11 and not what we + // just added + int got_tag = verif.Expect(tag_idx, expected_server_cq_result) + .Next(cq_.get(), ignore_cq_result); + GPR_ASSERT((got_tag == tag_idx) || (got_tag == 11 && want_done_tag)); + if (got_tag == 11) { + EXPECT_TRUE(srv_ctx.IsCancelled()); + want_done_tag = false; + // Now get the other entry that we were waiting on + EXPECT_EQ(verif.Next(cq_.get(), ignore_cq_result), tag_idx); + } } if (server_try_cancel_thd != NULL) { @@ -892,7 +921,15 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { } if (server_try_cancel == CANCEL_AFTER_PROCESSING) { - ServerTryCancel(&srv_ctx); + srv_ctx.TryCancel(); + want_done_tag = true; + verif.Expect(11, true); + } + + if (want_done_tag) { + verif.Verify(cq_.get()); + EXPECT_TRUE(srv_ctx.IsCancelled()); + want_done_tag = false; } // The RPC has been cancelled at this point for sure (i.e irrespective of @@ -945,6 +982,7 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { Verifier(GetParam()).Expect(1, true).Verify(cq_.get()); // On the server, request to be notified of 'ResponseStream' calls and // receive the call just made by the client + srv_ctx.AsyncNotifyWhenDone(tag(11)); service_.RequestResponseStream(&srv_ctx, &recv_request, &srv_stream, cq_.get(), cq_.get(), tag(2)); Verifier(GetParam()).Expect(2, true).Verify(cq_.get()); @@ -952,9 +990,12 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { bool expected_cq_result = true; bool ignore_cq_result = false; + bool want_done_tag = false; if (server_try_cancel == CANCEL_BEFORE_PROCESSING) { - ServerTryCancel(&srv_ctx); + srv_ctx.TryCancel(); + Verifier(GetParam()).Expect(11, true).Verify(cq_.get()); + EXPECT_TRUE(srv_ctx.IsCancelled()); // We know for sure that all cq results will be false from this point // since the server cancelled the RPC @@ -962,24 +1003,41 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { } std::thread* server_try_cancel_thd = NULL; + + auto verif = Verifier(GetParam()); + if (server_try_cancel == CANCEL_DURING_PROCESSING) { - server_try_cancel_thd = new std::thread( - &AsyncEnd2endServerTryCancelTest::ServerTryCancel, this, &srv_ctx); + server_try_cancel_thd = + new std::thread(&ServerContext::TryCancel, &srv_ctx); // Server will cancel the RPC in a parallel thread while writing responses // to the client. Since the cancellation can happen at anytime, some of // the cq results (i.e those until cancellation) might be true but it is // non deterministic. So better to ignore the cq results ignore_cq_result = true; + // Expect that we might possibly see the done tag that + // indicates cancellation completion in this case + want_done_tag = true; + verif.Expect(11, true); } // Server sends three messages (tags 3, 4 and 5) + // But if want_done tag is true, we might also see tag 11 for (int tag_idx = 3; tag_idx <= 5; tag_idx++) { send_response.set_message("Pong " + std::to_string(tag_idx)); srv_stream.Write(send_response, tag(tag_idx)); - Verifier(GetParam()) - .Expect(tag_idx, expected_cq_result) - .Verify(cq_.get(), ignore_cq_result); + // Note that we'll add something to the verifier and verify that + // something was seen, but it might be tag 11 and not what we + // just added + int got_tag = verif.Expect(tag_idx, expected_cq_result) + .Next(cq_.get(), ignore_cq_result); + GPR_ASSERT((got_tag == tag_idx) || (got_tag == 11 && want_done_tag)); + if (got_tag == 11) { + EXPECT_TRUE(srv_ctx.IsCancelled()); + want_done_tag = false; + // Now get the other entry that we were waiting on + EXPECT_EQ(verif.Next(cq_.get(), ignore_cq_result), tag_idx); + } } if (server_try_cancel_thd != NULL) { @@ -988,13 +1046,21 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { } if (server_try_cancel == CANCEL_AFTER_PROCESSING) { - ServerTryCancel(&srv_ctx); + srv_ctx.TryCancel(); + want_done_tag = true; + verif.Expect(11, true); // Client reads may fail bacause it is notified that the stream is // cancelled. ignore_cq_result = true; } + if (want_done_tag) { + verif.Verify(cq_.get()); + EXPECT_TRUE(srv_ctx.IsCancelled()); + want_done_tag = false; + } + // Client attemts to read the three messages from the server for (int tag_idx = 6; tag_idx <= 8; tag_idx++) { cli_stream->Read(&recv_response, tag(tag_idx)); @@ -1052,6 +1118,7 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { // On the server, request to be notified of the 'BidiStream' call and // receive the call just made by the client + srv_ctx.AsyncNotifyWhenDone(tag(11)); service_.RequestBidiStream(&srv_ctx, &srv_stream, cq_.get(), cq_.get(), tag(2)); Verifier(GetParam()).Expect(2, true).Verify(cq_.get()); @@ -1063,9 +1130,12 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { bool expected_cq_result = true; bool ignore_cq_result = false; + bool want_done_tag = false; if (server_try_cancel == CANCEL_BEFORE_PROCESSING) { - ServerTryCancel(&srv_ctx); + srv_ctx.TryCancel(); + Verifier(GetParam()).Expect(11, true).Verify(cq_.get()); + EXPECT_TRUE(srv_ctx.IsCancelled()); // We know for sure that all cq results will be false from this point // since the server cancelled the RPC @@ -1073,42 +1143,84 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { } std::thread* server_try_cancel_thd = NULL; + + auto verif = Verifier(GetParam()); + if (server_try_cancel == CANCEL_DURING_PROCESSING) { - server_try_cancel_thd = new std::thread( - &AsyncEnd2endServerTryCancelTest::ServerTryCancel, this, &srv_ctx); + server_try_cancel_thd = + new std::thread(&ServerContext::TryCancel, &srv_ctx); // Since server is going to cancel the RPC in a parallel thread, some of // the cq results (i.e those until the cancellation) might be true. Since // that number is non-deterministic, it is better to ignore the cq results ignore_cq_result = true; + // Expect that we might possibly see the done tag that + // indicates cancellation completion in this case + want_done_tag = true; + verif.Expect(11, true); } + int got_tag; srv_stream.Read(&recv_request, tag(4)); - Verifier(GetParam()) - .Expect(4, expected_cq_result) - .Verify(cq_.get(), ignore_cq_result); + verif.Expect(4, expected_cq_result); + got_tag = verif.Next(cq_.get(), ignore_cq_result); + GPR_ASSERT((got_tag == 4) || (got_tag == 11 && want_done_tag)); + if (got_tag == 11) { + EXPECT_TRUE(srv_ctx.IsCancelled()); + want_done_tag = false; + // Now get the other entry that we were waiting on + EXPECT_EQ(verif.Next(cq_.get(), ignore_cq_result), 4); + } send_response.set_message("Pong"); srv_stream.Write(send_response, tag(5)); - Verifier(GetParam()) - .Expect(5, expected_cq_result) - .Verify(cq_.get(), ignore_cq_result); + verif.Expect(5, expected_cq_result); + got_tag = verif.Next(cq_.get(), ignore_cq_result); + GPR_ASSERT((got_tag == 5) || (got_tag == 11 && want_done_tag)); + if (got_tag == 11) { + EXPECT_TRUE(srv_ctx.IsCancelled()); + want_done_tag = false; + // Now get the other entry that we were waiting on + EXPECT_EQ(verif.Next(cq_.get(), ignore_cq_result), 5); + } cli_stream->Read(&recv_response, tag(6)); - Verifier(GetParam()) - .Expect(6, expected_cq_result) - .Verify(cq_.get(), ignore_cq_result); + verif.Expect(6, expected_cq_result); + got_tag = verif.Next(cq_.get(), ignore_cq_result); + GPR_ASSERT((got_tag == 6) || (got_tag == 11 && want_done_tag)); + if (got_tag == 11) { + EXPECT_TRUE(srv_ctx.IsCancelled()); + want_done_tag = false; + // Now get the other entry that we were waiting on + EXPECT_EQ(verif.Next(cq_.get(), ignore_cq_result), 6); + } // This is expected to succeed in all cases cli_stream->WritesDone(tag(7)); - Verifier(GetParam()).Expect(7, true).Verify(cq_.get()); + verif.Expect(7, true); + got_tag = verif.Next(cq_.get(), ignore_cq_result); + GPR_ASSERT((got_tag == 7) || (got_tag == 11 && want_done_tag)); + if (got_tag == 11) { + EXPECT_TRUE(srv_ctx.IsCancelled()); + want_done_tag = false; + // Now get the other entry that we were waiting on + EXPECT_EQ(verif.Next(cq_.get(), ignore_cq_result), 7); + } // This is expected to fail in all cases i.e for all values of // server_try_cancel. This is because at this point, either there are no // more msgs from the client (because client called WritesDone) or the RPC // is cancelled on the server srv_stream.Read(&recv_request, tag(8)); - Verifier(GetParam()).Expect(8, false).Verify(cq_.get()); + verif.Expect(8, false); + got_tag = verif.Next(cq_.get(), ignore_cq_result); + GPR_ASSERT((got_tag == 8) || (got_tag == 11 && want_done_tag)); + if (got_tag == 11) { + EXPECT_TRUE(srv_ctx.IsCancelled()); + want_done_tag = false; + // Now get the other entry that we were waiting on + EXPECT_EQ(verif.Next(cq_.get(), ignore_cq_result), 8); + } if (server_try_cancel_thd != NULL) { server_try_cancel_thd->join(); @@ -1116,7 +1228,15 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { } if (server_try_cancel == CANCEL_AFTER_PROCESSING) { - ServerTryCancel(&srv_ctx); + srv_ctx.TryCancel(); + want_done_tag = true; + verif.Expect(11, true); + } + + if (want_done_tag) { + verif.Verify(cq_.get()); + EXPECT_TRUE(srv_ctx.IsCancelled()); + want_done_tag = false; } // The RPC has been cancelled at this point for sure (i.e irrespective of From 0e65fb36f24bceb05b51a9970068d6ba6b1f409d Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 7 Mar 2016 17:49:37 -0800 Subject: [PATCH 13/60] Node: propagate read errors back down to core --- src/node/src/client.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/node/src/client.js b/src/node/src/client.js index 9acf51bd98b..81299b337ca 100644 --- a/src/node/src/client.js +++ b/src/node/src/client.js @@ -149,6 +149,9 @@ function _readsDone(status) { if (!status) { status = {code: grpc.status.OK, details: 'OK'}; } + if (status.code !== grpc.status.OK) { + this.call.cancelWithStatus(status.code, status.details); + } this.finished = true; this.read_status = status; this._emitStatusIfDone(); From d4726c1ae63315726faa88e924c5749f13816ead Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 23 Feb 2016 16:57:36 -0800 Subject: [PATCH 14/60] add support for clang3.4 and 3.6 --- .../cxx_ubuntu1404_x64/Dockerfile.template | 39 +++++++++ .../test/cxx_ubuntu1404_x64/Dockerfile | 86 +++++++++++++++++++ tools/run_tests/run_tests.py | 24 ++++-- 3 files changed, 141 insertions(+), 8 deletions(-) create mode 100644 templates/tools/dockerfile/test/cxx_ubuntu1404_x64/Dockerfile.template create mode 100644 tools/dockerfile/test/cxx_ubuntu1404_x64/Dockerfile diff --git a/templates/tools/dockerfile/test/cxx_ubuntu1404_x64/Dockerfile.template b/templates/tools/dockerfile/test/cxx_ubuntu1404_x64/Dockerfile.template new file mode 100644 index 00000000000..d824220afea --- /dev/null +++ b/templates/tools/dockerfile/test/cxx_ubuntu1404_x64/Dockerfile.template @@ -0,0 +1,39 @@ +%YAML 1.2 +--- | + # Copyright 2015-2016, Google Inc. + # All rights reserved. + # + # Redistribution and use in source and binary forms, with or without + # modification, are permitted provided that the following conditions are + # met: + # + # * Redistributions of source code must retain the above copyright + # notice, this list of conditions and the following disclaimer. + # * Redistributions in binary form must reproduce the above + # copyright notice, this list of conditions and the following disclaimer + # in the documentation and/or other materials provided with the + # distribution. + # * Neither the name of Google Inc. nor the names of its + # contributors may be used to endorse or promote products derived from + # this software without specific prior written permission. + # + # THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + # "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + # LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + # A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + # OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + # SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + # LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + # DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + # THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + + FROM ubuntu:14.04 + + <%include file="../../apt_get_basic.include"/> + <%include file="../../cxx_deps.include"/> + <%include file="../../run_tests_addons.include"/> + # Define the default command. + CMD ["bash"] + \ No newline at end of file diff --git a/tools/dockerfile/test/cxx_ubuntu1404_x64/Dockerfile b/tools/dockerfile/test/cxx_ubuntu1404_x64/Dockerfile new file mode 100644 index 00000000000..e8dab1b4712 --- /dev/null +++ b/tools/dockerfile/test/cxx_ubuntu1404_x64/Dockerfile @@ -0,0 +1,86 @@ +# Copyright 2015-2016, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +FROM ubuntu:14.04 + +# Install Git and basic packages. +RUN apt-get update && apt-get install -y \ + autoconf \ + autotools-dev \ + build-essential \ + bzip2 \ + ccache \ + curl \ + gcc \ + gcc-multilib \ + git \ + golang \ + gyp \ + lcov \ + libc6 \ + libc6-dbg \ + libc6-dev \ + libgtest-dev \ + libtool \ + make \ + perl \ + strace \ + python-dev \ + python-setuptools \ + python-yaml \ + telnet \ + unzip \ + wget \ + zip && apt-get clean + +#================ +# Build profiling +RUN apt-get update && apt-get install -y time && apt-get clean + +#================= +# C++ dependencies +RUN apt-get update && apt-get -y install libgflags-dev libgtest-dev libc++-dev clang && apt-get clean + +# Prepare ccache +RUN ln -s /usr/bin/ccache /usr/local/bin/gcc +RUN ln -s /usr/bin/ccache /usr/local/bin/g++ +RUN ln -s /usr/bin/ccache /usr/local/bin/cc +RUN ln -s /usr/bin/ccache /usr/local/bin/c++ +RUN ln -s /usr/bin/ccache /usr/local/bin/clang +RUN ln -s /usr/bin/ccache /usr/local/bin/clang++ + +#====================== +# Zookeeper dependencies +# TODO(jtattermusch): is zookeeper still needed? +RUN apt-get install -y libzookeeper-mt-dev + +RUN mkdir /var/local/jenkins + +# Define the default command. +CMD ["bash"] diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index cc004f38d7f..e687d463427 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -142,9 +142,8 @@ class CLanguage(object): self._make_options = [_windows_toolset_option(self.args.compiler), _windows_arch_option(self.args.arch)] else: - self._make_options = [] - self._docker_distro = self._get_docker_distro(self.args.use_docker, - self.args.compiler) + self._docker_distro, self._make_options = self._compiler_options(self.args.use_docker, + self.args.compiler) def test_specs(self): out = [] @@ -229,18 +228,26 @@ class CLanguage(object): def makefile_name(self): return 'Makefile' - def _get_docker_distro(self, use_docker, compiler): + def _clang_make_options(self): + return ['CC=clang', 'CXX=clang++', 'LD=clang', 'LDXX=clang++'] + + def _compiler_options(self, use_docker, compiler): + """Returns docker distro and make options to use for given compiler.""" if _is_use_docker_child(): - return "already_under_docker" + return ("already_under_docker", []) if not use_docker: _check_compiler(compiler, ['default']) if compiler == 'gcc4.9' or compiler == 'default': - return 'jessie' + return ('jessie', []) elif compiler == 'gcc4.4': - return 'squeeze' + return ('squeeze', []) elif compiler == 'gcc5.3': - return 'ubuntu1604' + return ('ubuntu1604', []) + elif compiler == 'clang3.4': + return ('ubuntu1404', self._clang_make_options()) + elif compiler == 'clang3.6': + return ('ubuntu1604', self._clang_make_options()) else: raise Exception('Compiler %s not supported.' % compiler) @@ -774,6 +781,7 @@ argp.add_argument('--arch', argp.add_argument('--compiler', choices=['default', 'gcc4.4', 'gcc4.9', 'gcc5.3', + 'clang3.4', 'clang3.6', 'vs2010', 'vs2013', 'vs2015'], default='default', help='Selects compiler to use. Allowed values depend on the platform and language.') From 7212f16700f765f9acd9681d945401a7ba81081a Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 19:29:11 -0800 Subject: [PATCH 15/60] php: fix examples ClientStub constructor --- examples/php/greeter_client.php | 4 +++- examples/php/route_guide/route_guide_client.php | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/examples/php/greeter_client.php b/examples/php/greeter_client.php index e5e4c2651e9..718ef88c644 100644 --- a/examples/php/greeter_client.php +++ b/examples/php/greeter_client.php @@ -36,7 +36,9 @@ require dirname(__FILE__) . '/vendor/autoload.php'; require dirname(__FILE__) . '/helloworld.php'; function greet($name) { - $client = new helloworld\GreeterClient('localhost:50051', []); + $client = new helloworld\GreeterClient('localhost:50051', [ + 'credentials' => Grpc\ChannelCredentials::createInsecure() + ]); $request = new helloworld\HelloRequest(); $request->setName($name); list($reply, $status) = $client->SayHello($request)->wait(); diff --git a/examples/php/route_guide/route_guide_client.php b/examples/php/route_guide/route_guide_client.php index 3cd1df72541..2f9533be4b7 100644 --- a/examples/php/route_guide/route_guide_client.php +++ b/examples/php/route_guide/route_guide_client.php @@ -37,7 +37,9 @@ require dirname(__FILE__) . '/route_guide.php'; define('COORD_FACTOR', 1e7); -$client = new routeguide\RouteGuideClient('localhost:50051', []); +$client = new routeguide\RouteGuideClient('localhost:50051', [ + 'credentials' => Grpc\ChannelCredentials::createInsecure() +]); function printFeature($feature) { $name = $feature->getName(); From 40202158fddf89d16b0e638d9cf415489e948926 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 21:16:39 -0800 Subject: [PATCH 16/60] fix copyright --- examples/php/greeter_client.php | 2 +- examples/php/route_guide/route_guide_client.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/php/greeter_client.php b/examples/php/greeter_client.php index 718ef88c644..3fab14f33e1 100644 --- a/examples/php/greeter_client.php +++ b/examples/php/greeter_client.php @@ -1,7 +1,7 @@ Date: Mon, 7 Mar 2016 16:09:21 -0800 Subject: [PATCH 17/60] one more --- src/php/tests/generated_code/math_client.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/php/tests/generated_code/math_client.php b/src/php/tests/generated_code/math_client.php index 76ccabc0684..b8652ceb088 100644 --- a/src/php/tests/generated_code/math_client.php +++ b/src/php/tests/generated_code/math_client.php @@ -43,7 +43,9 @@ function p($line) $host = 'localhost:50051'; p("Connecting to host: $host"); -$client = new math\MathClient($host, []); +$client = new math\MathClient($host, [ + 'credentials' => Grpc\ChannelCredentials::createInsecure() +]); p('Client class: '.get_class($client)); p(''); From cfad24393931706e667554a867aa98a6c88236ea Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Tue, 8 Mar 2016 08:41:08 -0800 Subject: [PATCH 18/60] fixed copyright --- src/php/tests/generated_code/math_client.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/php/tests/generated_code/math_client.php b/src/php/tests/generated_code/math_client.php index b8652ceb088..2085560d19e 100644 --- a/src/php/tests/generated_code/math_client.php +++ b/src/php/tests/generated_code/math_client.php @@ -1,7 +1,7 @@ Date: Thu, 3 Mar 2016 20:39:01 -0800 Subject: [PATCH 19/60] update PHP composer.json files --- composer.json | 11 +++++++++-- examples/php/composer.json | 11 +---------- src/php/composer.json | 6 ++++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/composer.json b/composer.json index 61f81e02bf4..97b1a5cb49b 100644 --- a/composer.json +++ b/composer.json @@ -2,13 +2,20 @@ "name": "grpc/grpc", "type": "library", "description": "gRPC library for PHP", - "version": "0.6.0", + "version": "0.14.0", "keywords": ["rpc"], "homepage": "http://grpc.io", "license": "BSD-3-Clause", + "repositories": [ + { + "type": "vcs", + "url": "https://github.com/stanley-cheung/Protobuf-PHP" + } + ], "require": { "php": ">=5.5.0", - "google/auth": "dev-master" + "datto/protobuf-php": "dev-master", + "google/auth": "v0.7" }, "autoload": { "psr-4": { diff --git a/examples/php/composer.json b/examples/php/composer.json index 9f44f0b186a..314066b22df 100644 --- a/examples/php/composer.json +++ b/examples/php/composer.json @@ -1,17 +1,8 @@ { - "repositories": [ - { - "type": "vcs", - "url": "https://github.com/stanley-cheung/Protobuf-PHP" - } - ], "name": "grpc/grpc-demo", "description": "gRPC example for PHP", "minimum-stability": "dev", "require": { - "php": ">=5.5.0", - "datto/protobuf-php": "dev-master", - "google/auth": "dev-master", - "grpc/grpc": "dev-release-0_11" + "grpc/grpc": "dev-release-0_13" } } diff --git a/src/php/composer.json b/src/php/composer.json index 1d41f847ac0..01674a25db8 100644 --- a/src/php/composer.json +++ b/src/php/composer.json @@ -1,7 +1,9 @@ { "name": "grpc/grpc", + "type": "library", "description": "gRPC library for PHP", - "version": "0.6.0", + "version": "0.14.0", + "keywords": ["rpc"], "homepage": "http://grpc.io", "license": "BSD-3-Clause", "repositories": [ @@ -13,7 +15,7 @@ "require": { "php": ">=5.5.0", "datto/protobuf-php": "dev-master", - "google/auth": "dev-master" + "google/auth": "v0.7" }, "autoload": { "psr-4": { From 7fc1208a5cee894cada499a33d6a7e36d4209edc Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 22:14:54 -0800 Subject: [PATCH 20/60] still need this block for the example composer.json file --- examples/php/composer.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/examples/php/composer.json b/examples/php/composer.json index 314066b22df..c837bf7ac0f 100644 --- a/examples/php/composer.json +++ b/examples/php/composer.json @@ -2,6 +2,12 @@ "name": "grpc/grpc-demo", "description": "gRPC example for PHP", "minimum-stability": "dev", + "repositories": [ + { + "type": "vcs", + "url": "https://github.com/stanley-cheung/Protobuf-PHP" + } + ], "require": { "grpc/grpc": "dev-release-0_13" } From 5709e930faec4e88e63cec7edb396264031246a7 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Mon, 7 Mar 2016 17:34:47 -0800 Subject: [PATCH 21/60] remove stale src/php/ext/grpc/README.md --- package.xml | 2 +- src/php/ext/grpc/README.md | 67 ---------------------------------- templates/package.xml.template | 2 +- 3 files changed, 2 insertions(+), 69 deletions(-) delete mode 100644 src/php/ext/grpc/README.md diff --git a/package.xml b/package.xml index e65ab73b54a..bb4dcc2037a 100644 --- a/package.xml +++ b/package.xml @@ -27,9 +27,9 @@ + - diff --git a/src/php/ext/grpc/README.md b/src/php/ext/grpc/README.md deleted file mode 100644 index 6e1cb2002f5..00000000000 --- a/src/php/ext/grpc/README.md +++ /dev/null @@ -1,67 +0,0 @@ -gRPC PHP Extension -================== - -# Requirements - - * PHP 5.5+ - * [gRPC core library](https://github.com/grpc/grpc) 0.11.0 - -# Installation - -## Install PHP 5 - -``` -$ sudo apt-get install git php5 php5-dev php-pear unzip -``` - -## Compile gRPC Core Library - -Clone the gRPC source code repository - -``` -$ git clone https://github.com/grpc/grpc.git -``` - -Build and install the gRPC C core libraries - -```sh -$ cd grpc -$ git checkout --track origin/release-0_11 -$ git pull --recurse-submodules && git submodule update --init --recursive -$ make -$ sudo make install -``` - -Note: you may encounter a warning about the Protobuf compiler `protoc` 3.0.0+ not being installed. The following might help, and will be useful later on when we need to compile the `protoc-gen-php` tool. - -```sh -$ cd grpc/third_party/protobuf -$ sudo make install # 'make' should have been run by core grpc -``` - -## Install the gRPC PHP extension - -Quick install - -```sh -$ sudo pecl install grpc -``` - -Note: before a stable release, you may need to do - -```sh -$ sudo pecl install grpc-beta -``` - -OR - -Compile from source - -```sh -$ # from grpc -$ cd src/php/ext/grpc -$ phpize -$ ./configure -$ make -$ sudo make install -``` diff --git a/templates/package.xml.template b/templates/package.xml.template index d309bfddbc3..8f757401e4c 100644 --- a/templates/package.xml.template +++ b/templates/package.xml.template @@ -29,9 +29,9 @@ + - % for source in php_config_m4.src + php_config_m4.headers: % endfor From e1e4559690eb5ecea7b8c1a376936f1095dd0c11 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 08:01:58 -0800 Subject: [PATCH 22/60] Make blocking_resolve_address overridable --- src/core/iomgr/resolve_address.h | 4 ++-- src/core/iomgr/resolve_address_posix.c | 19 +++++++++++-------- src/core/iomgr/resolve_address_windows.c | 19 +++++++++++-------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/core/iomgr/resolve_address.h b/src/core/iomgr/resolve_address.h index 01eedffa889..508e161a7e0 100644 --- a/src/core/iomgr/resolve_address.h +++ b/src/core/iomgr/resolve_address.h @@ -66,7 +66,7 @@ void grpc_resolved_addresses_destroy(grpc_resolved_addresses *addresses); /* Resolve addr in a blocking fashion. Returns NULL on failure. On success, result must be freed with grpc_resolved_addresses_destroy. */ -grpc_resolved_addresses *grpc_blocking_resolve_address( - const char *addr, const char *default_port); +extern grpc_resolved_addresses *(*grpc_blocking_resolve_address)( + const char *name, const char *default_port); #endif /* GRPC_INTERNAL_CORE_IOMGR_RESOLVE_ADDRESS_H */ diff --git a/src/core/iomgr/resolve_address_posix.c b/src/core/iomgr/resolve_address_posix.c index c51745b9187..a6c9893f231 100644 --- a/src/core/iomgr/resolve_address_posix.c +++ b/src/core/iomgr/resolve_address_posix.c @@ -34,18 +34,13 @@ #include #ifdef GPR_POSIX_SOCKET -#include "src/core/iomgr/sockaddr.h" #include "src/core/iomgr/resolve_address.h" +#include "src/core/iomgr/sockaddr.h" +#include #include #include -#include -#include "src/core/iomgr/executor.h" -#include "src/core/iomgr/iomgr_internal.h" -#include "src/core/iomgr/sockaddr_utils.h" -#include "src/core/support/block_annotate.h" -#include "src/core/support/string.h" #include #include #include @@ -53,6 +48,11 @@ #include #include #include +#include "src/core/iomgr/executor.h" +#include "src/core/iomgr/iomgr_internal.h" +#include "src/core/iomgr/sockaddr_utils.h" +#include "src/core/support/block_annotate.h" +#include "src/core/support/string.h" typedef struct { char *name; @@ -62,7 +62,7 @@ typedef struct { void *arg; } request; -grpc_resolved_addresses *grpc_blocking_resolve_address( +static grpc_resolved_addresses *blocking_resolve_address_impl( const char *name, const char *default_port) { struct addrinfo hints; struct addrinfo *result = NULL, *resp; @@ -150,6 +150,9 @@ done: return addrs; } +grpc_resolved_addresses *(*grpc_blocking_resolve_address)( + const char *name, const char *default_port) = blocking_resolve_address_impl; + /* Callback to be passed to grpc_executor to asynch-ify * grpc_blocking_resolve_address */ static void do_request_thread(grpc_exec_ctx *exec_ctx, void *rp, bool success) { diff --git a/src/core/iomgr/resolve_address_windows.c b/src/core/iomgr/resolve_address_windows.c index 28c8661e73b..472e7971634 100644 --- a/src/core/iomgr/resolve_address_windows.c +++ b/src/core/iomgr/resolve_address_windows.c @@ -34,17 +34,12 @@ #include #ifdef GPR_WINSOCK_SOCKET -#include "src/core/iomgr/sockaddr.h" #include "src/core/iomgr/resolve_address.h" +#include "src/core/iomgr/sockaddr.h" -#include #include +#include -#include "src/core/iomgr/executor.h" -#include "src/core/iomgr/iomgr_internal.h" -#include "src/core/iomgr/sockaddr_utils.h" -#include "src/core/support/block_annotate.h" -#include "src/core/support/string.h" #include #include #include @@ -52,6 +47,11 @@ #include #include #include +#include "src/core/iomgr/executor.h" +#include "src/core/iomgr/iomgr_internal.h" +#include "src/core/iomgr/sockaddr_utils.h" +#include "src/core/support/block_annotate.h" +#include "src/core/support/string.h" typedef struct { char *name; @@ -61,7 +61,7 @@ typedef struct { void *arg; } request; -grpc_resolved_addresses *grpc_blocking_resolve_address( +static grpc_resolved_addresses *blocking_resolve_address_impl( const char *name, const char *default_port) { struct addrinfo hints; struct addrinfo *result = NULL, *resp; @@ -133,6 +133,9 @@ done: return addrs; } +grpc_resolved_addresses *(*grpc_blocking_resolve_address)( + const char *name, const char *default_port) = blocking_resolve_address_impl; + /* Callback to be passed to grpc_executor to asynch-ify * grpc_blocking_resolve_address */ static void do_request_thread(grpc_exec_ctx *exec_ctx, void *rp, bool success) { From e91ef68d2a9077641d6ece47b388e523942164fe Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 08:59:17 -0800 Subject: [PATCH 23/60] Add failing test --- Makefile | 36 ++++ build.yaml | 10 + .../dns_resolver_connectivity_test.c | 133 ++++++++++++ tools/run_tests/sources_and_headers.json | 16 ++ tools/run_tests/tests.json | 21 ++ vsprojects/buildtests_c.sln | 27 +++ .../dns_resolver_connectivity_test.vcxproj | 199 ++++++++++++++++++ ...resolver_connectivity_test.vcxproj.filters | 24 +++ 8 files changed, 466 insertions(+) create mode 100644 test/core/client_config/resolvers/dns_resolver_connectivity_test.c create mode 100644 vsprojects/vcxproj/test/dns_resolver_connectivity_test/dns_resolver_connectivity_test.vcxproj create mode 100644 vsprojects/vcxproj/test/dns_resolver_connectivity_test/dns_resolver_connectivity_test.vcxproj.filters diff --git a/Makefile b/Makefile index 5be0d146afc..171a83c2a3a 100644 --- a/Makefile +++ b/Makefile @@ -849,6 +849,7 @@ chttp2_status_conversion_test: $(BINDIR)/$(CONFIG)/chttp2_status_conversion_test chttp2_stream_map_test: $(BINDIR)/$(CONFIG)/chttp2_stream_map_test chttp2_varint_test: $(BINDIR)/$(CONFIG)/chttp2_varint_test compression_test: $(BINDIR)/$(CONFIG)/compression_test +dns_resolver_connectivity_test: $(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test dns_resolver_test: $(BINDIR)/$(CONFIG)/dns_resolver_test dualstack_socket_test: $(BINDIR)/$(CONFIG)/dualstack_socket_test endpoint_pair_test: $(BINDIR)/$(CONFIG)/endpoint_pair_test @@ -1159,6 +1160,7 @@ buildtests_c: privatelibs_c \ $(BINDIR)/$(CONFIG)/chttp2_stream_map_test \ $(BINDIR)/$(CONFIG)/chttp2_varint_test \ $(BINDIR)/$(CONFIG)/compression_test \ + $(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test \ $(BINDIR)/$(CONFIG)/dns_resolver_test \ $(BINDIR)/$(CONFIG)/dualstack_socket_test \ $(BINDIR)/$(CONFIG)/endpoint_pair_test \ @@ -1402,6 +1404,8 @@ test_c: buildtests_c $(Q) $(BINDIR)/$(CONFIG)/chttp2_varint_test || ( echo test chttp2_varint_test failed ; exit 1 ) $(E) "[RUN] Testing compression_test" $(Q) $(BINDIR)/$(CONFIG)/compression_test || ( echo test compression_test failed ; exit 1 ) + $(E) "[RUN] Testing dns_resolver_connectivity_test" + $(Q) $(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test || ( echo test dns_resolver_connectivity_test failed ; exit 1 ) $(E) "[RUN] Testing dns_resolver_test" $(Q) $(BINDIR)/$(CONFIG)/dns_resolver_test || ( echo test dns_resolver_test failed ; exit 1 ) $(E) "[RUN] Testing dualstack_socket_test" @@ -6075,6 +6079,38 @@ endif endif +DNS_RESOLVER_CONNECTIVITY_TEST_SRC = \ + test/core/client_config/resolvers/dns_resolver_connectivity_test.c \ + +DNS_RESOLVER_CONNECTIVITY_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(DNS_RESOLVER_CONNECTIVITY_TEST_SRC)))) +ifeq ($(NO_SECURE),true) + +# You can't build secure targets if you don't have OpenSSL. + +$(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test: openssl_dep_error + +else + + + +$(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test: $(DNS_RESOLVER_CONNECTIVITY_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + $(E) "[LD] Linking $@" + $(Q) mkdir -p `dirname $@` + $(Q) $(LD) $(LDFLAGS) $(DNS_RESOLVER_CONNECTIVITY_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test + +endif + +$(OBJDIR)/$(CONFIG)/test/core/client_config/resolvers/dns_resolver_connectivity_test.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + +deps_dns_resolver_connectivity_test: $(DNS_RESOLVER_CONNECTIVITY_TEST_OBJS:.o=.dep) + +ifneq ($(NO_SECURE),true) +ifneq ($(NO_DEPS),true) +-include $(DNS_RESOLVER_CONNECTIVITY_TEST_OBJS:.o=.dep) +endif +endif + + DNS_RESOLVER_TEST_SRC = \ test/core/client_config/resolvers/dns_resolver_test.c \ diff --git a/build.yaml b/build.yaml index a277539a054..7cafb0ac6fe 100644 --- a/build.yaml +++ b/build.yaml @@ -1057,6 +1057,16 @@ targets: - grpc - gpr_test_util - gpr +- name: dns_resolver_connectivity_test + build: test + language: c + src: + - test/core/client_config/resolvers/dns_resolver_connectivity_test.c + deps: + - grpc_test_util + - grpc + - gpr_test_util + - gpr - name: dns_resolver_test build: test language: c diff --git a/test/core/client_config/resolvers/dns_resolver_connectivity_test.c b/test/core/client_config/resolvers/dns_resolver_connectivity_test.c new file mode 100644 index 00000000000..e4d83539fb9 --- /dev/null +++ b/test/core/client_config/resolvers/dns_resolver_connectivity_test.c @@ -0,0 +1,133 @@ +/* + * + * Copyright 2015-2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include "src/core/client_config/resolvers/dns_resolver.h" + +#include + +#include +#include + +#include "src/core/iomgr/resolve_address.h" +#include "test/core/util/test_config.h" + +static void subchannel_factory_ref(grpc_subchannel_factory *scv) {} +static void subchannel_factory_unref(grpc_exec_ctx *exec_ctx, + grpc_subchannel_factory *scv) {} +static grpc_subchannel *subchannel_factory_create_subchannel( + grpc_exec_ctx *exec_ctx, grpc_subchannel_factory *factory, + grpc_subchannel_args *args) { + GPR_UNREACHABLE_CODE(return NULL); +} + +static const grpc_subchannel_factory_vtable sc_vtable = { + subchannel_factory_ref, subchannel_factory_unref, + subchannel_factory_create_subchannel}; + +static grpc_subchannel_factory sc_factory = {&sc_vtable}; + +static gpr_mu g_mu; +static bool g_fail_resolution = true; + +static grpc_resolved_addresses *my_resolve_address(const char *name, + const char *addr) { + gpr_mu_lock(&g_mu); + GPR_ASSERT(0 == strcmp("test", name)); + if (g_fail_resolution) { + g_fail_resolution = false; + gpr_mu_unlock(&g_mu); + return NULL; + } else { + gpr_mu_unlock(&g_mu); + grpc_resolved_addresses *addrs = gpr_malloc(sizeof(*addrs)); + addrs->naddrs = 1; + addrs->addrs = gpr_malloc(sizeof(*addrs->addrs)); + addrs->addrs[0].len = 123; + return addrs; + } +} + +static grpc_resolver *create_resolver(const char *name) { + grpc_resolver_factory *factory = grpc_dns_resolver_factory_create(); + grpc_uri *uri = grpc_uri_parse(name, 0); + GPR_ASSERT(uri); + grpc_resolver_args args; + memset(&args, 0, sizeof(args)); + args.uri = uri; + args.subchannel_factory = &sc_factory; + grpc_resolver *resolver = + grpc_resolver_factory_create_resolver(factory, &args); + grpc_resolver_factory_unref(factory); + grpc_uri_destroy(uri); + return resolver; +} + +static void on_done(grpc_exec_ctx *exec_ctx, void *ev, bool success) { + gpr_event_set(ev, (void *)1); +} + +int main(int argc, char **argv) { + grpc_test_init(argc, argv); + + grpc_init(); + gpr_mu_init(&g_mu); + grpc_blocking_resolve_address = my_resolve_address; + + grpc_resolver *resolver = create_resolver("dns:test"); + + grpc_client_config *config = (grpc_client_config *)1; + + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + gpr_event ev1; + gpr_event_init(&ev1); + grpc_resolver_next(&exec_ctx, resolver, &config, + grpc_closure_create(on_done, &ev1)); + grpc_exec_ctx_flush(&exec_ctx); + GPR_ASSERT(gpr_event_wait(&ev1, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5))); + GPR_ASSERT(config == NULL); + + gpr_event ev2; + gpr_event_init(&ev2); + grpc_resolver_next(&exec_ctx, resolver, &config, + grpc_closure_create(on_done, &ev2)); + grpc_exec_ctx_flush(&exec_ctx); + GPR_ASSERT(gpr_event_wait(&ev2, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5))); + GPR_ASSERT(config != NULL); + + grpc_client_config_unref(&exec_ctx, config); + GRPC_RESOLVER_UNREF(&exec_ctx, resolver, "test"); + grpc_exec_ctx_finish(&exec_ctx); + + grpc_shutdown(); + gpr_mu_destroy(&g_mu); +} diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 0c7a6c7b5f3..0039b99a55e 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -189,6 +189,22 @@ "third_party": false, "type": "target" }, + { + "deps": [ + "gpr", + "gpr_test_util", + "grpc", + "grpc_test_util" + ], + "headers": [], + "language": "c", + "name": "dns_resolver_connectivity_test", + "src": [ + "test/core/client_config/resolvers/dns_resolver_connectivity_test.c" + ], + "third_party": false, + "type": "target" + }, { "deps": [ "gpr", diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index d91245cd06a..c2d6cadd530 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -253,6 +253,27 @@ "windows" ] }, + { + "args": [], + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "gtest": false, + "language": "c", + "name": "dns_resolver_connectivity_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ] + }, { "args": [], "ci_platforms": [ diff --git a/vsprojects/buildtests_c.sln b/vsprojects/buildtests_c.sln index 7cdc3b7ee81..be2c4a3bb14 100644 --- a/vsprojects/buildtests_c.sln +++ b/vsprojects/buildtests_c.sln @@ -251,6 +251,17 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "compression_test", "vcxproj {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} = {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} EndProjectSection EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "dns_resolver_connectivity_test", "vcxproj\test\dns_resolver_connectivity_test\dns_resolver_connectivity_test.vcxproj", "{F7B6FE68-E847-D7CA-4062-E737E542BCC3}" + ProjectSection(myProperties) = preProject + lib = "False" + EndProjectSection + ProjectSection(ProjectDependencies) = postProject + {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} = {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} + {29D16885-7228-4C31-81ED-5F9187C7F2A9} = {29D16885-7228-4C31-81ED-5F9187C7F2A9} + {EAB0A629-17A9-44DB-B5FF-E91A721FE037} = {EAB0A629-17A9-44DB-B5FF-E91A721FE037} + {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} = {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} + EndProjectSection +EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "dns_resolver_test", "vcxproj\test\dns_resolver_test\dns_resolver_test.vcxproj", "{D06E10DC-272A-5203-7066-2698A247DF26}" ProjectSection(myProperties) = preProject lib = "False" @@ -1736,6 +1747,22 @@ Global {5AFE7D17-A4A7-D68E-4491-CBC852F9D2A0}.Release-DLL|Win32.Build.0 = Release|Win32 {5AFE7D17-A4A7-D68E-4491-CBC852F9D2A0}.Release-DLL|x64.ActiveCfg = Release|x64 {5AFE7D17-A4A7-D68E-4491-CBC852F9D2A0}.Release-DLL|x64.Build.0 = Release|x64 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Debug|Win32.ActiveCfg = Debug|Win32 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Debug|x64.ActiveCfg = Debug|x64 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Release|Win32.ActiveCfg = Release|Win32 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Release|x64.ActiveCfg = Release|x64 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Debug|Win32.Build.0 = Debug|Win32 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Debug|x64.Build.0 = Debug|x64 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Release|Win32.Build.0 = Release|Win32 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Release|x64.Build.0 = Release|x64 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Debug-DLL|Win32.ActiveCfg = Debug|Win32 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Debug-DLL|Win32.Build.0 = Debug|Win32 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Debug-DLL|x64.ActiveCfg = Debug|x64 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Debug-DLL|x64.Build.0 = Debug|x64 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Release-DLL|Win32.ActiveCfg = Release|Win32 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Release-DLL|Win32.Build.0 = Release|Win32 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Release-DLL|x64.ActiveCfg = Release|x64 + {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Release-DLL|x64.Build.0 = Release|x64 {D06E10DC-272A-5203-7066-2698A247DF26}.Debug|Win32.ActiveCfg = Debug|Win32 {D06E10DC-272A-5203-7066-2698A247DF26}.Debug|x64.ActiveCfg = Debug|x64 {D06E10DC-272A-5203-7066-2698A247DF26}.Release|Win32.ActiveCfg = Release|Win32 diff --git a/vsprojects/vcxproj/test/dns_resolver_connectivity_test/dns_resolver_connectivity_test.vcxproj b/vsprojects/vcxproj/test/dns_resolver_connectivity_test/dns_resolver_connectivity_test.vcxproj new file mode 100644 index 00000000000..76ac196dda6 --- /dev/null +++ b/vsprojects/vcxproj/test/dns_resolver_connectivity_test/dns_resolver_connectivity_test.vcxproj @@ -0,0 +1,199 @@ + + + + + + Debug + Win32 + + + Debug + x64 + + + Release + Win32 + + + Release + x64 + + + + {F7B6FE68-E847-D7CA-4062-E737E542BCC3} + true + $(SolutionDir)IntDir\$(MSBuildProjectName)\ + + + + v100 + + + v110 + + + v120 + + + v140 + + + Application + true + Unicode + + + Application + false + true + Unicode + + + + + + + + + + + + + + dns_resolver_connectivity_test + static + Debug + static + Debug + + + dns_resolver_connectivity_test + static + Release + static + Release + + + + NotUsing + Level3 + Disabled + WIN32;_DEBUG;_LIB;%(PreprocessorDefinitions) + true + MultiThreadedDebug + true + None + false + + + Console + true + false + + + + + + NotUsing + Level3 + Disabled + WIN32;_DEBUG;_LIB;%(PreprocessorDefinitions) + true + MultiThreadedDebug + true + None + false + + + Console + true + false + + + + + + NotUsing + Level3 + MaxSpeed + WIN32;NDEBUG;_LIB;%(PreprocessorDefinitions) + true + true + true + MultiThreaded + true + None + false + + + Console + true + false + true + true + + + + + + NotUsing + Level3 + MaxSpeed + WIN32;NDEBUG;_LIB;%(PreprocessorDefinitions) + true + true + true + MultiThreaded + true + None + false + + + Console + true + false + true + true + + + + + + + + + + {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} + + + {29D16885-7228-4C31-81ED-5F9187C7F2A9} + + + {EAB0A629-17A9-44DB-B5FF-E91A721FE037} + + + {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} + + + + + + + + + + + + + + + This project references NuGet package(s) that are missing on this computer. Enable NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}. + + + + + + + + + diff --git a/vsprojects/vcxproj/test/dns_resolver_connectivity_test/dns_resolver_connectivity_test.vcxproj.filters b/vsprojects/vcxproj/test/dns_resolver_connectivity_test/dns_resolver_connectivity_test.vcxproj.filters new file mode 100644 index 00000000000..e49318df110 --- /dev/null +++ b/vsprojects/vcxproj/test/dns_resolver_connectivity_test/dns_resolver_connectivity_test.vcxproj.filters @@ -0,0 +1,24 @@ + + + + + test\core\client_config\resolvers + + + + + + {7743e287-3311-1206-2766-32381628ff07} + + + {cc06b800-cd26-f7a8-7ecf-ec789e78881b} + + + {4fbd5c0a-d1e3-6658-5788-2fc6f2cc9071} + + + {eab637a2-7ac1-f6be-4fc2-c8989c66070b} + + + + From 1e55bd455d5fa901e19c1ccf64996c0adc7a4584 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 09:47:43 -0800 Subject: [PATCH 24/60] Add retry for dns resolution --- src/core/client_config/client_config.c | 4 +- .../client_config/lb_policies/pick_first.c | 2 +- .../client_config/resolvers/dns_resolver.c | 53 ++++++++++++++++--- .../dns_resolver_connectivity_test.c | 21 ++++++-- 4 files changed, 67 insertions(+), 13 deletions(-) diff --git a/src/core/client_config/client_config.c b/src/core/client_config/client_config.c index 6ecffb38541..25f4ac25177 100644 --- a/src/core/client_config/client_config.c +++ b/src/core/client_config/client_config.c @@ -53,7 +53,9 @@ void grpc_client_config_ref(grpc_client_config *c) { gpr_ref(&c->refs); } void grpc_client_config_unref(grpc_exec_ctx *exec_ctx, grpc_client_config *c) { if (gpr_unref(&c->refs)) { - GRPC_LB_POLICY_UNREF(exec_ctx, c->lb_policy, "client_config"); + if (c->lb_policy != NULL) { + GRPC_LB_POLICY_UNREF(exec_ctx, c->lb_policy, "client_config"); + } gpr_free(c); } } diff --git a/src/core/client_config/lb_policies/pick_first.c b/src/core/client_config/lb_policies/pick_first.c index 81167b31c8c..8ed1223d399 100644 --- a/src/core/client_config/lb_policies/pick_first.c +++ b/src/core/client_config/lb_policies/pick_first.c @@ -387,8 +387,8 @@ static void pick_first_factory_unref(grpc_lb_policy_factory *factory) {} static grpc_lb_policy *create_pick_first(grpc_lb_policy_factory *factory, grpc_lb_policy_args *args) { + if (args->num_subchannels == 0) return NULL; pick_first_lb_policy *p = gpr_malloc(sizeof(*p)); - GPR_ASSERT(args->num_subchannels > 0); memset(p, 0, sizeof(*p)); grpc_lb_policy_init(&p->base, &pick_first_lb_policy_vtable); p->subchannels = diff --git a/src/core/client_config/resolvers/dns_resolver.c b/src/core/client_config/resolvers/dns_resolver.c index 376b6b3d768..55457647b3c 100644 --- a/src/core/client_config/resolvers/dns_resolver.c +++ b/src/core/client_config/resolvers/dns_resolver.c @@ -41,6 +41,7 @@ #include "src/core/client_config/lb_policy_registry.h" #include "src/core/iomgr/resolve_address.h" +#include "src/core/iomgr/timer.h" #include "src/core/support/string.h" typedef struct { @@ -71,6 +72,9 @@ typedef struct { grpc_client_config **target_config; /** current (fully resolved) config */ grpc_client_config *resolved_config; + /** retry timer */ + bool have_retry_timer; + grpc_timer retry_timer; } dns_resolver; static void dns_destroy(grpc_exec_ctx *exec_ctx, grpc_resolver *r); @@ -125,6 +129,21 @@ static void dns_next(grpc_exec_ctx *exec_ctx, grpc_resolver *resolver, gpr_mu_unlock(&r->mu); } +static void dns_on_retry_timer(grpc_exec_ctx *exec_ctx, void *arg, + bool success) { + dns_resolver *r = arg; + + if (success) { + gpr_mu_lock(&r->mu); + if (!r->resolving) { + dns_start_resolving_locked(r); + } + gpr_mu_unlock(&r->mu); + } + + GRPC_RESOLVER_UNREF(exec_ctx, &r->base, "retry-timer"); +} + static void dns_on_resolved(grpc_exec_ctx *exec_ctx, void *arg, grpc_resolved_addresses *addresses) { dns_resolver *r = arg; @@ -133,29 +152,47 @@ static void dns_on_resolved(grpc_exec_ctx *exec_ctx, void *arg, grpc_subchannel_args args; grpc_lb_policy *lb_policy; size_t i; - if (addresses) { + gpr_mu_lock(&r->mu); + GPR_ASSERT(r->resolving); + r->resolving = 0; + if (addresses != NULL) { grpc_lb_policy_args lb_policy_args; config = grpc_client_config_create(); subchannels = gpr_malloc(sizeof(grpc_subchannel *) * addresses->naddrs); + size_t naddrs = 0; for (i = 0; i < addresses->naddrs; i++) { memset(&args, 0, sizeof(args)); args.addr = (struct sockaddr *)(addresses->addrs[i].addr); args.addr_len = (size_t)addresses->addrs[i].len; - subchannels[i] = grpc_subchannel_factory_create_subchannel( + grpc_subchannel *subchannel = grpc_subchannel_factory_create_subchannel( exec_ctx, r->subchannel_factory, &args); + if (subchannel != NULL) { + subchannels[naddrs++] = subchannel; + } } memset(&lb_policy_args, 0, sizeof(lb_policy_args)); lb_policy_args.subchannels = subchannels; - lb_policy_args.num_subchannels = addresses->naddrs; + lb_policy_args.num_subchannels = naddrs; lb_policy = grpc_lb_policy_create(r->lb_policy_name, &lb_policy_args); - grpc_client_config_set_lb_policy(config, lb_policy); - GRPC_LB_POLICY_UNREF(exec_ctx, lb_policy, "construction"); + if (lb_policy != NULL) { + grpc_client_config_set_lb_policy(config, lb_policy); + GRPC_LB_POLICY_UNREF(exec_ctx, lb_policy, "construction"); + } grpc_resolved_addresses_destroy(addresses); gpr_free(subchannels); + } else { + int retry_seconds = 15; + gpr_log(GPR_DEBUG, "dns resolution failed: retrying in %d seconds", + retry_seconds); + GPR_ASSERT(!r->have_retry_timer); + r->have_retry_timer = true; + gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); + GRPC_RESOLVER_REF(&r->base, "retry-timer"); + grpc_timer_init( + exec_ctx, &r->retry_timer, + gpr_time_add(now, gpr_time_from_seconds(retry_seconds, GPR_TIMESPAN)), + dns_on_retry_timer, r, now); } - gpr_mu_lock(&r->mu); - GPR_ASSERT(r->resolving); - r->resolving = 0; if (r->resolved_config) { grpc_client_config_unref(exec_ctx, r->resolved_config); } diff --git a/test/core/client_config/resolvers/dns_resolver_connectivity_test.c b/test/core/client_config/resolvers/dns_resolver_connectivity_test.c index e4d83539fb9..75d1eb674f6 100644 --- a/test/core/client_config/resolvers/dns_resolver_connectivity_test.c +++ b/test/core/client_config/resolvers/dns_resolver_connectivity_test.c @@ -39,6 +39,7 @@ #include #include "src/core/iomgr/resolve_address.h" +#include "src/core/iomgr/timer.h" #include "test/core/util/test_config.h" static void subchannel_factory_ref(grpc_subchannel_factory *scv) {} @@ -47,7 +48,7 @@ static void subchannel_factory_unref(grpc_exec_ctx *exec_ctx, static grpc_subchannel *subchannel_factory_create_subchannel( grpc_exec_ctx *exec_ctx, grpc_subchannel_factory *factory, grpc_subchannel_args *args) { - GPR_UNREACHABLE_CODE(return NULL); + return NULL; } static const grpc_subchannel_factory_vtable sc_vtable = { @@ -96,6 +97,20 @@ static void on_done(grpc_exec_ctx *exec_ctx, void *ev, bool success) { gpr_event_set(ev, (void *)1); } +// interleave waiting for an event with a timer check +static bool wait_loop(int deadline_seconds, gpr_event *ev) { + while (deadline_seconds) { + gpr_log(GPR_DEBUG, "Test: waiting for %d more seconds", deadline_seconds); + if (gpr_event_wait(ev, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(1))) return true; + deadline_seconds--; + + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + grpc_timer_check(&exec_ctx, gpr_now(GPR_CLOCK_MONOTONIC), NULL); + grpc_exec_ctx_finish(&exec_ctx); + } + return false; +} + int main(int argc, char **argv) { grpc_test_init(argc, argv); @@ -113,7 +128,7 @@ int main(int argc, char **argv) { grpc_resolver_next(&exec_ctx, resolver, &config, grpc_closure_create(on_done, &ev1)); grpc_exec_ctx_flush(&exec_ctx); - GPR_ASSERT(gpr_event_wait(&ev1, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5))); + GPR_ASSERT(wait_loop(5, &ev1)); GPR_ASSERT(config == NULL); gpr_event ev2; @@ -121,7 +136,7 @@ int main(int argc, char **argv) { grpc_resolver_next(&exec_ctx, resolver, &config, grpc_closure_create(on_done, &ev2)); grpc_exec_ctx_flush(&exec_ctx); - GPR_ASSERT(gpr_event_wait(&ev2, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5))); + GPR_ASSERT(wait_loop(30, &ev2)); GPR_ASSERT(config != NULL); grpc_client_config_unref(&exec_ctx, config); From e2327dbb8e45936c81802119142a7cda28230f24 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 09:52:42 -0800 Subject: [PATCH 25/60] Fit and finish for dns retry loop --- build.yaml | 1 + src/core/client_config/resolvers/dns_resolver.c | 8 ++++++-- tools/run_tests/tests.json | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/build.yaml b/build.yaml index 7cafb0ac6fe..e003dd83396 100644 --- a/build.yaml +++ b/build.yaml @@ -1058,6 +1058,7 @@ targets: - gpr_test_util - gpr - name: dns_resolver_connectivity_test + cpu_cost: 0.1 build: test language: c src: diff --git a/src/core/client_config/resolvers/dns_resolver.c b/src/core/client_config/resolvers/dns_resolver.c index 55457647b3c..e28e4757a10 100644 --- a/src/core/client_config/resolvers/dns_resolver.c +++ b/src/core/client_config/resolvers/dns_resolver.c @@ -95,6 +95,9 @@ static const grpc_resolver_vtable dns_resolver_vtable = { static void dns_shutdown(grpc_exec_ctx *exec_ctx, grpc_resolver *resolver) { dns_resolver *r = (dns_resolver *)resolver; gpr_mu_lock(&r->mu); + if (r->have_retry_timer) { + grpc_timer_cancel(exec_ctx, &r->retry_timer); + } if (r->next_completion != NULL) { *r->target_config = NULL; grpc_exec_ctx_enqueue(exec_ctx, r->next_completion, true, NULL); @@ -133,13 +136,14 @@ static void dns_on_retry_timer(grpc_exec_ctx *exec_ctx, void *arg, bool success) { dns_resolver *r = arg; + gpr_mu_lock(&r->mu); + r->have_retry_timer = false; if (success) { - gpr_mu_lock(&r->mu); if (!r->resolving) { dns_start_resolving_locked(r); } - gpr_mu_unlock(&r->mu); } + gpr_mu_unlock(&r->mu); GRPC_RESOLVER_UNREF(exec_ctx, &r->base, "retry-timer"); } diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index c2d6cadd530..724cd64d19c 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -261,7 +261,7 @@ "posix", "windows" ], - "cpu_cost": 1.0, + "cpu_cost": 0.1, "exclude_configs": [], "flaky": false, "gtest": false, From b18fe559629cea0c2a6d345449071cfb6c31bdc1 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 10:23:16 -0800 Subject: [PATCH 26/60] Update copyrights --- src/core/client_config/client_config.c | 2 +- src/core/iomgr/resolve_address.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/client_config/client_config.c b/src/core/client_config/client_config.c index 25f4ac25177..c500af25eec 100644 --- a/src/core/client_config/client_config.c +++ b/src/core/client_config/client_config.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/core/iomgr/resolve_address.h b/src/core/iomgr/resolve_address.h index 508e161a7e0..b0596304576 100644 --- a/src/core/iomgr/resolve_address.h +++ b/src/core/iomgr/resolve_address.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without From 9aa4f1b062686e01c2e1dfb792e5ec9c3a91e1c9 Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 2 Mar 2016 16:39:13 -0800 Subject: [PATCH 27/60] fix windows leaks --- src/core/iomgr/iocp_windows.c | 14 +++++++++----- src/core/iomgr/iocp_windows.h | 11 +++++++++-- src/core/iomgr/tcp_server_windows.c | 3 +-- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/core/iomgr/iocp_windows.c b/src/core/iomgr/iocp_windows.c index 807729708ea..fa87e5246b0 100644 --- a/src/core/iomgr/iocp_windows.c +++ b/src/core/iomgr/iocp_windows.c @@ -71,7 +71,8 @@ static DWORD deadline_to_millis_timeout(gpr_timespec deadline, timeout, gpr_time_from_nanos(GPR_NS_PER_MS - 1, GPR_TIMESPAN))); } -void grpc_iocp_work(grpc_exec_ctx *exec_ctx, gpr_timespec deadline) { +grpc_iocp_work_status grpc_iocp_work(grpc_exec_ctx *exec_ctx, + gpr_timespec deadline) { BOOL success; DWORD bytes = 0; DWORD flags = 0; @@ -84,14 +85,14 @@ void grpc_iocp_work(grpc_exec_ctx *exec_ctx, gpr_timespec deadline) { g_iocp, &bytes, &completion_key, &overlapped, deadline_to_millis_timeout(deadline, gpr_now(deadline.clock_type))); if (success == 0 && overlapped == NULL) { - return; + return GRPC_IOCP_WORK_TIMEOUT; } GPR_ASSERT(completion_key && overlapped); if (overlapped == &g_iocp_custom_overlap) { gpr_atm_full_fetch_add(&g_custom_events, -1); if (completion_key == (ULONG_PTR)&g_iocp_kick_token) { /* We were awoken from a kick. */ - return; + return GRPC_IOCP_WORK_KICK; } gpr_log(GPR_ERROR, "Unknown custom completion key."); abort(); @@ -121,6 +122,7 @@ void grpc_iocp_work(grpc_exec_ctx *exec_ctx, gpr_timespec deadline) { } gpr_mu_unlock(&socket->state_mu); grpc_exec_ctx_enqueue(exec_ctx, closure, true, NULL); + return GRPC_IOCP_WORK_WORK; } void grpc_iocp_init(void) { @@ -140,10 +142,12 @@ void grpc_iocp_kick(void) { void grpc_iocp_flush(void) { grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + grpc_iocp_work_status work_status; do { - grpc_iocp_work(&exec_ctx, gpr_inf_past(GPR_CLOCK_MONOTONIC)); - } while (grpc_exec_ctx_flush(&exec_ctx)); + work_status = grpc_iocp_work(&exec_ctx, gpr_inf_past(GPR_CLOCK_MONOTONIC)); + } while (work_status == GRPC_IOCP_WORK_KICK || + grpc_exec_ctx_flush(&exec_ctx)); } void grpc_iocp_shutdown(void) { diff --git a/src/core/iomgr/iocp_windows.h b/src/core/iomgr/iocp_windows.h index 75f3ba84770..8b2b1aeb5c2 100644 --- a/src/core/iomgr/iocp_windows.h +++ b/src/core/iomgr/iocp_windows.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -38,7 +38,14 @@ #include "src/core/iomgr/socket_windows.h" -void grpc_iocp_work(grpc_exec_ctx *exec_ctx, gpr_timespec deadline); +typedef enum { + GRPC_IOCP_WORK_WORK, + GRPC_IOCP_WORK_TIMEOUT, + GRPC_IOCP_WORK_KICK +} grpc_iocp_work_status; + +grpc_iocp_work_status grpc_iocp_work(grpc_exec_ctx *exec_ctx, + gpr_timespec deadline); void grpc_iocp_init(void); void grpc_iocp_kick(void); void grpc_iocp_flush(void); diff --git a/src/core/iomgr/tcp_server_windows.c b/src/core/iomgr/tcp_server_windows.c index ce930b8f41d..a4abc5b9743 100644 --- a/src/core/iomgr/tcp_server_windows.c +++ b/src/core/iomgr/tcp_server_windows.c @@ -240,8 +240,7 @@ static void decrement_active_ports_and_notify(grpc_exec_ctx *exec_ctx, sp->shutting_down = 0; gpr_mu_lock(&sp->server->mu); GPR_ASSERT(sp->server->active_ports > 0); - if (0 == --sp->server->active_ports && - sp->server->shutdown_complete != NULL) { + if (0 == --sp->server->active_ports) { notify = 1; } gpr_mu_unlock(&sp->server->mu); From 9ee163557bda7a86e33c12626cf160342796014a Mon Sep 17 00:00:00 2001 From: Masood Malekghassemi Date: Fri, 11 Mar 2016 11:13:59 -0800 Subject: [PATCH 28/60] Remove unnecessary (potentially bad) safety-refs --- src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi | 1 - src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi | 1 - 2 files changed, 2 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi index 1f1833d5ecc..bc03c4dcf19 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi @@ -95,7 +95,6 @@ cdef class Channel: self, grpc_connectivity_state last_observed_state, Timespec deadline not None, CompletionQueue queue not None, tag): cdef OperationTag operation_tag = OperationTag(tag) - operation_tag.references = [self, queue] cpython.Py_INCREF(operation_tag) grpc_channel_watch_connectivity_state( self.c_channel, last_observed_state, deadline.c_time, diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi index fe93da6c124..5f859235246 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi @@ -106,7 +106,6 @@ cdef class Server: self.is_shutting_down = True operation_tag = OperationTag(tag) operation_tag.shutting_down_server = self - operation_tag.references.extend([self, queue]) cpython.Py_INCREF(operation_tag) grpc_server_shutdown_and_notify( self.c_server, queue.c_completion_queue, From a41940d0e0dc384318e6ea116e4ff8e10f7747be Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Fri, 11 Mar 2016 20:31:32 +0100 Subject: [PATCH 29/60] Disabling check_version. --- tools/run_tests/sanity/sanity_tests.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/run_tests/sanity/sanity_tests.yaml b/tools/run_tests/sanity/sanity_tests.yaml index cffc180fb07..3840f4d8df6 100644 --- a/tools/run_tests/sanity/sanity_tests.yaml +++ b/tools/run_tests/sanity/sanity_tests.yaml @@ -2,7 +2,6 @@ - script: tools/run_tests/sanity/check_cache_mk.sh - script: tools/run_tests/sanity/check_sources_and_headers.py - script: tools/run_tests/sanity/check_submodules.sh -- script: tools/run_tests/sanity/check_version.py - script: tools/buildgen/generate_projects.sh -j 3 cpu_cost: 3 - script: tools/distrib/check_copyright.py From f1d084a4d159eb07536e4b24f2bf65a3771a747c Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 30 Oct 2015 11:14:09 -0700 Subject: [PATCH 30/60] Add monitoring of connectivity Move C-layer call creation into GRPCChannel, so that it always acts on a non-destroyed channel. Listen for connectivity in the GRPCCall, and destroy the channel. --- src/objective-c/GRPCClient/GRPCCall.m | 37 +++- .../GRPCClient/private/GRPCChannel.h | 3 + .../GRPCClient/private/GRPCChannel.m | 14 ++ .../private/GRPCConnectivityMonitor.h | 77 +++++++ .../private/GRPCConnectivityMonitor.m | 188 ++++++++++++++++++ src/objective-c/GRPCClient/private/GRPCHost.h | 26 ++- src/objective-c/GRPCClient/private/GRPCHost.m | 93 ++++----- .../GRPCReachabilityFlagNames.xmacro.h | 65 ++++++ .../GRPCClient/private/GRPCWrappedCall.h | 2 +- 9 files changed, 445 insertions(+), 60 deletions(-) create mode 100644 src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.h create mode 100644 src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.m create mode 100644 src/objective-c/GRPCClient/private/GRPCReachabilityFlagNames.xmacro.h diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index f79b7d0bc0c..2d45818b6e9 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -37,6 +37,8 @@ #include #import +#import "private/GRPCConnectivityMonitor.h" +#import "private/GRPCHost.h" #import "private/GRPCRequestHeaders.h" #import "private/GRPCWrappedCall.h" #import "private/NSData+GRPC.h" @@ -71,8 +73,11 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; @implementation GRPCCall { dispatch_queue_t _callQueue; + NSString *_host; + NSString *_path; GRPCWrappedCall *_wrappedCall; dispatch_once_t _callAlreadyInvoked; + GRPCConnectivityMonitor *_connectivityMonitor; // The C gRPC library has less guarantees on the ordering of events than we // do. Particularly, in the face of errors, there's no ordering guarantee at @@ -115,13 +120,11 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; format:@"The requests writer can't be already started."]; } if ((self = [super init])) { - _wrappedCall = [[GRPCWrappedCall alloc] initWithHost:host path:path]; - if (!_wrappedCall) { - return nil; - } + _host = [host copy]; + _path = [path copy]; // Serial queue to invoke the non-reentrant methods of the grpc_call object. - _callQueue = dispatch_queue_create("org.grpc.call", NULL); + _callQueue = dispatch_queue_create("io.grpc.call", NULL); _requestWriter = requestWriter; @@ -156,7 +159,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; - (void)cancel { [self finishWithError:[NSError errorWithDomain:kGRPCErrorDomain code:GRPCErrorCodeCancelled - userInfo:nil]]; + userInfo:@{NSLocalizedDescriptionKey: @"Canceled by app"}]]; [self cancelCall]; } @@ -354,8 +357,29 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; _retainSelf = self; _responseWriteable = [[GRXConcurrentWriteable alloc] initWithWriteable:writeable]; + + _wrappedCall = [[GRPCWrappedCall alloc] initWithHost:_host path:_path]; + NSAssert(_wrappedCall, @"Error allocating RPC objects. Low memory?"); + [self sendHeaders:_requestHeaders]; [self invokeCall]; + // TODO(jcanizales): Extract this logic somewhere common. + NSString *host = [NSURL URLWithString:[@"https://" stringByAppendingString:_host]].host; + if (!host) { + // TODO(jcanizales): Check this on init. + [NSException raise:NSInvalidArgumentException format:@"host of %@ is nil", _host]; + } + __weak typeof(self) weakSelf = self; + _connectivityMonitor = [GRPCConnectivityMonitor monitorWithHost:host]; + [_connectivityMonitor handleLossWithHandler:^{ + typeof(self) strongSelf = weakSelf; + if (strongSelf) { + [strongSelf finishWithError:[NSError errorWithDomain:kGRPCErrorDomain + code:GRPCErrorCodeUnavailable + userInfo:@{NSLocalizedDescriptionKey: @"Connectivity lost."}]]; + [[GRPCHost hostWithAddress:strongSelf->_host] disconnect]; + } + }]; } - (void)setState:(GRXWriterState)newState { @@ -385,4 +409,5 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; return; } } + @end diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.h b/src/objective-c/GRPCClient/private/GRPCChannel.h index 8661ae6f972..e49a6aca297 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCChannel.h @@ -35,6 +35,7 @@ #include +@class GRPCCompletionQueue; struct grpc_channel_credentials; @@ -80,4 +81,6 @@ struct grpc_channel_credentials; + (nonnull GRPCChannel *)insecureChannelWithHost:(nonnull NSString *)host channelArgs:(nullable NSDictionary *)channelArgs; +- (nullable grpc_call *)unmanagedCallWithPath:(nonnull NSString *)path + completionQueue:(nonnull GRPCCompletionQueue *)queue; @end diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.m b/src/objective-c/GRPCClient/private/GRPCChannel.m index 7e55a473d74..d7de025e21d 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCChannel.m @@ -38,6 +38,8 @@ #include #include +#import "GRPCCompletionQueue.h" + /** * Returns @c grpc_channel_credentials from the specified @c path. If the file at the path could not * be read then NULL is returned. If NULL is returned, @c errorPtr may not be NULL if there are @@ -205,4 +207,16 @@ grpc_channel_args * buildChannelArgs(NSDictionary *dictionary) { channelArgs:channelArgs]; } +- (grpc_call *)unmanagedCallWithPath:(NSString *)path + completionQueue:(GRPCCompletionQueue *)queue { + return grpc_channel_create_call(_unmanagedChannel, + NULL, GRPC_PROPAGATE_DEFAULTS, + queue.unmanagedQueue, + path.UTF8String, + // Get "host" from "host:port" + // TODO(jcanizales): Use NSURLs throughout, to clarify these. + [_host componentsSeparatedByString:@":"][0].UTF8String, + gpr_inf_future(GPR_CLOCK_REALTIME), NULL); +} + @end diff --git a/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.h b/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.h new file mode 100644 index 00000000000..d9d6c22ae9d --- /dev/null +++ b/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.h @@ -0,0 +1,77 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#import +#import + +@interface GRPCReachabilityFlags : NSObject + ++ (nonnull instancetype)flagsWithFlags:(SCNetworkReachabilityFlags)flags; + +/** + * One accessor method to query each of the different flags. Example: + +@property(nonatomic, readonly) BOOL isCell; + + */ +#define GRPC_XMACRO_ITEM(methodName, FlagName) \ +@property(nonatomic, readonly) BOOL methodName; + +#include "GRPCReachabilityFlagNames.xmacro.h" +#undef GRPC_XMACRO_ITEM + +@property(nonatomic, readonly) BOOL isHostReachable; +@end + + +@interface GRPCConnectivityMonitor : NSObject + ++ (nullable instancetype)monitorWithHost:(nonnull NSString *)hostName; + +- (nonnull instancetype)init NS_UNAVAILABLE; + +/** + * Queue on which callbacks will be dispatched. Default is the main queue. Set it before calling + * handleLossWithHandler:. + */ +// TODO(jcanizales): Default to a serial background queue instead. +@property(nonatomic, strong, null_resettable) dispatch_queue_t queue; + +/** + * Calls handler the next time the connectivity to this instance's host is lost. If this instance is + * released before that happens, the handler won't be called. + * Only one handler is active at a time, so if this method is called again before the previous + * handler has been called, it might never be called at all (or yes, if it has already been queued). + */ +- (void)handleLossWithHandler:(nonnull void (^)())handler; +@end diff --git a/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.m b/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.m new file mode 100644 index 00000000000..f66a77c9b20 --- /dev/null +++ b/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.m @@ -0,0 +1,188 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#import "GRPCConnectivityMonitor.h" + +#pragma mark Flags + +@implementation GRPCReachabilityFlags { + SCNetworkReachabilityFlags _flags; +} + ++ (instancetype)flagsWithFlags:(SCNetworkReachabilityFlags)flags { + return [[self alloc] initWithFlags:flags]; +} + +- (instancetype)initWithFlags:(SCNetworkReachabilityFlags)flags { + if ((self = [super init])) { + _flags = flags; + } + return self; +} + +/* + * One accessor method implementation per flag. Example: + +- (BOOL)isCell { \ + return !!(_flags & kSCNetworkReachabilityFlagsIsWWAN); \ +} + + */ +#define GRPC_XMACRO_ITEM(methodName, FlagName) \ +- (BOOL)methodName { \ + return !!(_flags & kSCNetworkReachabilityFlags ## FlagName); \ +} +#include "GRPCReachabilityFlagNames.xmacro.h" +#undef GRPC_XMACRO_ITEM + +- (BOOL)isHostReachable { + // Note: connectionOnDemand means it'll be reachable only if using the CFSocketStream API or APIs + // on top of it. + // connectionRequired means we can't tell until a connection is attempted (e.g. for VPN on + // demand). + return self.reachable && !self.interventionRequired && !self.connectionOnDemand; +} + +- (NSString *)description { + NSMutableArray *activeOptions = [NSMutableArray arrayWithCapacity:9]; + + /* + * For each flag, add its name to the array if it's ON. Example: + + if (self.isCell) { + [activeOptions addObject:@"isCell"]; + } + + */ +#define GRPC_XMACRO_ITEM(methodName, FlagName) \ + if (self.methodName) { \ + [activeOptions addObject:@#methodName]; \ + } +#include "GRPCReachabilityFlagNames.xmacro.h" +#undef GRPC_XMACRO_ITEM + + return activeOptions.count == 0 ? @"(none)" : [activeOptions componentsJoinedByString:@", "]; +} + +- (BOOL)isEqual:(id)object { + return [object isKindOfClass:[GRPCReachabilityFlags class]] && + _flags == ((GRPCReachabilityFlags *)object)->_flags; +} + +- (NSUInteger)hash { + return _flags; +} +@end + +#pragma mark Connectivity Monitor + +// Assumes the third argument is a block that accepts SCNetworkReachabilityFlags, and passes the +// received ones to it. +static void PassFlagsToContextInfoBlock(SCNetworkReachabilityRef target, + SCNetworkReachabilityFlags flags, + void *info) { + #pragma unused (target) + ((__bridge void (^)(SCNetworkReachabilityFlags))info)(flags); +} + +@implementation GRPCConnectivityMonitor { + SCNetworkReachabilityRef _reachabilityRef; +} + +- (nullable instancetype)initWithReachability:(nullable SCNetworkReachabilityRef)reachability { + if (!reachability) { + return nil; + } + if ((self = [super init])) { + _reachabilityRef = CFRetain(reachability); + _queue = dispatch_get_main_queue(); + } + return self; +} + ++ (nullable instancetype)monitorWithHost:(nonnull NSString *)host { + const char *hostName = host.UTF8String; + if (!hostName) { + [NSException raise:NSInvalidArgumentException + format:@"host.UTF8String returns NULL for %@", host]; + } + SCNetworkReachabilityRef reachability = + SCNetworkReachabilityCreateWithName(NULL, hostName); + + GRPCConnectivityMonitor *returnValue = [[self alloc] initWithReachability:reachability]; + if (reachability) { + CFRelease(reachability); + } + return returnValue; +} + +- (void)handleLossWithHandler:(void (^)())handler { + __weak typeof(self) weakSelf = self; + [self startListeningWithHandler:^(GRPCReachabilityFlags *flags) { + if (!flags.isHostReachable) { + [weakSelf stopListening]; + handler(); + } + }]; +} + +- (void)startListeningWithHandler:(void (^)(GRPCReachabilityFlags *))handler { + SCNetworkReachabilityContext context = { + .version = 0, + .info = (__bridge_retained void *)^(SCNetworkReachabilityFlags flags){ + // Pass the flags as as Objective-C wrapper. + handler([[GRPCReachabilityFlags alloc] initWithFlags:flags]); + }, + .release = CFRelease + }; + SCNetworkReachabilitySetCallback(_reachabilityRef, PassFlagsToContextInfoBlock, &context); + SCNetworkReachabilitySetDispatchQueue(_reachabilityRef, _queue); +} + +- (void)stopListening { + SCNetworkReachabilitySetCallback(_reachabilityRef, NULL, NULL); + SCNetworkReachabilitySetDispatchQueue(_reachabilityRef, NULL); +} + +- (void)setQueue:(dispatch_queue_t)queue { + _queue = queue ?: dispatch_get_main_queue(); +} + +- (void)dealloc { + if (_reachabilityRef) { + [self stopListening]; + CFRelease(_reachabilityRef); + } +} + +@end diff --git a/src/objective-c/GRPCClient/private/GRPCHost.h b/src/objective-c/GRPCClient/private/GRPCHost.h index 82c0ad6cf63..987d3e9f597 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.h +++ b/src/objective-c/GRPCClient/private/GRPCHost.h @@ -33,27 +33,39 @@ #import +NS_ASSUME_NONNULL_BEGIN + @class GRPCCompletionQueue; struct grpc_call; @interface GRPCHost : NSObject @property(nonatomic, readonly) NSString *address; -@property(nonatomic, copy) NSString *userAgentPrefix; +@property(nonatomic, copy, nullable) NSString *userAgentPrefix; /** The following properties should only be modified for testing: */ @property(nonatomic, getter=isSecure) BOOL secure; -@property(nonatomic, copy) NSString *pathToCertificates; -@property(nonatomic, copy) NSString *hostNameOverride; +@property(nonatomic, copy, nullable) NSString *pathToCertificates; +@property(nonatomic, copy, nullable) NSString *hostNameOverride; +- (nullable instancetype)init NS_UNAVAILABLE; /** Host objects initialized with the same address are the same. */ -+ (instancetype)hostWithAddress:(NSString *)address; -- (instancetype)initWithAddress:(NSString *)address NS_DESIGNATED_INITIALIZER; ++ (nullable instancetype)hostWithAddress:(NSString *)address; +- (nullable instancetype)initWithAddress:(NSString *)address NS_DESIGNATED_INITIALIZER; /** Create a grpc_call object to the provided path on this host. */ -- (struct grpc_call *)unmanagedCallWithPath:(NSString *)path - completionQueue:(GRPCCompletionQueue *)queue; +- (nullable struct grpc_call *)unmanagedCallWithPath:(NSString *)path + completionQueue:(GRPCCompletionQueue *)queue; +// TODO: There's a race when a new RPC is coming through just as an existing one is getting +// notified that there's no connectivity. If connectivity comes back at that moment, the new RPC +// will have its channel destroyed by the other RPC, and will never get notified of a problem, so +// it'll hang (the C layer logs a timeout, with exponential back off). One solution could be to pass +// the GRPCChannel to the GRPCCall, renaming this as "disconnectChannel:channel", which would only +// act on that specific channel. +- (void)disconnect; @end + +NS_ASSUME_NONNULL_END diff --git a/src/objective-c/GRPCClient/private/GRPCHost.m b/src/objective-c/GRPCClient/private/GRPCHost.m index eb1db899b70..b6a6d48d964 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.m +++ b/src/objective-c/GRPCClient/private/GRPCHost.m @@ -34,33 +34,30 @@ #import "GRPCHost.h" #include +#import #import #import "GRPCChannel.h" #import "GRPCCompletionQueue.h" #import "NSDictionary+GRPC.h" +NS_ASSUME_NONNULL_BEGIN + // TODO(jcanizales): Generate the version in a standalone header, from templates. Like // templates/src/core/surface/version.c.template . #define GRPC_OBJC_VERSION_STRING @"0.13.0" -@interface GRPCHost () -// TODO(mlumish): Investigate whether caching channels with strong links is a good idea. -@property(nonatomic, strong) GRPCChannel *channel; -@end - -@implementation GRPCHost - -+ (instancetype)hostWithAddress:(NSString *)address { - return [[self alloc] initWithAddress:address]; +@implementation GRPCHost { + // TODO(mlumish): Investigate whether caching channels with strong links is a good idea. + GRPCChannel *_channel; } -- (instancetype)init { - return [self initWithAddress:nil]; ++ (nullable instancetype)hostWithAddress:(NSString *)address { + return [[self alloc] initWithAddress:address]; } // Default initializer. -- (instancetype)initWithAddress:(NSString *)address { +- (nullable instancetype)initWithAddress:(NSString *)address { if (!address) { return nil; } @@ -95,46 +92,42 @@ return self; } -- (grpc_call *)unmanagedCallWithPath:(NSString *)path completionQueue:(GRPCCompletionQueue *)queue { - if (!queue || !path || !self.channel) { - return NULL; +- (nullable grpc_call *)unmanagedCallWithPath:(NSString *)path + completionQueue:(GRPCCompletionQueue *)queue { + @synchronized(self) { + if (!_channel) { + _channel = [self newChannel]; + } + return [_channel unmanagedCallWithPath:path completionQueue:queue]; } - return grpc_channel_create_call(self.channel.unmanagedChannel, - NULL, GRPC_PROPAGATE_DEFAULTS, - queue.unmanagedQueue, - path.UTF8String, - self.hostName.UTF8String, - gpr_inf_future(GPR_CLOCK_REALTIME), NULL); } -- (GRPCChannel *)channel { - // Create it lazily, because we don't want to open a connection just because someone is - // configuring a host. +- (NSDictionary *)channelArgs { + NSMutableDictionary *args = [NSMutableDictionary dictionary]; - if (!_channel) { - NSMutableDictionary *args = [NSMutableDictionary dictionary]; + // TODO(jcanizales): Add OS and device information (see + // https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#user-agents ). + NSString *userAgent = @"grpc-objc/" GRPC_OBJC_VERSION_STRING; + if (_userAgentPrefix) { + userAgent = [_userAgentPrefix stringByAppendingFormat:@" %@", userAgent]; + } + args[@GRPC_ARG_PRIMARY_USER_AGENT_STRING] = userAgent; - // TODO(jcanizales): Add OS and device information (see - // https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#user-agents ). - NSString *userAgent = @"grpc-objc/" GRPC_OBJC_VERSION_STRING; - if (_userAgentPrefix) { - userAgent = [@[_userAgentPrefix, userAgent] componentsJoinedByString:@" "]; - } - args[@GRPC_ARG_PRIMARY_USER_AGENT_STRING] = userAgent; - - if (_secure) { - if (_hostNameOverride) { - args[@GRPC_SSL_TARGET_NAME_OVERRIDE_ARG] = _hostNameOverride; - } - - _channel = [GRPCChannel secureChannelWithHost:_address - pathToCertificates:_pathToCertificates - channelArgs:args]; - } else { - _channel = [GRPCChannel insecureChannelWithHost:_address channelArgs:args]; - } + if (_secure && _hostNameOverride) { + args[@GRPC_SSL_TARGET_NAME_OVERRIDE_ARG] = _hostNameOverride; + } + return args; +} + +- (GRPCChannel *)newChannel { + NSDictionary *args = [self channelArgs]; + if (_secure) { + return [GRPCChannel secureChannelWithHost:_address + pathToCertificates:_pathToCertificates + channelArgs:args]; + } else { + return [GRPCChannel insecureChannelWithHost:_address channelArgs:args]; } - return _channel; } - (NSString *)hostName { @@ -142,7 +135,15 @@ return _hostNameOverride ?: _address; } +- (void)disconnect { + @synchronized(self) { + _channel = nil; + } +} + // TODO(jcanizales): Don't let set |secure| to |NO| if |pathToCertificates| or |hostNameOverride| // have been set. Don't let set either of the latter if |secure| has been set to |NO|. @end + +NS_ASSUME_NONNULL_END diff --git a/src/objective-c/GRPCClient/private/GRPCReachabilityFlagNames.xmacro.h b/src/objective-c/GRPCClient/private/GRPCReachabilityFlagNames.xmacro.h new file mode 100644 index 00000000000..02871d5d028 --- /dev/null +++ b/src/objective-c/GRPCClient/private/GRPCReachabilityFlagNames.xmacro.h @@ -0,0 +1,65 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +/** + * "X-macro" file that lists the flags names of Apple's Network Reachability API, along with a nice + * Objective-C method name used to query each of them. + * + * Example usage: To generate a dictionary from flag value to name, one can do: + + NSDictionary *flagNames = @{ +#define GRPC_XMACRO_ITEM(methodName, FlagName) \ + @(kSCNetworkReachabilityFlags ## FlagName): @#methodName, +#include "GRXReachabilityFlagNames.xmacro.h" +#undef GRPC_XMACRO_ITEM + }; + + XCTAssertEqualObjects(flagNames[@(kSCNetworkReachabilityFlagsIsWWAN)], @"isCell"); + + */ + +#ifndef GRPC_XMACRO_ITEM +#error This file is to be used with the "X-macro" pattern: Please #define \ + GRPC_XMACRO_ITEM(methodName, FlagName), then #include this file, and then #undef \ + GRPC_XMACRO_ITEM. +#endif + +GRPC_XMACRO_ITEM(isCell, IsWWAN) +GRPC_XMACRO_ITEM(reachable, Reachable) +GRPC_XMACRO_ITEM(transientConnection, TransientConnection) +GRPC_XMACRO_ITEM(connectionRequired, ConnectionRequired) +GRPC_XMACRO_ITEM(connectionOnTraffic, ConnectionOnTraffic) +GRPC_XMACRO_ITEM(interventionRequired, InterventionRequired) +GRPC_XMACRO_ITEM(connectionOnDemand, ConnectionOnDemand) +GRPC_XMACRO_ITEM(isLocalAddress, IsLocalAddress) +GRPC_XMACRO_ITEM(isDirect, IsDirect) diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h index 71e7e0e54ee..e37ed1b59fb 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h @@ -34,7 +34,6 @@ #import #include -#import "GRPCChannel.h" #import "GRPCRequestHeaders.h" @interface GRPCOperation : NSObject @@ -94,4 +93,5 @@ - (void)startBatchWithOperations:(NSArray *)ops; - (void)cancel; + @end From 07a2a70bca9a2ffa3c69b43a49f29c4d52808c98 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 11 Mar 2016 10:49:56 -0800 Subject: [PATCH 31/60] Not call external methods from within a critical section --- src/objective-c/GRPCClient/private/GRPCHost.m | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/objective-c/GRPCClient/private/GRPCHost.m b/src/objective-c/GRPCClient/private/GRPCHost.m index b6a6d48d964..508cb206444 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.m +++ b/src/objective-c/GRPCClient/private/GRPCHost.m @@ -94,12 +94,15 @@ NS_ASSUME_NONNULL_BEGIN - (nullable grpc_call *)unmanagedCallWithPath:(NSString *)path completionQueue:(GRPCCompletionQueue *)queue { + GRPCChannel *channel; + // This is racing -[GRPCHost disconnect]. @synchronized(self) { if (!_channel) { _channel = [self newChannel]; } - return [_channel unmanagedCallWithPath:path completionQueue:queue]; + channel = _channel; } + return [channel unmanagedCallWithPath:path completionQueue:queue]; } - (NSDictionary *)channelArgs { @@ -136,6 +139,7 @@ NS_ASSUME_NONNULL_BEGIN } - (void)disconnect { + // This is racing -[GRPCHost unmanagedCallWithPath:completionQueue:]. @synchronized(self) { _channel = nil; } From 8b414f184db9295e30164140d54357c6ad310368 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 11 Mar 2016 10:28:40 -0800 Subject: [PATCH 32/60] Clean up ownership of the connection loss handler --- .../private/GRPCConnectivityMonitor.h | 2 +- .../private/GRPCConnectivityMonitor.m | 22 +++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.h b/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.h index d9d6c22ae9d..2fae4103315 100644 --- a/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.h +++ b/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.h @@ -68,7 +68,7 @@ @property(nonatomic, strong, null_resettable) dispatch_queue_t queue; /** - * Calls handler the next time the connectivity to this instance's host is lost. If this instance is + * Calls handler every time the connectivity to this instance's host is lost. If this instance is * released before that happens, the handler won't be called. * Only one handler is active at a time, so if this method is called again before the previous * handler has been called, it might never be called at all (or yes, if it has already been queued). diff --git a/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.m b/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.m index f66a77c9b20..b4061bd5ef5 100644 --- a/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.m +++ b/src/objective-c/GRPCClient/private/GRPCConnectivityMonitor.m @@ -106,13 +106,16 @@ #pragma mark Connectivity Monitor -// Assumes the third argument is a block that accepts SCNetworkReachabilityFlags, and passes the +// Assumes the third argument is a block that accepts a GRPCReachabilityFlags object, and passes the // received ones to it. static void PassFlagsToContextInfoBlock(SCNetworkReachabilityRef target, SCNetworkReachabilityFlags flags, void *info) { #pragma unused (target) - ((__bridge void (^)(SCNetworkReachabilityFlags))info)(flags); + // This can be called many times with the same info. The info is retained by SCNetworkReachability + // while this function is being executed. + void (^handler)(GRPCReachabilityFlags *) = (__bridge void (^)(GRPCReachabilityFlags *))info; + handler([[GRPCReachabilityFlags alloc] initWithFlags:flags]); } @implementation GRPCConnectivityMonitor { @@ -147,29 +150,30 @@ static void PassFlagsToContextInfoBlock(SCNetworkReachabilityRef target, } - (void)handleLossWithHandler:(void (^)())handler { - __weak typeof(self) weakSelf = self; [self startListeningWithHandler:^(GRPCReachabilityFlags *flags) { if (!flags.isHostReachable) { - [weakSelf stopListening]; handler(); } }]; } - (void)startListeningWithHandler:(void (^)(GRPCReachabilityFlags *))handler { + // Copy to ensure the handler block is in the heap (and so can't be deallocated when this method + // returns). + void (^copiedHandler)(GRPCReachabilityFlags *) = [handler copy]; SCNetworkReachabilityContext context = { .version = 0, - .info = (__bridge_retained void *)^(SCNetworkReachabilityFlags flags){ - // Pass the flags as as Objective-C wrapper. - handler([[GRPCReachabilityFlags alloc] initWithFlags:flags]); - }, - .release = CFRelease + .info = (__bridge void *)copiedHandler, + .retain = CFRetain, + .release = CFRelease, }; + // The following will retain context.info, and release it when the callback is set to NULL. SCNetworkReachabilitySetCallback(_reachabilityRef, PassFlagsToContextInfoBlock, &context); SCNetworkReachabilitySetDispatchQueue(_reachabilityRef, _queue); } - (void)stopListening { + // This releases the block on context.info. SCNetworkReachabilitySetCallback(_reachabilityRef, NULL, NULL); SCNetworkReachabilitySetDispatchQueue(_reachabilityRef, NULL); } From 232b6a8f191d9cbb905680dd2a95f3b1815d1eb7 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 11 Mar 2016 12:26:27 -0800 Subject: [PATCH 33/60] Test robustness of WriteableSingleHandler against non-single Writers --- src/objective-c/tests/RxLibraryUnitTests.m | 51 ++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/objective-c/tests/RxLibraryUnitTests.m b/src/objective-c/tests/RxLibraryUnitTests.m index d3426628141..ae9465f58cc 100644 --- a/src/objective-c/tests/RxLibraryUnitTests.m +++ b/src/objective-c/tests/RxLibraryUnitTests.m @@ -64,6 +64,8 @@ } @end +// TODO(jcanizales): Split into one file per tested class. + @interface RxLibraryUnitTests : XCTestCase @end @@ -79,6 +81,7 @@ // If: id writeable = [GRXWriteable writeableWithSingleHandler:handler.block]; [writeable writeValue:anyValue]; + [writeable writesFinishedWithError:nil]; // Then: XCTAssertEqual(handler.timesCalled, 1); @@ -101,6 +104,54 @@ XCTAssertEqualObjects(handler.errorOrNil, anyError); } +- (void)testWriteableSingleHandlerIsCalledOnlyOnce_ValueThenError { + // Given: + CapturingSingleValueHandler *handler = [CapturingSingleValueHandler handler]; + id anyValue = @7; + NSError *anyError = [NSError errorWithDomain:@"domain" code:7 userInfo:nil]; + + // If: + id writeable = [GRXWriteable writeableWithSingleHandler:handler.block]; + [writeable writeValue:anyValue]; + [writeable writesFinishedWithError:anyError]; + + // Then: + XCTAssertEqual(handler.timesCalled, 1); + XCTAssertEqualObjects(handler.value, anyValue); + XCTAssertEqualObjects(handler.errorOrNil, nil); +} + +- (void)testWriteableSingleHandlerIsCalledOnlyOnce_ValueThenValue { + // Given: + CapturingSingleValueHandler *handler = [CapturingSingleValueHandler handler]; + id anyValue = @7; + + // If: + id writeable = [GRXWriteable writeableWithSingleHandler:handler.block]; + [writeable writeValue:anyValue]; + [writeable writeValue:anyValue]; + [writeable writesFinishedWithError:nil]; + + // Then: + XCTAssertEqual(handler.timesCalled, 1); + XCTAssertEqualObjects(handler.value, anyValue); + XCTAssertEqualObjects(handler.errorOrNil, nil); +} + +- (void)testWriteableSingleHandlerFailsOnEmptyWriter { + // Given: + CapturingSingleValueHandler *handler = [CapturingSingleValueHandler handler]; + + // If: + id writeable = [GRXWriteable writeableWithSingleHandler:handler.block]; + [writeable writesFinishedWithError:nil]; + + // Then: + XCTAssertEqual(handler.timesCalled, 1); + XCTAssertEqualObjects(handler.value, nil); + XCTAssertNotNil(handler.errorOrNil); +} + #pragma mark BufferedPipe - (void)testBufferedPipePropagatesValue { From 5e6723203013b365c2a60158578f73abf480b8bf Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 9 Mar 2016 13:34:28 -0800 Subject: [PATCH 34/60] Make Writeable with single handler robust against stream Writers --- src/objective-c/RxLibrary/GRXWriteable.m | 37 ++++++++++++++++++++---- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/objective-c/RxLibrary/GRXWriteable.m b/src/objective-c/RxLibrary/GRXWriteable.m index 2729d62b72f..b122c04ce0c 100644 --- a/src/objective-c/RxLibrary/GRXWriteable.m +++ b/src/objective-c/RxLibrary/GRXWriteable.m @@ -42,11 +42,38 @@ if (!handler) { return [[self alloc] init]; } - return [[self alloc] initWithValueHandler:^(id value) { - handler(value, nil); - } completionHandler:^(NSError *errorOrNil) { - if (errorOrNil) { - handler(nil, errorOrNil); + // We nilify this variable when the block is invoked, so that handler is only invoked once even if + // the writer tries to write multiple values. + __block GRXEventHandler eventHandler = ^(BOOL done, id value, NSError *error) { + // Nillify eventHandler before invoking handler, in case the latter causes the former to be + // executed recursively. Because blocks can be deallocated even during execution, we have to + // first retain handler locally to guarantee it's valid. + // TODO(jcanizales): Just turn this craziness into a simple subclass of GRXWriteable. + GRXSingleHandler singleHandler = handler; + eventHandler = nil; + + if (value) { + singleHandler(value, nil); + } else if (error) { + singleHandler(nil, error); + } else { + NSDictionary *userInfo = @{ + NSLocalizedDescriptionKey: @"The writer finished without producing any value." + }; + // Even though RxLibrary is independent of gRPC, the domain and code here are, for the moment, + // set to the values of kGRPCErrorDomain and GRPCErrorCodeInternal. This way, the error formed + // is the one user of gRPC would expect if the server failed to produce a response. + // + // TODO(jcanizales): Figure out a way to keep errors of RxLibrary generic without making users + // of gRPC take care of two different error domains and error code enums. A possibility is to + // add error handling to GRXWriters or GRXWriteables, and use them to translate errors between + // the two domains. + singleHandler(nil, [NSError errorWithDomain:@"io.grpc" code:13 userInfo:userInfo]); + } + }; + return [self writeableWithEventHandler:^(BOOL done, id value, NSError *error) { + if (eventHandler) { + eventHandler(done, value, error); } }]; } From cfcaccc6d20c67a2595b02b18fc31a3a2da83daf Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 11 Mar 2016 12:36:18 -0800 Subject: [PATCH 35/60] Revert "Revert "Pass a non-infinite deadline to grpc_completion_queue_next() to prevent queues from blocking indefinitely in poll()"" --- .../GRPCClient/private/GRPCCompletionQueue.h | 7 ++++++ .../GRPCClient/private/GRPCCompletionQueue.m | 24 +++++++++++++++---- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.h b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.h index 7b66cd4c329..a52095dd011 100644 --- a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.h +++ b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.h @@ -36,6 +36,8 @@ typedef void(^GRPCQueueCompletionHandler)(bool success); +extern const int64_t kGRPCCompletionQueueDefaultTimeoutSecs; + /** * This class lets one more easily use |grpc_completion_queue|. To use it, pass the value of the * |unmanagedQueue| property of an instance of this class to |grpc_channel_create_call|. Then for @@ -49,6 +51,11 @@ typedef void(^GRPCQueueCompletionHandler)(bool success); */ @interface GRPCCompletionQueue : NSObject @property(nonatomic, readonly) grpc_completion_queue *unmanagedQueue; +@property(nonatomic, readonly) int64_t timeoutSecs; + (instancetype)completionQueue; + +- (instancetype)init; +- (instancetype)initWithTimeout:(int64_t)timeoutSecs NS_DESIGNATED_INITIALIZER; + @end diff --git a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m index ff3031678c5..250bbf03a66 100644 --- a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m +++ b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m @@ -35,6 +35,9 @@ #import + +const int64_t kGRPCCompletionQueueDefaultTimeoutSecs = 60; + @implementation GRPCCompletionQueue + (instancetype)completionQueue { @@ -42,8 +45,13 @@ } - (instancetype)init { + return [self initWithTimeout:kGRPCCompletionQueueDefaultTimeoutSecs]; +} + +- (instancetype)initWithTimeout:(int64_t)timeoutSecs { if ((self = [super init])) { _unmanagedQueue = grpc_completion_queue_create(NULL); + _timeoutSecs = timeoutSecs; // This is for the following block to capture the pointer by value (instead // of retaining self and doing self->_unmanagedQueue). This is essential @@ -61,22 +69,28 @@ gDefaultConcurrentQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); }); dispatch_async(gDefaultConcurrentQueue, ^{ + // Using a non-infinite deadline to re-enter grpc_completion_queue_next() + // alleviates https://github.com/grpc/grpc/issues/5593 + gpr_timespec deadline = (timeoutSecs < 0) + ? gpr_inf_future(GPR_CLOCK_REALTIME) + : gpr_time_from_seconds(timeoutSecs, GPR_CLOCK_REALTIME); while (YES) { - // The following call blocks until an event is available. - grpc_event event = grpc_completion_queue_next(unmanagedQueue, - gpr_inf_future(GPR_CLOCK_REALTIME), - NULL); + // The following call blocks until an event is available or the deadline elapses. + grpc_event event = grpc_completion_queue_next(unmanagedQueue, deadline, NULL); GRPCQueueCompletionHandler handler; switch (event.type) { case GRPC_OP_COMPLETE: handler = (__bridge_transfer GRPCQueueCompletionHandler)event.tag; handler(event.success); break; + case GRPC_QUEUE_TIMEOUT: + // Nothing to do here + break; case GRPC_QUEUE_SHUTDOWN: grpc_completion_queue_destroy(unmanagedQueue); return; default: - [NSException raise:@"Unrecognized completion type" format:@""]; + [NSException raise:@"Unrecognized completion type" format:@"type=%d", event.type]; } }; }); From aad4156a0ba79d963bfa3cd37e48a1074f956809 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Fri, 11 Mar 2016 22:21:11 +0100 Subject: [PATCH 36/60] Disabling iOS user-agent test. --- src/objective-c/tests/GRPCClientTests.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 624958f4b97..7dd6873c802 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -273,10 +273,12 @@ static ProtoMethod *kUnaryCallMethod; id responsesWriteable = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { XCTAssertNotNil(value, @"nil value received as response."); XCTAssertEqual([value length], 0, @"Non-empty response received: %@", value); + /* This test needs to be more clever in regards to changing the version of the core. XCTAssertEqualObjects(call.responseHeaders[@"x-grpc-test-echo-useragent"], @"Foo grpc-objc/0.13.0 grpc-c/0.14.0-dev (ios)", @"Did not receive expected user agent %@", call.responseHeaders[@"x-grpc-test-echo-useragent"]); + */ [response fulfill]; } completionHandler:^(NSError *errorOrNil) { XCTAssertNil(errorOrNil, @"Finished with unexpected error: %@", errorOrNil); From afc59cbc886424e9c3ca9b5da1ffd2a817ee903c Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 11 Mar 2016 15:06:15 -0800 Subject: [PATCH 37/60] remove tests for EventInvocation API that is going to be removed --- src/python/grpcio/tests/tests.json | 9 - ...nt_invocation_synchronous_event_service.py | 381 ------------------ .../framework/interfaces/face/test_cases.py | 2 - 3 files changed, 392 deletions(-) delete mode 100644 src/python/grpcio/tests/unit/framework/interfaces/face/_event_invocation_synchronous_event_service.py diff --git a/src/python/grpcio/tests/tests.json b/src/python/grpcio/tests/tests.json index 388d040d5ca..84870aaa5cb 100644 --- a/src/python/grpcio/tests/tests.json +++ b/src/python/grpcio/tests/tests.json @@ -12,31 +12,22 @@ "_core_over_links_base_interface_test.SyncEasyTest", "_core_over_links_base_interface_test.SyncPeasyTest", "_crust_over_core_face_interface_test.DynamicInvokerBlockingInvocationInlineServiceTest", - "_crust_over_core_face_interface_test.DynamicInvokerEventInvocationSynchronousEventServiceTest", "_crust_over_core_face_interface_test.DynamicInvokerFutureInvocationAsynchronousEventServiceTest", "_crust_over_core_face_interface_test.GenericInvokerBlockingInvocationInlineServiceTest", - "_crust_over_core_face_interface_test.GenericInvokerEventInvocationSynchronousEventServiceTest", "_crust_over_core_face_interface_test.GenericInvokerFutureInvocationAsynchronousEventServiceTest", "_crust_over_core_face_interface_test.MultiCallableInvokerBlockingInvocationInlineServiceTest", - "_crust_over_core_face_interface_test.MultiCallableInvokerEventInvocationSynchronousEventServiceTest", "_crust_over_core_face_interface_test.MultiCallableInvokerFutureInvocationAsynchronousEventServiceTest", "_crust_over_core_over_links_face_interface_test.DynamicInvokerBlockingInvocationInlineServiceTest", - "_crust_over_core_over_links_face_interface_test.DynamicInvokerEventInvocationSynchronousEventServiceTest", "_crust_over_core_over_links_face_interface_test.DynamicInvokerFutureInvocationAsynchronousEventServiceTest", "_crust_over_core_over_links_face_interface_test.GenericInvokerBlockingInvocationInlineServiceTest", - "_crust_over_core_over_links_face_interface_test.GenericInvokerEventInvocationSynchronousEventServiceTest", "_crust_over_core_over_links_face_interface_test.GenericInvokerFutureInvocationAsynchronousEventServiceTest", "_crust_over_core_over_links_face_interface_test.MultiCallableInvokerBlockingInvocationInlineServiceTest", - "_crust_over_core_over_links_face_interface_test.MultiCallableInvokerEventInvocationSynchronousEventServiceTest", "_crust_over_core_over_links_face_interface_test.MultiCallableInvokerFutureInvocationAsynchronousEventServiceTest", "_face_interface_test.DynamicInvokerBlockingInvocationInlineServiceTest", - "_face_interface_test.DynamicInvokerEventInvocationSynchronousEventServiceTest", "_face_interface_test.DynamicInvokerFutureInvocationAsynchronousEventServiceTest", "_face_interface_test.GenericInvokerBlockingInvocationInlineServiceTest", - "_face_interface_test.GenericInvokerEventInvocationSynchronousEventServiceTest", "_face_interface_test.GenericInvokerFutureInvocationAsynchronousEventServiceTest", "_face_interface_test.MultiCallableInvokerBlockingInvocationInlineServiceTest", - "_face_interface_test.MultiCallableInvokerEventInvocationSynchronousEventServiceTest", "_face_interface_test.MultiCallableInvokerFutureInvocationAsynchronousEventServiceTest", "_implementations_test.ChannelCredentialsTest", "_insecure_interop_test.InsecureInteropTest", diff --git a/src/python/grpcio/tests/unit/framework/interfaces/face/_event_invocation_synchronous_event_service.py b/src/python/grpcio/tests/unit/framework/interfaces/face/_event_invocation_synchronous_event_service.py deleted file mode 100644 index 34db6c3e55f..00000000000 --- a/src/python/grpcio/tests/unit/framework/interfaces/face/_event_invocation_synchronous_event_service.py +++ /dev/null @@ -1,381 +0,0 @@ -# Copyright 2015, Google Inc. -# All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are -# met: -# -# * Redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer. -# * Redistributions in binary form must reproduce the above -# copyright notice, this list of conditions and the following disclaimer -# in the documentation and/or other materials provided with the -# distribution. -# * Neither the name of Google Inc. nor the names of its -# contributors may be used to endorse or promote products derived from -# this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -"""Test code for the Face layer of RPC Framework.""" - -import abc -import unittest - -# test_interfaces is referenced from specification in this module. -from grpc.framework.interfaces.face import face -from tests.unit.framework.common import test_constants -from tests.unit.framework.common import test_control -from tests.unit.framework.common import test_coverage -from tests.unit.framework.interfaces.face import _3069_test_constant -from tests.unit.framework.interfaces.face import _digest -from tests.unit.framework.interfaces.face import _receiver -from tests.unit.framework.interfaces.face import _stock_service -from tests.unit.framework.interfaces.face import test_interfaces # pylint: disable=unused-import - - -class TestCase(test_coverage.Coverage, unittest.TestCase): - """A test of the Face layer of RPC Framework. - - Concrete subclasses must have an "implementation" attribute of type - test_interfaces.Implementation and an "invoker_constructor" attribute of type - _invocation.InvokerConstructor. - """ - __metaclass__ = abc.ABCMeta - - NAME = 'EventInvocationSynchronousEventServiceTest' - - def setUp(self): - """See unittest.TestCase.setUp for full specification. - - Overriding implementations must call this implementation. - """ - self._control = test_control.PauseFailControl() - self._digest = _digest.digest( - _stock_service.STOCK_TEST_SERVICE, self._control, None) - - generic_stub, dynamic_stubs, self._memo = self.implementation.instantiate( - self._digest.methods, self._digest.event_method_implementations, None) - self._invoker = self.invoker_constructor.construct_invoker( - generic_stub, dynamic_stubs, self._digest.methods) - - def tearDown(self): - """See unittest.TestCase.tearDown for full specification. - - Overriding implementations must call this implementation. - """ - self._invoker = None - self.implementation.destantiate(self._memo) - - def testSuccessfulUnaryRequestUnaryResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.unary_unary_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - request = test_messages.request() - receiver = _receiver.Receiver() - - self._invoker.event(group, method)( - request, receiver, receiver.abort, test_constants.LONG_TIMEOUT) - receiver.block_until_terminated() - response = receiver.unary_response() - - test_messages.verify(request, response, self) - - def testSuccessfulUnaryRequestStreamResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.unary_stream_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - request = test_messages.request() - receiver = _receiver.Receiver() - - self._invoker.event(group, method)( - request, receiver, receiver.abort, test_constants.LONG_TIMEOUT) - receiver.block_until_terminated() - responses = receiver.stream_responses() - - test_messages.verify(request, responses, self) - - def testSuccessfulStreamRequestUnaryResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.stream_unary_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - requests = test_messages.requests() - receiver = _receiver.Receiver() - - call_consumer = self._invoker.event(group, method)( - receiver, receiver.abort, test_constants.LONG_TIMEOUT) - for request in requests: - call_consumer.consume(request) - call_consumer.terminate() - receiver.block_until_terminated() - response = receiver.unary_response() - - test_messages.verify(requests, response, self) - - def testSuccessfulStreamRequestStreamResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.stream_stream_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - requests = test_messages.requests() - receiver = _receiver.Receiver() - - call_consumer = self._invoker.event(group, method)( - receiver, receiver.abort, test_constants.LONG_TIMEOUT) - for request in requests: - call_consumer.consume(request) - call_consumer.terminate() - receiver.block_until_terminated() - responses = receiver.stream_responses() - - test_messages.verify(requests, responses, self) - - def testSequentialInvocations(self): - # pylint: disable=cell-var-from-loop - for (group, method), test_messages_sequence in ( - self._digest.unary_unary_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - first_request = test_messages.request() - second_request = test_messages.request() - second_receiver = _receiver.Receiver() - - def make_second_invocation(): - self._invoker.event(group, method)( - second_request, second_receiver, second_receiver.abort, - test_constants.LONG_TIMEOUT) - - class FirstReceiver(_receiver.Receiver): - - def complete(self, terminal_metadata, code, details): - super(FirstReceiver, self).complete( - terminal_metadata, code, details) - make_second_invocation() - - first_receiver = FirstReceiver() - - self._invoker.event(group, method)( - first_request, first_receiver, first_receiver.abort, - test_constants.LONG_TIMEOUT) - second_receiver.block_until_terminated() - - first_response = first_receiver.unary_response() - second_response = second_receiver.unary_response() - test_messages.verify(first_request, first_response, self) - test_messages.verify(second_request, second_response, self) - - def testParallelInvocations(self): - for (group, method), test_messages_sequence in ( - self._digest.unary_unary_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - first_request = test_messages.request() - first_receiver = _receiver.Receiver() - second_request = test_messages.request() - second_receiver = _receiver.Receiver() - - self._invoker.event(group, method)( - first_request, first_receiver, first_receiver.abort, - test_constants.LONG_TIMEOUT) - self._invoker.event(group, method)( - second_request, second_receiver, second_receiver.abort, - test_constants.LONG_TIMEOUT) - first_receiver.block_until_terminated() - second_receiver.block_until_terminated() - - first_response = first_receiver.unary_response() - second_response = second_receiver.unary_response() - test_messages.verify(first_request, first_response, self) - test_messages.verify(second_request, second_response, self) - - @unittest.skip('TODO(nathaniel): implement.') - def testWaitingForSomeButNotAllParallelInvocations(self): - raise NotImplementedError() - - def testCancelledUnaryRequestUnaryResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.unary_unary_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - request = test_messages.request() - receiver = _receiver.Receiver() - - with self._control.pause(): - call = self._invoker.event(group, method)( - request, receiver, receiver.abort, test_constants.LONG_TIMEOUT) - call.cancel() - receiver.block_until_terminated() - - self.assertIs(face.Abortion.Kind.CANCELLED, receiver.abortion().kind) - - def testCancelledUnaryRequestStreamResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.unary_stream_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - request = test_messages.request() - receiver = _receiver.Receiver() - - call = self._invoker.event(group, method)( - request, receiver, receiver.abort, test_constants.LONG_TIMEOUT) - call.cancel() - receiver.block_until_terminated() - - self.assertIs(face.Abortion.Kind.CANCELLED, receiver.abortion().kind) - - def testCancelledStreamRequestUnaryResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.stream_unary_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - requests = test_messages.requests() - receiver = _receiver.Receiver() - - call_consumer = self._invoker.event(group, method)( - receiver, receiver.abort, test_constants.LONG_TIMEOUT) - for request in requests: - call_consumer.consume(request) - call_consumer.cancel() - receiver.block_until_terminated() - - self.assertIs(face.Abortion.Kind.CANCELLED, receiver.abortion().kind) - - def testCancelledStreamRequestStreamResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.stream_stream_messages_sequences.iteritems()): - for unused_test_messages in test_messages_sequence: - receiver = _receiver.Receiver() - - call_consumer = self._invoker.event(group, method)( - receiver, receiver.abort, test_constants.LONG_TIMEOUT) - call_consumer.cancel() - receiver.block_until_terminated() - - self.assertIs(face.Abortion.Kind.CANCELLED, receiver.abortion().kind) - - def testExpiredUnaryRequestUnaryResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.unary_unary_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - request = test_messages.request() - receiver = _receiver.Receiver() - - with self._control.pause(): - self._invoker.event(group, method)( - request, receiver, receiver.abort, - _3069_test_constant.REALLY_SHORT_TIMEOUT) - receiver.block_until_terminated() - - self.assertIs(face.Abortion.Kind.EXPIRED, receiver.abortion().kind) - - def testExpiredUnaryRequestStreamResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.unary_stream_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - request = test_messages.request() - receiver = _receiver.Receiver() - - with self._control.pause(): - self._invoker.event(group, method)( - request, receiver, receiver.abort, - _3069_test_constant.REALLY_SHORT_TIMEOUT) - receiver.block_until_terminated() - - self.assertIs(face.Abortion.Kind.EXPIRED, receiver.abortion().kind) - - def testExpiredStreamRequestUnaryResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.stream_unary_messages_sequences.iteritems()): - for unused_test_messages in test_messages_sequence: - receiver = _receiver.Receiver() - - self._invoker.event(group, method)( - receiver, receiver.abort, _3069_test_constant.REALLY_SHORT_TIMEOUT) - receiver.block_until_terminated() - - self.assertIs(face.Abortion.Kind.EXPIRED, receiver.abortion().kind) - - def testExpiredStreamRequestStreamResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.stream_stream_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - requests = test_messages.requests() - receiver = _receiver.Receiver() - - call_consumer = self._invoker.event(group, method)( - receiver, receiver.abort, _3069_test_constant.REALLY_SHORT_TIMEOUT) - for request in requests: - call_consumer.consume(request) - receiver.block_until_terminated() - - self.assertIs(face.Abortion.Kind.EXPIRED, receiver.abortion().kind) - - def testFailedUnaryRequestUnaryResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.unary_unary_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - request = test_messages.request() - receiver = _receiver.Receiver() - - with self._control.fail(): - self._invoker.event(group, method)( - request, receiver, receiver.abort, test_constants.LONG_TIMEOUT) - receiver.block_until_terminated() - - self.assertIs( - face.Abortion.Kind.REMOTE_FAILURE, receiver.abortion().kind) - - def testFailedUnaryRequestStreamResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.unary_stream_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - request = test_messages.request() - receiver = _receiver.Receiver() - - with self._control.fail(): - self._invoker.event(group, method)( - request, receiver, receiver.abort, test_constants.LONG_TIMEOUT) - receiver.block_until_terminated() - - self.assertIs( - face.Abortion.Kind.REMOTE_FAILURE, receiver.abortion().kind) - - def testFailedStreamRequestUnaryResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.stream_unary_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - requests = test_messages.requests() - receiver = _receiver.Receiver() - - with self._control.fail(): - call_consumer = self._invoker.event(group, method)( - receiver, receiver.abort, test_constants.LONG_TIMEOUT) - for request in requests: - call_consumer.consume(request) - call_consumer.terminate() - receiver.block_until_terminated() - - self.assertIs( - face.Abortion.Kind.REMOTE_FAILURE, receiver.abortion().kind) - - def testFailedStreamRequestStreamResponse(self): - for (group, method), test_messages_sequence in ( - self._digest.stream_stream_messages_sequences.iteritems()): - for test_messages in test_messages_sequence: - requests = test_messages.requests() - receiver = _receiver.Receiver() - - with self._control.fail(): - call_consumer = self._invoker.event(group, method)( - receiver, receiver.abort, test_constants.LONG_TIMEOUT) - for request in requests: - call_consumer.consume(request) - call_consumer.terminate() - receiver.block_until_terminated() - - self.assertIs( - face.Abortion.Kind.REMOTE_FAILURE, receiver.abortion().kind) diff --git a/src/python/grpcio/tests/unit/framework/interfaces/face/test_cases.py b/src/python/grpcio/tests/unit/framework/interfaces/face/test_cases.py index 462829b6606..71de9d835e9 100644 --- a/src/python/grpcio/tests/unit/framework/interfaces/face/test_cases.py +++ b/src/python/grpcio/tests/unit/framework/interfaces/face/test_cases.py @@ -34,14 +34,12 @@ import unittest # pylint: disable=unused-import # test_interfaces is referenced from specification in this module. from tests.unit.framework.interfaces.face import _blocking_invocation_inline_service -from tests.unit.framework.interfaces.face import _event_invocation_synchronous_event_service from tests.unit.framework.interfaces.face import _future_invocation_asynchronous_event_service from tests.unit.framework.interfaces.face import _invocation from tests.unit.framework.interfaces.face import test_interfaces # pylint: disable=unused-import _TEST_CASE_SUPERCLASSES = ( _blocking_invocation_inline_service.TestCase, - _event_invocation_synchronous_event_service.TestCase, _future_invocation_asynchronous_event_service.TestCase, ) From 8734e02f52025219de2a3908f1eb0002568d1ecd Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 11 Mar 2016 15:52:53 -0800 Subject: [PATCH 38/60] fix copyright --- .../grpcio/tests/unit/framework/interfaces/face/test_cases.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/grpcio/tests/unit/framework/interfaces/face/test_cases.py b/src/python/grpcio/tests/unit/framework/interfaces/face/test_cases.py index 71de9d835e9..06b9d77e521 100644 --- a/src/python/grpcio/tests/unit/framework/interfaces/face/test_cases.py +++ b/src/python/grpcio/tests/unit/framework/interfaces/face/test_cases.py @@ -1,4 +1,4 @@ -# Copyright 2015, Google Inc. +# Copyright 2015-2016, Google Inc. # All rights reserved. # # Redistribution and use in source and binary forms, with or without From 57e2743b6580ae19326ceccb5ac9ed88e0691a94 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 16:53:58 -0800 Subject: [PATCH 39/60] Add contributed test --- Makefile | 36 ++++ build.yaml | 10 + .../surface/concurrent_connectivity_test.c | 48 +++++ tools/run_tests/sources_and_headers.json | 16 ++ tools/run_tests/tests.json | 21 ++ vsprojects/buildtests_c.sln | 27 +++ .../concurrent_connectivity_test.vcxproj | 199 ++++++++++++++++++ ...ncurrent_connectivity_test.vcxproj.filters | 21 ++ 8 files changed, 378 insertions(+) create mode 100644 test/core/surface/concurrent_connectivity_test.c create mode 100644 vsprojects/vcxproj/test/concurrent_connectivity_test/concurrent_connectivity_test.vcxproj create mode 100644 vsprojects/vcxproj/test/concurrent_connectivity_test/concurrent_connectivity_test.vcxproj.filters diff --git a/Makefile b/Makefile index 171a83c2a3a..f118d542538 100644 --- a/Makefile +++ b/Makefile @@ -849,6 +849,7 @@ chttp2_status_conversion_test: $(BINDIR)/$(CONFIG)/chttp2_status_conversion_test chttp2_stream_map_test: $(BINDIR)/$(CONFIG)/chttp2_stream_map_test chttp2_varint_test: $(BINDIR)/$(CONFIG)/chttp2_varint_test compression_test: $(BINDIR)/$(CONFIG)/compression_test +concurrent_connectivity_test: $(BINDIR)/$(CONFIG)/concurrent_connectivity_test dns_resolver_connectivity_test: $(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test dns_resolver_test: $(BINDIR)/$(CONFIG)/dns_resolver_test dualstack_socket_test: $(BINDIR)/$(CONFIG)/dualstack_socket_test @@ -1160,6 +1161,7 @@ buildtests_c: privatelibs_c \ $(BINDIR)/$(CONFIG)/chttp2_stream_map_test \ $(BINDIR)/$(CONFIG)/chttp2_varint_test \ $(BINDIR)/$(CONFIG)/compression_test \ + $(BINDIR)/$(CONFIG)/concurrent_connectivity_test \ $(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test \ $(BINDIR)/$(CONFIG)/dns_resolver_test \ $(BINDIR)/$(CONFIG)/dualstack_socket_test \ @@ -1404,6 +1406,8 @@ test_c: buildtests_c $(Q) $(BINDIR)/$(CONFIG)/chttp2_varint_test || ( echo test chttp2_varint_test failed ; exit 1 ) $(E) "[RUN] Testing compression_test" $(Q) $(BINDIR)/$(CONFIG)/compression_test || ( echo test compression_test failed ; exit 1 ) + $(E) "[RUN] Testing concurrent_connectivity_test" + $(Q) $(BINDIR)/$(CONFIG)/concurrent_connectivity_test || ( echo test concurrent_connectivity_test failed ; exit 1 ) $(E) "[RUN] Testing dns_resolver_connectivity_test" $(Q) $(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test || ( echo test dns_resolver_connectivity_test failed ; exit 1 ) $(E) "[RUN] Testing dns_resolver_test" @@ -6079,6 +6083,38 @@ endif endif +CONCURRENT_CONNECTIVITY_TEST_SRC = \ + test/core/surface/concurrent_connectivity_test.c \ + +CONCURRENT_CONNECTIVITY_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(CONCURRENT_CONNECTIVITY_TEST_SRC)))) +ifeq ($(NO_SECURE),true) + +# You can't build secure targets if you don't have OpenSSL. + +$(BINDIR)/$(CONFIG)/concurrent_connectivity_test: openssl_dep_error + +else + + + +$(BINDIR)/$(CONFIG)/concurrent_connectivity_test: $(CONCURRENT_CONNECTIVITY_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + $(E) "[LD] Linking $@" + $(Q) mkdir -p `dirname $@` + $(Q) $(LD) $(LDFLAGS) $(CONCURRENT_CONNECTIVITY_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/concurrent_connectivity_test + +endif + +$(OBJDIR)/$(CONFIG)/test/core/surface/concurrent_connectivity_test.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + +deps_concurrent_connectivity_test: $(CONCURRENT_CONNECTIVITY_TEST_OBJS:.o=.dep) + +ifneq ($(NO_SECURE),true) +ifneq ($(NO_DEPS),true) +-include $(CONCURRENT_CONNECTIVITY_TEST_OBJS:.o=.dep) +endif +endif + + DNS_RESOLVER_CONNECTIVITY_TEST_SRC = \ test/core/client_config/resolvers/dns_resolver_connectivity_test.c \ diff --git a/build.yaml b/build.yaml index e003dd83396..83b7714e655 100644 --- a/build.yaml +++ b/build.yaml @@ -1057,6 +1057,16 @@ targets: - grpc - gpr_test_util - gpr +- name: concurrent_connectivity_test + build: test + language: c + src: + - test/core/surface/concurrent_connectivity_test.c + deps: + - grpc_test_util + - grpc + - gpr_test_util + - gpr - name: dns_resolver_connectivity_test cpu_cost: 0.1 build: test diff --git a/test/core/surface/concurrent_connectivity_test.c b/test/core/surface/concurrent_connectivity_test.c new file mode 100644 index 00000000000..d8639f5e510 --- /dev/null +++ b/test/core/surface/concurrent_connectivity_test.c @@ -0,0 +1,48 @@ +#include + +#include +#include +#include + +#define NUM_THREADS 100 +static grpc_channel* channels[NUM_THREADS]; +static grpc_completion_queue* queues[NUM_THREADS]; + +void create_loop_destroy(void* actually_an_int) { + int thread_index = (int)(actually_an_int); + for (int i = 0; i < 10; ++i) { + grpc_completion_queue* cq = grpc_completion_queue_create(NULL); + grpc_channel* chan = grpc_insecure_channel_create("localhost", NULL, NULL); + + channels[thread_index] = chan; + queues[thread_index] = cq; + + gpr_timespec inf_future = gpr_inf_future(GPR_CLOCK_REALTIME); + gpr_timespec delta = gpr_time_from_millis(10, GPR_TIMESPAN); + for (int j = 0; j < 10; ++j) { + gpr_timespec later_time = + gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), delta); + grpc_connectivity_state state = + grpc_channel_check_connectivity_state(chan, 1); + grpc_channel_watch_connectivity_state(chan, state, later_time, cq, NULL); + grpc_completion_queue_next(cq, inf_future, NULL); + } + grpc_channel_destroy(channels[thread_index]); + grpc_completion_queue_destroy(queues[thread_index]); + } +} + +int main() { + grpc_init(); + gpr_thd_id threads[NUM_THREADS]; + for (intptr_t i = 0; i < NUM_THREADS; ++i) { + gpr_thd_options options = gpr_thd_options_default(); + gpr_thd_options_set_joinable(&options); + gpr_thd_new(&threads[i], create_loop_destroy, (void*)i, &options); + } + for (int i = 0; i < NUM_THREADS; ++i) { + gpr_thd_join(threads[i]); + } + grpc_shutdown(); + return 0; +} diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 0039b99a55e..092ed35ad97 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -189,6 +189,22 @@ "third_party": false, "type": "target" }, + { + "deps": [ + "gpr", + "gpr_test_util", + "grpc", + "grpc_test_util" + ], + "headers": [], + "language": "c", + "name": "concurrent_connectivity_test", + "src": [ + "test/core/surface/concurrent_connectivity_test.c" + ], + "third_party": false, + "type": "target" + }, { "deps": [ "gpr", diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index 724cd64d19c..000315f2f4a 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -253,6 +253,27 @@ "windows" ] }, + { + "args": [], + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "gtest": false, + "language": "c", + "name": "concurrent_connectivity_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ] + }, { "args": [], "ci_platforms": [ diff --git a/vsprojects/buildtests_c.sln b/vsprojects/buildtests_c.sln index be2c4a3bb14..413ed3e3f38 100644 --- a/vsprojects/buildtests_c.sln +++ b/vsprojects/buildtests_c.sln @@ -251,6 +251,17 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "compression_test", "vcxproj {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} = {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} EndProjectSection EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "concurrent_connectivity_test", "vcxproj\test\concurrent_connectivity_test\concurrent_connectivity_test.vcxproj", "{391B366C-D916-45AA-3FE5-67363A46193B}" + ProjectSection(myProperties) = preProject + lib = "False" + EndProjectSection + ProjectSection(ProjectDependencies) = postProject + {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} = {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} + {29D16885-7228-4C31-81ED-5F9187C7F2A9} = {29D16885-7228-4C31-81ED-5F9187C7F2A9} + {EAB0A629-17A9-44DB-B5FF-E91A721FE037} = {EAB0A629-17A9-44DB-B5FF-E91A721FE037} + {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} = {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} + EndProjectSection +EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "dns_resolver_connectivity_test", "vcxproj\test\dns_resolver_connectivity_test\dns_resolver_connectivity_test.vcxproj", "{F7B6FE68-E847-D7CA-4062-E737E542BCC3}" ProjectSection(myProperties) = preProject lib = "False" @@ -1747,6 +1758,22 @@ Global {5AFE7D17-A4A7-D68E-4491-CBC852F9D2A0}.Release-DLL|Win32.Build.0 = Release|Win32 {5AFE7D17-A4A7-D68E-4491-CBC852F9D2A0}.Release-DLL|x64.ActiveCfg = Release|x64 {5AFE7D17-A4A7-D68E-4491-CBC852F9D2A0}.Release-DLL|x64.Build.0 = Release|x64 + {391B366C-D916-45AA-3FE5-67363A46193B}.Debug|Win32.ActiveCfg = Debug|Win32 + {391B366C-D916-45AA-3FE5-67363A46193B}.Debug|x64.ActiveCfg = Debug|x64 + {391B366C-D916-45AA-3FE5-67363A46193B}.Release|Win32.ActiveCfg = Release|Win32 + {391B366C-D916-45AA-3FE5-67363A46193B}.Release|x64.ActiveCfg = Release|x64 + {391B366C-D916-45AA-3FE5-67363A46193B}.Debug|Win32.Build.0 = Debug|Win32 + {391B366C-D916-45AA-3FE5-67363A46193B}.Debug|x64.Build.0 = Debug|x64 + {391B366C-D916-45AA-3FE5-67363A46193B}.Release|Win32.Build.0 = Release|Win32 + {391B366C-D916-45AA-3FE5-67363A46193B}.Release|x64.Build.0 = Release|x64 + {391B366C-D916-45AA-3FE5-67363A46193B}.Debug-DLL|Win32.ActiveCfg = Debug|Win32 + {391B366C-D916-45AA-3FE5-67363A46193B}.Debug-DLL|Win32.Build.0 = Debug|Win32 + {391B366C-D916-45AA-3FE5-67363A46193B}.Debug-DLL|x64.ActiveCfg = Debug|x64 + {391B366C-D916-45AA-3FE5-67363A46193B}.Debug-DLL|x64.Build.0 = Debug|x64 + {391B366C-D916-45AA-3FE5-67363A46193B}.Release-DLL|Win32.ActiveCfg = Release|Win32 + {391B366C-D916-45AA-3FE5-67363A46193B}.Release-DLL|Win32.Build.0 = Release|Win32 + {391B366C-D916-45AA-3FE5-67363A46193B}.Release-DLL|x64.ActiveCfg = Release|x64 + {391B366C-D916-45AA-3FE5-67363A46193B}.Release-DLL|x64.Build.0 = Release|x64 {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Debug|Win32.ActiveCfg = Debug|Win32 {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Debug|x64.ActiveCfg = Debug|x64 {F7B6FE68-E847-D7CA-4062-E737E542BCC3}.Release|Win32.ActiveCfg = Release|Win32 diff --git a/vsprojects/vcxproj/test/concurrent_connectivity_test/concurrent_connectivity_test.vcxproj b/vsprojects/vcxproj/test/concurrent_connectivity_test/concurrent_connectivity_test.vcxproj new file mode 100644 index 00000000000..6d69eae302f --- /dev/null +++ b/vsprojects/vcxproj/test/concurrent_connectivity_test/concurrent_connectivity_test.vcxproj @@ -0,0 +1,199 @@ + + + + + + Debug + Win32 + + + Debug + x64 + + + Release + Win32 + + + Release + x64 + + + + {391B366C-D916-45AA-3FE5-67363A46193B} + true + $(SolutionDir)IntDir\$(MSBuildProjectName)\ + + + + v100 + + + v110 + + + v120 + + + v140 + + + Application + true + Unicode + + + Application + false + true + Unicode + + + + + + + + + + + + + + concurrent_connectivity_test + static + Debug + static + Debug + + + concurrent_connectivity_test + static + Release + static + Release + + + + NotUsing + Level3 + Disabled + WIN32;_DEBUG;_LIB;%(PreprocessorDefinitions) + true + MultiThreadedDebug + true + None + false + + + Console + true + false + + + + + + NotUsing + Level3 + Disabled + WIN32;_DEBUG;_LIB;%(PreprocessorDefinitions) + true + MultiThreadedDebug + true + None + false + + + Console + true + false + + + + + + NotUsing + Level3 + MaxSpeed + WIN32;NDEBUG;_LIB;%(PreprocessorDefinitions) + true + true + true + MultiThreaded + true + None + false + + + Console + true + false + true + true + + + + + + NotUsing + Level3 + MaxSpeed + WIN32;NDEBUG;_LIB;%(PreprocessorDefinitions) + true + true + true + MultiThreaded + true + None + false + + + Console + true + false + true + true + + + + + + + + + + {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} + + + {29D16885-7228-4C31-81ED-5F9187C7F2A9} + + + {EAB0A629-17A9-44DB-B5FF-E91A721FE037} + + + {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} + + + + + + + + + + + + + + + This project references NuGet package(s) that are missing on this computer. Enable NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}. + + + + + + + + + diff --git a/vsprojects/vcxproj/test/concurrent_connectivity_test/concurrent_connectivity_test.vcxproj.filters b/vsprojects/vcxproj/test/concurrent_connectivity_test/concurrent_connectivity_test.vcxproj.filters new file mode 100644 index 00000000000..a6492c775a2 --- /dev/null +++ b/vsprojects/vcxproj/test/concurrent_connectivity_test/concurrent_connectivity_test.vcxproj.filters @@ -0,0 +1,21 @@ + + + + + test\core\surface + + + + + + {1320b717-a107-004b-c517-074e6a7baf97} + + + {f205219c-cee2-c0e8-a922-5486f2d31026} + + + {6804b6ae-88f4-acf9-b3fc-6b12f8e28d4d} + + + + From 0ce7bec923867aae3b115a9a7e4fc2e56f021353 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 17:02:07 -0800 Subject: [PATCH 40/60] Fail faster --- test/core/surface/concurrent_connectivity_test.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/core/surface/concurrent_connectivity_test.c b/test/core/surface/concurrent_connectivity_test.c index d8639f5e510..1046d422774 100644 --- a/test/core/surface/concurrent_connectivity_test.c +++ b/test/core/surface/concurrent_connectivity_test.c @@ -2,7 +2,9 @@ #include #include +#include #include +#include "test/core/util/test_config.h" #define NUM_THREADS 100 static grpc_channel* channels[NUM_THREADS]; @@ -17,22 +19,22 @@ void create_loop_destroy(void* actually_an_int) { channels[thread_index] = chan; queues[thread_index] = cq; - gpr_timespec inf_future = gpr_inf_future(GPR_CLOCK_REALTIME); - gpr_timespec delta = gpr_time_from_millis(10, GPR_TIMESPAN); for (int j = 0; j < 10; ++j) { - gpr_timespec later_time = - gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), delta); + gpr_timespec later_time = GRPC_TIMEOUT_MILLIS_TO_DEADLINE(10); grpc_connectivity_state state = grpc_channel_check_connectivity_state(chan, 1); grpc_channel_watch_connectivity_state(chan, state, later_time, cq, NULL); - grpc_completion_queue_next(cq, inf_future, NULL); + GPR_ASSERT(grpc_completion_queue_next( + cq, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(3), NULL) + .type == GRPC_OP_COMPLETE); } grpc_channel_destroy(channels[thread_index]); grpc_completion_queue_destroy(queues[thread_index]); } } -int main() { +int main(int argc, char** argv) { + grpc_test_init(argc, argv); grpc_init(); gpr_thd_id threads[NUM_THREADS]; for (intptr_t i = 0; i < NUM_THREADS; ++i) { From 2ba4133d3a260f3fe218160ef6fe68e0edc1e4a7 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 17:09:12 -0800 Subject: [PATCH 41/60] Fix race in subchannel.c --- src/core/client_config/subchannel.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/core/client_config/subchannel.c b/src/core/client_config/subchannel.c index d91dd116b8c..ddd129c5397 100644 --- a/src/core/client_config/subchannel.c +++ b/src/core/client_config/subchannel.c @@ -184,8 +184,8 @@ static void connection_destroy(grpc_exec_ctx *exec_ctx, void *arg, gpr_free(c); } -void grpc_connected_subchannel_ref(grpc_connected_subchannel *c - GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { +void grpc_connected_subchannel_ref( + grpc_connected_subchannel *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { GRPC_CHANNEL_STACK_REF(CHANNEL_STACK_FROM_CONNECTION(c), REF_REASON); } @@ -226,8 +226,8 @@ static gpr_atm ref_mutate(grpc_subchannel *c, gpr_atm delta, return old_val; } -grpc_subchannel *grpc_subchannel_ref(grpc_subchannel *c - GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { +grpc_subchannel *grpc_subchannel_ref( + grpc_subchannel *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { gpr_atm old_refs; old_refs = ref_mutate(c, (1 << INTERNAL_REF_BITS), 0 REF_MUTATE_PURPOSE("STRONG_REF")); @@ -235,8 +235,8 @@ grpc_subchannel *grpc_subchannel_ref(grpc_subchannel *c return c; } -grpc_subchannel *grpc_subchannel_weak_ref(grpc_subchannel *c - GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { +grpc_subchannel *grpc_subchannel_weak_ref( + grpc_subchannel *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { gpr_atm old_refs; old_refs = ref_mutate(c, 1, 0 REF_MUTATE_PURPOSE("WEAK_REF")); GPR_ASSERT(old_refs != 0); @@ -646,19 +646,23 @@ static void subchannel_connected(grpc_exec_ctx *exec_ctx, void *arg, if (c->connecting_result.transport != NULL) { publish_transport(exec_ctx, c); - } else if (c->disconnected) { + } + + GRPC_SUBCHANNEL_WEAK_REF(c, "connected"); + gpr_mu_lock(&c->mu); + if (c->disconnected) { GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, c, "connecting"); } else { gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); - gpr_mu_lock(&c->mu); GPR_ASSERT(!c->have_alarm); c->have_alarm = 1; grpc_connectivity_state_set(exec_ctx, &c->state_tracker, GRPC_CHANNEL_TRANSIENT_FAILURE, "connect_failed"); grpc_timer_init(exec_ctx, &c->alarm, c->next_attempt, on_alarm, c, now); - gpr_mu_unlock(&c->mu); } + gpr_mu_unlock(&c->mu); + GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, c, "connecting"); } static gpr_timespec compute_connect_deadline(grpc_subchannel *c) { @@ -686,8 +690,8 @@ static void subchannel_call_destroy(grpc_exec_ctx *exec_ctx, void *call, GPR_TIMER_END("grpc_subchannel_call_unref.destroy", 0); } -void grpc_subchannel_call_ref(grpc_subchannel_call *c - GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { +void grpc_subchannel_call_ref( + grpc_subchannel_call *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { GRPC_CALL_STACK_REF(SUBCHANNEL_CALL_TO_CALL_STACK(c), REF_REASON); } From 64baf2cbab3bc3caa98907758ee0a6c7db72e68a Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 17:16:33 -0800 Subject: [PATCH 42/60] Extend timeout --- test/core/surface/concurrent_connectivity_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/surface/concurrent_connectivity_test.c b/test/core/surface/concurrent_connectivity_test.c index 1046d422774..7306a394ebd 100644 --- a/test/core/surface/concurrent_connectivity_test.c +++ b/test/core/surface/concurrent_connectivity_test.c @@ -25,7 +25,7 @@ void create_loop_destroy(void* actually_an_int) { grpc_channel_check_connectivity_state(chan, 1); grpc_channel_watch_connectivity_state(chan, state, later_time, cq, NULL); GPR_ASSERT(grpc_completion_queue_next( - cq, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(3), NULL) + cq, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(30), NULL) .type == GRPC_OP_COMPLETE); } grpc_channel_destroy(channels[thread_index]); From d24fc85b59b4224d7932c5a7a9f17e73402f388b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 17:18:19 -0800 Subject: [PATCH 43/60] Fix cast --- test/core/surface/concurrent_connectivity_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/surface/concurrent_connectivity_test.c b/test/core/surface/concurrent_connectivity_test.c index 7306a394ebd..cd1aead5e51 100644 --- a/test/core/surface/concurrent_connectivity_test.c +++ b/test/core/surface/concurrent_connectivity_test.c @@ -11,7 +11,7 @@ static grpc_channel* channels[NUM_THREADS]; static grpc_completion_queue* queues[NUM_THREADS]; void create_loop_destroy(void* actually_an_int) { - int thread_index = (int)(actually_an_int); + int thread_index = (int)(intptr_t)(actually_an_int); for (int i = 0; i < 10; ++i) { grpc_completion_queue* cq = grpc_completion_queue_create(NULL); grpc_channel* chan = grpc_insecure_channel_create("localhost", NULL, NULL); From 5a8ec75012862815bc28a2145c6d294b1993cbd4 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 11 Mar 2016 18:41:54 -0800 Subject: [PATCH 44/60] Use a named constant for error domain and code --- src/objective-c/RxLibrary/GRXWriteable.m | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/objective-c/RxLibrary/GRXWriteable.m b/src/objective-c/RxLibrary/GRXWriteable.m index b122c04ce0c..c15ccb17fb9 100644 --- a/src/objective-c/RxLibrary/GRXWriteable.m +++ b/src/objective-c/RxLibrary/GRXWriteable.m @@ -68,7 +68,11 @@ // of gRPC take care of two different error domains and error code enums. A possibility is to // add error handling to GRXWriters or GRXWriteables, and use them to translate errors between // the two domains. - singleHandler(nil, [NSError errorWithDomain:@"io.grpc" code:13 userInfo:userInfo]); + static NSString *kGRPCErrorDomain = @"io.grpc"; + static NSUInteger kGRPCErrorCodeInternal = 13; + singleHandler(nil, [NSError errorWithDomain:kGRPCErrorDomain + code:kGRPCErrorCodeInternal + userInfo:userInfo]); } }; return [self writeableWithEventHandler:^(BOOL done, id value, NSError *error) { From 3591482a372dee047a7c0867ada8cc8d360d247d Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 19:29:23 -0800 Subject: [PATCH 45/60] Expand lock --- src/core/client_config/subchannel.c | 14 +++++--------- test/core/surface/concurrent_connectivity_test.c | 2 +- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/core/client_config/subchannel.c b/src/core/client_config/subchannel.c index ddd129c5397..e8f308b6076 100644 --- a/src/core/client_config/subchannel.c +++ b/src/core/client_config/subchannel.c @@ -505,7 +505,8 @@ void grpc_connected_subchannel_ping(grpc_exec_ctx *exec_ctx, elem->filter->start_transport_op(exec_ctx, elem, &op); } -static void publish_transport(grpc_exec_ctx *exec_ctx, grpc_subchannel *c) { +static void publish_transport_locked(grpc_exec_ctx *exec_ctx, + grpc_subchannel *c) { size_t channel_stack_size; grpc_connected_subchannel *con; grpc_channel_stack *stk; @@ -541,8 +542,6 @@ static void publish_transport(grpc_exec_ctx *exec_ctx, grpc_subchannel *c) { grpc_closure_init(&sw_subchannel->closure, subchannel_on_child_state_changed, sw_subchannel); - gpr_mu_lock(&c->mu); - if (c->disconnected) { gpr_mu_unlock(&c->mu); gpr_free(sw_subchannel); @@ -575,7 +574,6 @@ static void publish_transport(grpc_exec_ctx *exec_ctx, grpc_subchannel *c) { grpc_connectivity_state_set(exec_ctx, &c->state_tracker, GRPC_CHANNEL_READY, "connected"); - gpr_mu_unlock(&c->mu); gpr_free((void *)filters); } @@ -644,13 +642,11 @@ static void subchannel_connected(grpc_exec_ctx *exec_ctx, void *arg, bool iomgr_success) { grpc_subchannel *c = arg; - if (c->connecting_result.transport != NULL) { - publish_transport(exec_ctx, c); - } - GRPC_SUBCHANNEL_WEAK_REF(c, "connected"); gpr_mu_lock(&c->mu); - if (c->disconnected) { + if (c->connecting_result.transport != NULL) { + publish_transport_locked(exec_ctx, c); + } else if (c->disconnected) { GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, c, "connecting"); } else { gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); diff --git a/test/core/surface/concurrent_connectivity_test.c b/test/core/surface/concurrent_connectivity_test.c index 7306a394ebd..1046d422774 100644 --- a/test/core/surface/concurrent_connectivity_test.c +++ b/test/core/surface/concurrent_connectivity_test.c @@ -25,7 +25,7 @@ void create_loop_destroy(void* actually_an_int) { grpc_channel_check_connectivity_state(chan, 1); grpc_channel_watch_connectivity_state(chan, state, later_time, cq, NULL); GPR_ASSERT(grpc_completion_queue_next( - cq, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(30), NULL) + cq, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(3), NULL) .type == GRPC_OP_COMPLETE); } grpc_channel_destroy(channels[thread_index]); From 9fb89ea9e31dbe376fc1ce9c173d4e52f4b31290 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 20:12:45 -0800 Subject: [PATCH 46/60] If we cant check timers due to contention, at least make sure we do a follow up check soon --- src/core/client_config/subchannel.c | 16 ++++---- src/core/iomgr/timer.c | 3 ++ src/core/iomgr/timer.h | 1 - src/core/surface/completion_queue.c | 2 +- .../surface/concurrent_connectivity_test.c | 39 +++++++++++++++++-- test/core/surface/lame_client_test.c | 2 +- 6 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/core/client_config/subchannel.c b/src/core/client_config/subchannel.c index e8f308b6076..ec9e47a6ddb 100644 --- a/src/core/client_config/subchannel.c +++ b/src/core/client_config/subchannel.c @@ -184,8 +184,8 @@ static void connection_destroy(grpc_exec_ctx *exec_ctx, void *arg, gpr_free(c); } -void grpc_connected_subchannel_ref( - grpc_connected_subchannel *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { +void grpc_connected_subchannel_ref(grpc_connected_subchannel *c + GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { GRPC_CHANNEL_STACK_REF(CHANNEL_STACK_FROM_CONNECTION(c), REF_REASON); } @@ -226,8 +226,8 @@ static gpr_atm ref_mutate(grpc_subchannel *c, gpr_atm delta, return old_val; } -grpc_subchannel *grpc_subchannel_ref( - grpc_subchannel *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { +grpc_subchannel *grpc_subchannel_ref(grpc_subchannel *c + GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { gpr_atm old_refs; old_refs = ref_mutate(c, (1 << INTERNAL_REF_BITS), 0 REF_MUTATE_PURPOSE("STRONG_REF")); @@ -235,8 +235,8 @@ grpc_subchannel *grpc_subchannel_ref( return c; } -grpc_subchannel *grpc_subchannel_weak_ref( - grpc_subchannel *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { +grpc_subchannel *grpc_subchannel_weak_ref(grpc_subchannel *c + GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { gpr_atm old_refs; old_refs = ref_mutate(c, 1, 0 REF_MUTATE_PURPOSE("WEAK_REF")); GPR_ASSERT(old_refs != 0); @@ -686,8 +686,8 @@ static void subchannel_call_destroy(grpc_exec_ctx *exec_ctx, void *call, GPR_TIMER_END("grpc_subchannel_call_unref.destroy", 0); } -void grpc_subchannel_call_ref( - grpc_subchannel_call *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { +void grpc_subchannel_call_ref(grpc_subchannel_call *c + GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { GRPC_CALL_STACK_REF(SUBCHANNEL_CALL_TO_CALL_STACK(c), REF_REASON); } diff --git a/src/core/iomgr/timer.c b/src/core/iomgr/timer.c index 8379fffad02..8badd83bbb1 100644 --- a/src/core/iomgr/timer.c +++ b/src/core/iomgr/timer.c @@ -330,6 +330,9 @@ static int run_some_expired_timers(grpc_exec_ctx *exec_ctx, gpr_timespec now, gpr_mu_unlock(&g_mu); gpr_mu_unlock(&g_checker_mu); + } else if (next != NULL) { + *next = gpr_time_min( + *next, gpr_time_add(now, gpr_time_from_millis(100, GPR_TIMESPAN))); } return (int)n; diff --git a/src/core/iomgr/timer.h b/src/core/iomgr/timer.h index 9ad1e92f42e..e239e884e7b 100644 --- a/src/core/iomgr/timer.h +++ b/src/core/iomgr/timer.h @@ -96,7 +96,6 @@ void grpc_timer_cancel(grpc_exec_ctx *exec_ctx, grpc_timer *timer); *next is never guaranteed to be updated on any given execution; however, with high probability at least one thread in the system will see an update at any time slice. */ - bool grpc_timer_check(grpc_exec_ctx *exec_ctx, gpr_timespec now, gpr_timespec *next); void grpc_timer_list_init(gpr_timespec now); diff --git a/src/core/surface/completion_queue.c b/src/core/surface/completion_queue.c index f6a95ebbd3f..b22818ea87d 100644 --- a/src/core/surface/completion_queue.c +++ b/src/core/surface/completion_queue.c @@ -86,7 +86,7 @@ struct grpc_completion_queue { #define POLLSET_FROM_CQ(cq) ((grpc_pollset *)(cq + 1)) static gpr_mu g_freelist_mu; -grpc_completion_queue *g_freelist; +static grpc_completion_queue *g_freelist; static void on_pollset_shutdown_done(grpc_exec_ctx *exec_ctx, void *cc, bool success); diff --git a/test/core/surface/concurrent_connectivity_test.c b/test/core/surface/concurrent_connectivity_test.c index b380d4471fc..5361201db72 100644 --- a/test/core/surface/concurrent_connectivity_test.c +++ b/test/core/surface/concurrent_connectivity_test.c @@ -1,3 +1,36 @@ +/* +* +* Copyright 2015-2016, Google Inc. +* All rights reserved. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions are +* met: +* +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above +* copyright notice, this list of conditions and the following disclaimer +* in the documentation and/or other materials provided with the +* distribution. +* * Neither the name of Google Inc. nor the names of its +* contributors may be used to endorse or promote products derived from +* this software without specific prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +* +*/ + #include #include @@ -24,9 +57,9 @@ void create_loop_destroy(void* actually_an_int) { grpc_connectivity_state state = grpc_channel_check_connectivity_state(chan, 1); grpc_channel_watch_connectivity_state(chan, state, later_time, cq, NULL); - GPR_ASSERT(grpc_completion_queue_next( - cq, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(3), NULL) - .type == GRPC_OP_COMPLETE); + GPR_ASSERT(grpc_completion_queue_next(cq, + GRPC_TIMEOUT_SECONDS_TO_DEADLINE(3), + NULL).type == GRPC_OP_COMPLETE); } grpc_channel_destroy(channels[thread_index]); grpc_completion_queue_destroy(queues[thread_index]); diff --git a/test/core/surface/lame_client_test.c b/test/core/surface/lame_client_test.c index 79e53cb4222..c154d99d4cc 100644 --- a/test/core/surface/lame_client_test.c +++ b/test/core/surface/lame_client_test.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015-2016, Google Inc. + * Copyright 2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without From a45782f090dfd997d2e80bb7761ddc8db4481b0b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 11 Mar 2016 20:15:51 -0800 Subject: [PATCH 47/60] Fix copyright --- test/core/surface/concurrent_connectivity_test.c | 2 +- test/core/surface/lame_client_test.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/core/surface/concurrent_connectivity_test.c b/test/core/surface/concurrent_connectivity_test.c index 5361201db72..4d3b7bf22a3 100644 --- a/test/core/surface/concurrent_connectivity_test.c +++ b/test/core/surface/concurrent_connectivity_test.c @@ -1,6 +1,6 @@ /* * -* Copyright 2015-2016, Google Inc. +* Copyright 2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/test/core/surface/lame_client_test.c b/test/core/surface/lame_client_test.c index c154d99d4cc..79e53cb4222 100644 --- a/test/core/surface/lame_client_test.c +++ b/test/core/surface/lame_client_test.c @@ -1,6 +1,6 @@ /* * - * Copyright 2016, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without From 822f1d7509fe7426a291de1054ea5e9f4892ee15 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 12 Mar 2016 06:54:47 -0800 Subject: [PATCH 48/60] Add comment, tighten loop --- src/core/iomgr/timer.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/core/iomgr/timer.c b/src/core/iomgr/timer.c index 8badd83bbb1..f444643428d 100644 --- a/src/core/iomgr/timer.c +++ b/src/core/iomgr/timer.c @@ -33,11 +33,11 @@ #include "src/core/iomgr/timer.h" -#include "src/core/iomgr/timer_heap.h" -#include "src/core/iomgr/time_averaged_stats.h" #include #include #include +#include "src/core/iomgr/time_averaged_stats.h" +#include "src/core/iomgr/timer_heap.h" #define INVALID_HEAP_INDEX 0xffffffffu @@ -331,8 +331,17 @@ static int run_some_expired_timers(grpc_exec_ctx *exec_ctx, gpr_timespec now, gpr_mu_unlock(&g_mu); gpr_mu_unlock(&g_checker_mu); } else if (next != NULL) { + /* TODO(ctiller): this forces calling code to do an short poll, and + then retry the timer check (because this time through the timer list was + contended). + + We could reduce the cost here dramatically by keeping a count of how many + currently active pollers got through the uncontended case above + successfully, and waking up other pollers IFF that count drops to zero. + + Once that count is in place, this entire else branch could disappear. */ *next = gpr_time_min( - *next, gpr_time_add(now, gpr_time_from_millis(100, GPR_TIMESPAN))); + *next, gpr_time_add(now, gpr_time_from_millis(1, GPR_TIMESPAN))); } return (int)n; From 2480f98e192f9e9a00c7cabcd6a77321f1d9525e Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Fri, 26 Feb 2016 14:18:46 -0800 Subject: [PATCH 49/60] Add a script to run interop tests against all versions of prod servers. --- tools/jenkins/run_full_interop.sh | 37 ++++++++++++++++++++++++++++ tools/run_tests/run_interop_tests.py | 6 ++++- 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 tools/jenkins/run_full_interop.sh diff --git a/tools/jenkins/run_full_interop.sh b/tools/jenkins/run_full_interop.sh new file mode 100644 index 00000000000..a82da1cb68c --- /dev/null +++ b/tools/jenkins/run_full_interop.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# Copyright 2016, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +# This script is invoked by Jenkins and runs interop test suite. +set -ex + +# Enter the gRPC repo root +cd $(dirname $0)/../.. + +tools/run_tests/run_interop_tests.py -l all -s all --cloud_to_prod --cloud_to_prod_auth --prod_servers default cloud_gateway gateway_v2 cloud_gateway_v2 gateway_v4 cloud_gateway_v4 --use_docker --http2_interop -t -j 12 $@ || true diff --git a/tools/run_tests/run_interop_tests.py b/tools/run_tests/run_interop_tests.py index 1dc772a8565..86eb52bbde1 100755 --- a/tools/run_tests/run_interop_tests.py +++ b/tools/run_tests/run_interop_tests.py @@ -589,7 +589,11 @@ prod_servers = { 'cloud_gateway': ('216.239.32.255', 'grpc-test.sandbox.googleapis.com', False), 'cloud_gateway_v2': ('216.239.32.255', 'grpc-test2.sandbox.googleapis.com', - True) + True), + 'gateway_v4': ('grpc-test4.sandbox.googleapis.com', + 'grpc-test4.sandbox.googleapis.com', True), + 'cloud_gateway_v4': ('216.239.32.255', 'grpc-test4.sandbox.googleapis.com', + True), } argp = argparse.ArgumentParser(description='Run interop tests.') From c5dd4d47aa0828cb7ba180cef35bf565a57231e5 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 11 Mar 2016 12:57:43 -0800 Subject: [PATCH 50/60] Use a singleton completion queue --- src/objective-c/GRPCClient/private/GRPCCompletionQueue.m | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m index 250bbf03a66..be214d4d36b 100644 --- a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m +++ b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m @@ -41,7 +41,12 @@ const int64_t kGRPCCompletionQueueDefaultTimeoutSecs = 60; @implementation GRPCCompletionQueue + (instancetype)completionQueue { - return [[self alloc] init]; + static GRPCCompletionQueue *singleton = nil; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + singleton = [[self alloc] init]; + }); + return singleton; } - (instancetype)init { From cc43d693e5c715fcbdab1717efc4c84ef836a1fb Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Mon, 14 Mar 2016 15:53:30 -0700 Subject: [PATCH 51/60] Reduce the number of threads so that this test works on x86 systems --- test/core/support/thd_test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/core/support/thd_test.c b/test/core/support/thd_test.c index f7807d280a2..2671dd3d3d5 100644 --- a/test/core/support/thd_test.c +++ b/test/core/support/thd_test.c @@ -79,9 +79,9 @@ static void test_options(void) { static void test(void) { int i; gpr_thd_id thd; - gpr_thd_id thds[1000]; + gpr_thd_id thds[300]; struct test t; - int n = 1000; + int n = 300; gpr_thd_options options = gpr_thd_options_default(); gpr_mu_init(&t.mu); gpr_cv_init(&t.done_cv); From 9506ef292259625f675d58875937053e2623a400 Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Wed, 2 Mar 2016 13:53:34 -0800 Subject: [PATCH 52/60] Fix code coverage. --- .../multilang_jessie_x64/Dockerfile.template | 43 +++++ .../test/multilang_jessie_x64/Dockerfile | 155 ++++++++++++++++++ tools/run_tests/run_tests.py | 15 +- 3 files changed, 209 insertions(+), 4 deletions(-) create mode 100644 templates/tools/dockerfile/test/multilang_jessie_x64/Dockerfile.template create mode 100644 tools/dockerfile/test/multilang_jessie_x64/Dockerfile diff --git a/templates/tools/dockerfile/test/multilang_jessie_x64/Dockerfile.template b/templates/tools/dockerfile/test/multilang_jessie_x64/Dockerfile.template new file mode 100644 index 00000000000..fdf44312ec3 --- /dev/null +++ b/templates/tools/dockerfile/test/multilang_jessie_x64/Dockerfile.template @@ -0,0 +1,43 @@ +%YAML 1.2 +--- | + # Copyright 2016, Google Inc. + # All rights reserved. + # + # Redistribution and use in source and binary forms, with or without + # modification, are permitted provided that the following conditions are + # met: + # + # * Redistributions of source code must retain the above copyright + # notice, this list of conditions and the following disclaimer. + # * Redistributions in binary form must reproduce the above + # copyright notice, this list of conditions and the following disclaimer + # in the documentation and/or other materials provided with the + # distribution. + # * Neither the name of Google Inc. nor the names of its + # contributors may be used to endorse or promote products derived from + # this software without specific prior written permission. + # + # THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + # "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + # LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + # A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + # OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + # SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + # LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + # DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + # THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + + FROM debian:jessie + + <%include file="../../apt_get_basic.include"/> + <%include file="../../csharp_deps.include"/> + <%include file="../../cxx_deps.include"/> + <%include file="../../node_deps.include"/> + <%include file="../../php_deps.include"/> + <%include file="../../ruby_deps.include"/> + <%include file="../../python_deps.include"/> + <%include file="../../run_tests_addons.include"/> + # Define the default command. + CMD ["bash"] diff --git a/tools/dockerfile/test/multilang_jessie_x64/Dockerfile b/tools/dockerfile/test/multilang_jessie_x64/Dockerfile new file mode 100644 index 00000000000..71ebf2bf715 --- /dev/null +++ b/tools/dockerfile/test/multilang_jessie_x64/Dockerfile @@ -0,0 +1,155 @@ +# Copyright 2016, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +FROM debian:jessie + +# Install Git and basic packages. +RUN apt-get update && apt-get install -y \ + autoconf \ + autotools-dev \ + build-essential \ + bzip2 \ + ccache \ + curl \ + gcc \ + gcc-multilib \ + git \ + golang \ + gyp \ + lcov \ + libc6 \ + libc6-dbg \ + libc6-dev \ + libgtest-dev \ + libtool \ + make \ + perl \ + strace \ + python-dev \ + python-setuptools \ + python-yaml \ + telnet \ + unzip \ + wget \ + zip && apt-get clean + +#================ +# Build profiling +RUN apt-get update && apt-get install -y time && apt-get clean + +#================ +# C# dependencies + +# Update to a newer version of mono +RUN apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 3FA7E0328081BFF6A14DA29AA6A19B38D3D831EF +RUN echo "deb http://download.mono-project.com/repo/debian wheezy main" | tee /etc/apt/sources.list.d/mono-xamarin.list +RUN echo "deb http://download.mono-project.com/repo/debian wheezy-apache24-compat main" | tee -a /etc/apt/sources.list.d/mono-xamarin.list +RUN echo "deb http://download.mono-project.com/repo/debian wheezy-libjpeg62-compat main" | tee -a /etc/apt/sources.list.d/mono-xamarin.list +RUN echo "deb http://download.mono-project.com/repo/debian wheezy-libtiff-compat main" | tee -a /etc/apt/sources.list.d/mono-xamarin.list + +# Install dependencies +RUN apt-get update && apt-get -y dist-upgrade && apt-get install -y \ + mono-devel \ + ca-certificates-mono \ + nuget \ + && apt-get clean + +#================= +# C++ dependencies +RUN apt-get update && apt-get -y install libgflags-dev libgtest-dev libc++-dev clang && apt-get clean + +#================== +# Node dependencies + +# Install nvm +RUN touch .profile +RUN curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.25.4/install.sh | bash +RUN /bin/bash -l -c "nvm install 0.12 && npm config set cache /tmp/npm-cache" + +#================= +# PHP dependencies + +# Install dependencies + +RUN /bin/bash -l -c "echo 'deb http://packages.dotdeb.org wheezy-php55 all' \ + >> /etc/apt/sources.list.d/dotdeb.list" +RUN /bin/bash -l -c "echo 'deb-src http://packages.dotdeb.org wheezy-php55 all' \ + >> /etc/apt/sources.list.d/dotdeb.list" +RUN wget http://www.dotdeb.org/dotdeb.gpg -O- | apt-key add - + +RUN apt-get update && apt-get install -y \ + git php5 php5-dev phpunit unzip + +#================== +# Ruby dependencies + +# Install rvm +RUN gpg --keyserver hkp://keys.gnupg.net --recv-keys 409B6B1796C275462A1703113804BB82D39DC0E3 +RUN \curl -sSL https://get.rvm.io | bash -s stable + +# Install Ruby 2.1 +RUN /bin/bash -l -c "rvm install ruby-2.1" +RUN /bin/bash -l -c "rvm use --default ruby-2.1" +RUN /bin/bash -l -c "echo 'gem: --no-ri --no-rdoc' > ~/.gemrc" +RUN /bin/bash -l -c "echo 'export PATH=/usr/local/rvm/bin:$PATH' >> ~/.bashrc" +RUN /bin/bash -l -c "echo 'rvm --default use ruby-2.1' >> ~/.bashrc" +RUN /bin/bash -l -c "gem install bundler --no-ri --no-rdoc" + +#==================== +# Python dependencies + +# Install dependencies + +RUN apt-get update && apt-get install -y \ + python-all-dev \ + python3-all-dev \ + python-pip + +# Install Python packages from PyPI +RUN pip install pip --upgrade +RUN pip install virtualenv +RUN pip install futures==2.2.0 enum34==1.0.4 protobuf==3.0.0a2 tox + +# Prepare ccache +RUN ln -s /usr/bin/ccache /usr/local/bin/gcc +RUN ln -s /usr/bin/ccache /usr/local/bin/g++ +RUN ln -s /usr/bin/ccache /usr/local/bin/cc +RUN ln -s /usr/bin/ccache /usr/local/bin/c++ +RUN ln -s /usr/bin/ccache /usr/local/bin/clang +RUN ln -s /usr/bin/ccache /usr/local/bin/clang++ + +#====================== +# Zookeeper dependencies +# TODO(jtattermusch): is zookeeper still needed? +RUN apt-get install -y libzookeeper-mt-dev + +RUN mkdir /var/local/jenkins + +# Define the default command. +CMD ["bash"] diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index aa5073fbbd2..99bf5df1f92 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -869,10 +869,17 @@ if args.use_docker: dockerfile_dirs = set([l.dockerfile_dir() for l in languages]) if len(dockerfile_dirs) > 1: - print 'Languages to be tested require running under different docker images.' - sys.exit(1) - dockerfile_dir = next(iter(dockerfile_dirs)) - + if 'gcov' in args.config: + dockerfile_dir = 'tools/dockerfile/test/multilang_jessie_x64' + print ('Using multilang_jessie_x64 docker image for code coverage for ' + 'all languages.') + else: + print ('Languages to be tested require running under different docker ' + 'images.') + sys.exit(1) + else: + dockerfile_dir = next(iter(dockerfile_dirs)) + child_argv = [ arg for arg in sys.argv if not arg == '--use_docker' ] run_tests_cmd = 'python tools/run_tests/run_tests.py %s' % ' '.join(child_argv[1:]) From b19489c6c8ae7d3da0bb3830eef0ad91ddcfe506 Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Mon, 14 Mar 2016 16:12:14 -0700 Subject: [PATCH 53/60] Minor code cleanup --- test/core/support/thd_test.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/core/support/thd_test.c b/test/core/support/thd_test.c index 2671dd3d3d5..4b5e41eac78 100644 --- a/test/core/support/thd_test.c +++ b/test/core/support/thd_test.c @@ -41,6 +41,8 @@ #include #include "test/core/util/test_config.h" +#define NUM_THREADS 300 + struct test { gpr_mu mu; int n; @@ -79,15 +81,14 @@ static void test_options(void) { static void test(void) { int i; gpr_thd_id thd; - gpr_thd_id thds[300]; + gpr_thd_id thds[NUM_THREADS]; struct test t; - int n = 300; gpr_thd_options options = gpr_thd_options_default(); gpr_mu_init(&t.mu); gpr_cv_init(&t.done_cv); - t.n = n; + t.n = NUM_THREADS; t.is_done = 0; - for (i = 0; i != n; i++) { + for (i = 0; i < NUM_THREADS; i++) { GPR_ASSERT(gpr_thd_new(&thd, &thd_body, &t, NULL)); } gpr_mu_lock(&t.mu); @@ -97,10 +98,10 @@ static void test(void) { gpr_mu_unlock(&t.mu); GPR_ASSERT(t.n == 0); gpr_thd_options_set_joinable(&options); - for (i = 0; i < n; i++) { + for (i = 0; i < NUM_THREADS; i++) { GPR_ASSERT(gpr_thd_new(&thds[i], &thd_body_joinable, NULL, &options)); } - for (i = 0; i < n; i++) { + for (i = 0; i < NUM_THREADS; i++) { gpr_thd_join(thds[i]); } } From 47d841de4d56314153775349df74feeaa814c6c7 Mon Sep 17 00:00:00 2001 From: Paul Querna Date: Thu, 10 Mar 2016 11:19:17 -0800 Subject: [PATCH 54/60] Add support for IP Addresses in Subject Alt Names. --- src/core/tsi/ssl_transport_security.c | 67 +++++++-- src/core/tsi/ssl_transport_security.h | 3 +- test/core/tsi/transport_security_test.c | 186 ++++++++++++++---------- test/cpp/end2end/end2end_test.cc | 3 +- 4 files changed, 160 insertions(+), 99 deletions(-) diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c index 6adcaac9ede..7d0c74bb878 100644 --- a/src/core/tsi/ssl_transport_security.c +++ b/src/core/tsi/ssl_transport_security.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -33,8 +33,15 @@ #include "src/core/tsi/ssl_transport_security.h" +#include + #include #include +#ifdef GPR_WINSOCK_SOCKET +#include +#else +#include +#endif #include #include @@ -296,21 +303,44 @@ static tsi_result add_subject_alt_names_properties_to_peer( sk_GENERAL_NAME_value(subject_alt_names, TSI_SIZE_AS_SIZE(i)); /* Filter out the non-dns entries names. */ if (subject_alt_name->type == GEN_DNS) { - unsigned char *dns_name = NULL; - int dns_name_size = - ASN1_STRING_to_UTF8(&dns_name, subject_alt_name->d.dNSName); - if (dns_name_size < 0) { + unsigned char *name = NULL; + int name_size; + name_size = ASN1_STRING_to_UTF8(&name, subject_alt_name->d.dNSName); + if (name_size < 0) { gpr_log(GPR_ERROR, "Could not get utf8 from asn1 string."); result = TSI_INTERNAL_ERROR; break; } result = tsi_construct_string_peer_property( - TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, - (const char *)dns_name, (size_t)dns_name_size, + TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, (const char *)name, + (size_t)name_size, &peer->properties[peer->property_count++]); + OPENSSL_free(name); + } else if (subject_alt_name->type == GEN_IPADD) { + char ntop_buf[INET6_ADDRSTRLEN]; + int af; + + if (subject_alt_name->d.iPAddress->length == 4) { + af = AF_INET; + } else if (subject_alt_name->d.iPAddress->length == 16) { + af = AF_INET6; + } else { + gpr_log(GPR_ERROR, "SAN IP Address contained invalid IP"); + result = TSI_INTERNAL_ERROR; + break; + } + const char *name = inet_ntop(af, subject_alt_name->d.iPAddress->data, + ntop_buf, INET6_ADDRSTRLEN); + if (name == NULL) { + gpr_log(GPR_ERROR, "Could not get IP string from asn1 octet."); + result = TSI_INTERNAL_ERROR; + break; + } + + result = tsi_construct_string_peer_property_from_cstring( + TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, name, &peer->properties[peer->property_count++]); - OPENSSL_free(dns_name); - if (result != TSI_OK) break; } + if (result != TSI_OK) break; } return result; } @@ -1436,9 +1466,7 @@ int tsi_ssl_peer_matches_name(const tsi_peer *peer, const char *name) { size_t i = 0; size_t san_count = 0; const tsi_peer_property *cn_property = NULL; - - /* For now reject what looks like an IP address. */ - if (looks_like_ip_address(name)) return 0; + int like_ip = looks_like_ip_address(name); /* Check the SAN first. */ for (i = 0; i < peer->property_count; i++) { @@ -1447,8 +1475,15 @@ int tsi_ssl_peer_matches_name(const tsi_peer *peer, const char *name) { if (strcmp(property->name, TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY) == 0) { san_count++; - if (does_entry_match_name(property->value.data, property->value.length, - name)) { + + if (!like_ip && does_entry_match_name(property->value.data, + property->value.length, name)) { + return 1; + } else if (like_ip && + strncmp(name, property->value.data, property->value.length) == + 0 && + strlen(name) == property->value.length) { + /* IP Addresses are exact matches only. */ return 1; } } else if (strcmp(property->name, @@ -1457,8 +1492,8 @@ int tsi_ssl_peer_matches_name(const tsi_peer *peer, const char *name) { } } - /* If there's no SAN, try the CN. */ - if (san_count == 0 && cn_property != NULL) { + /* If there's no SAN, try the CN, but only if its not like an IP Address */ + if (san_count == 0 && cn_property != NULL && !like_ip) { if (does_entry_match_name(cn_property->value.data, cn_property->value.length, name)) { return 1; diff --git a/src/core/tsi/ssl_transport_security.h b/src/core/tsi/ssl_transport_security.h index 51c0003a854..b587d7ce31a 100644 --- a/src/core/tsi/ssl_transport_security.h +++ b/src/core/tsi/ssl_transport_security.h @@ -162,8 +162,7 @@ void tsi_ssl_handshaker_factory_destroy(tsi_ssl_handshaker_factory *self); Still TODO(jboeuf): - handle mixed case. - handle %encoded chars. - - handle public suffix wildchar more strictly (e.g. *.co.uk) - - handle IP addresses in SAN. */ + - handle public suffix wildchar more strictly (e.g. *.co.uk) */ int tsi_ssl_peer_matches_name(const tsi_peer *peer, const char *name); #ifdef __cplusplus diff --git a/test/core/tsi/transport_security_test.c b/test/core/tsi/transport_security_test.c index 7ce343987bb..f1e7f7f8a87 100644 --- a/test/core/tsi/transport_security_test.c +++ b/test/core/tsi/transport_security_test.c @@ -61,35 +61,37 @@ typedef struct { of '#' will be replaced with a null character before processing. */ const char *dns_names; + /* Comma separated list of IP SANs to match aggainst */ + const char *ip_names; } cert_name_test_entry; /* Largely inspired from: chromium/src/net/cert/x509_certificate_unittest.cc. TODO(jboeuf) uncomment test cases as we fix tsi_ssl_peer_matches_name. */ const cert_name_test_entry cert_name_test_entries[] = { - {1, "foo.com", "foo.com", NULL}, - {1, "f", "f", NULL}, - {0, "h", "i", NULL}, - {1, "bar.foo.com", "*.foo.com", NULL}, + {1, "foo.com", "foo.com", NULL, NULL}, + {1, "f", "f", NULL, NULL}, + {0, "h", "i", NULL, NULL}, + {1, "bar.foo.com", "*.foo.com", NULL, NULL}, {1, "www.test.fr", "common.name", - "*.test.com,*.test.co.uk,*.test.de,*.test.fr"}, + "*.test.com,*.test.co.uk,*.test.de,*.test.fr", NULL}, /* {1, "wwW.tESt.fr", "common.name", ",*.*,*.test.de,*.test.FR,www"}, */ - {0, "f.uk", ".uk", NULL}, - {0, "w.bar.foo.com", "?.bar.foo.com", NULL}, - {0, "www.foo.com", "(www|ftp).foo.com", NULL}, - {0, "www.foo.com", "www.foo.com#", NULL}, /* # = null char. */ - {0, "www.foo.com", "", "www.foo.com#*.foo.com,#,#"}, - {0, "www.house.example", "ww.house.example", NULL}, - {0, "test.org", "", "www.test.org,*.test.org,*.org"}, - {0, "w.bar.foo.com", "w*.bar.foo.com", NULL}, - {0, "www.bar.foo.com", "ww*ww.bar.foo.com", NULL}, - {0, "wwww.bar.foo.com", "ww*ww.bar.foo.com", NULL}, - {0, "wwww.bar.foo.com", "w*w.bar.foo.com", NULL}, - {0, "wwww.bar.foo.com", "w*w.bar.foo.c0m", NULL}, - {0, "WALLY.bar.foo.com", "wa*.bar.foo.com", NULL}, - {0, "wally.bar.foo.com", "*Ly.bar.foo.com", NULL}, + {0, "f.uk", ".uk", NULL, NULL}, + {0, "w.bar.foo.com", "?.bar.foo.com", NULL, NULL}, + {0, "www.foo.com", "(www|ftp).foo.com", NULL, NULL}, + {0, "www.foo.com", "www.foo.com#", NULL, NULL}, /* # = null char. */ + {0, "www.foo.com", "", "www.foo.com#*.foo.com,#,#", NULL}, + {0, "www.house.example", "ww.house.example", NULL, NULL}, + {0, "test.org", "", "www.test.org,*.test.org,*.org", NULL}, + {0, "w.bar.foo.com", "w*.bar.foo.com", NULL, NULL}, + {0, "www.bar.foo.com", "ww*ww.bar.foo.com", NULL, NULL}, + {0, "wwww.bar.foo.com", "ww*ww.bar.foo.com", NULL, NULL}, + {0, "wwww.bar.foo.com", "w*w.bar.foo.com", NULL, NULL}, + {0, "wwww.bar.foo.com", "w*w.bar.foo.c0m", NULL, NULL}, + {0, "WALLY.bar.foo.com", "wa*.bar.foo.com", NULL, NULL}, + {0, "wally.bar.foo.com", "*Ly.bar.foo.com", NULL, NULL}, /* {1, "ww%57.foo.com", "", "www.foo.com"}, {1, "www&.foo.com", "www%26.foo.com", NULL}, @@ -97,94 +99,101 @@ const cert_name_test_entry cert_name_test_entries[] = { /* Common name must not be used if subject alternative name was provided. */ {0, "www.test.co.jp", "www.test.co.jp", - "*.test.de,*.jp,www.test.co.uk,www.*.co.jp"}, + "*.test.de,*.jp,www.test.co.uk,www.*.co.jp", NULL}, {0, "www.bar.foo.com", "www.bar.foo.com", - "*.foo.com,*.*.foo.com,*.*.bar.foo.com,*..bar.foo.com,"}, + "*.foo.com,*.*.foo.com,*.*.bar.foo.com,*..bar.foo.com,", NULL}, /* IDN tests */ - {1, "xn--poema-9qae5a.com.br", "xn--poema-9qae5a.com.br", NULL}, - {1, "www.xn--poema-9qae5a.com.br", "*.xn--poema-9qae5a.com.br", NULL}, + {1, "xn--poema-9qae5a.com.br", "xn--poema-9qae5a.com.br", NULL, NULL}, + {1, "www.xn--poema-9qae5a.com.br", "*.xn--poema-9qae5a.com.br", NULL, NULL}, {0, "xn--poema-9qae5a.com.br", "", "*.xn--poema-9qae5a.com.br," "xn--poema-*.com.br," "xn--*-9qae5a.com.br," - "*--poema-9qae5a.com.br"}, + "*--poema-9qae5a.com.br", + NULL}, /* The following are adapted from the examples quoted from http://tools.ietf.org/html/rfc6125#section-6.4.3 (e.g., *.example.com would match foo.example.com but not bar.foo.example.com or example.com). */ - {1, "foo.example.com", "*.example.com", NULL}, - {0, "bar.foo.example.com", "*.example.com", NULL}, - {0, "example.com", "*.example.com", NULL}, + {1, "foo.example.com", "*.example.com", NULL, NULL}, + {0, "bar.foo.example.com", "*.example.com", NULL, NULL}, + {0, "example.com", "*.example.com", NULL, NULL}, /* Partial wildcards are disallowed, though RFC 2818 rules allow them. That is, forms such as baz*.example.net, *baz.example.net, and b*z.example.net should NOT match domains. Instead, the wildcard must always be the left-most label, and only a single label. */ - {0, "baz1.example.net", "baz*.example.net", NULL}, - {0, "foobaz.example.net", "*baz.example.net", NULL}, - {0, "buzz.example.net", "b*z.example.net", NULL}, - {0, "www.test.example.net", "www.*.example.net", NULL}, + {0, "baz1.example.net", "baz*.example.net", NULL, NULL}, + {0, "foobaz.example.net", "*baz.example.net", NULL, NULL}, + {0, "buzz.example.net", "b*z.example.net", NULL, NULL}, + {0, "www.test.example.net", "www.*.example.net", NULL, NULL}, /* Wildcards should not be valid for public registry controlled domains, and unknown/unrecognized domains, at least three domain components must be present. */ - {1, "www.test.example", "*.test.example", NULL}, - {1, "test.example.co.uk", "*.example.co.uk", NULL}, - {0, "test.example", "*.example", NULL}, + {1, "www.test.example", "*.test.example", NULL, NULL}, + {1, "test.example.co.uk", "*.example.co.uk", NULL, NULL}, + {0, "test.example", "*.example", NULL, NULL}, /* {0, "example.co.uk", "*.co.uk", NULL}, */ - {0, "foo.com", "*.com", NULL}, - {0, "foo.us", "*.us", NULL}, - {0, "foo", "*", NULL}, + {0, "foo.com", "*.com", NULL, NULL}, + {0, "foo.us", "*.us", NULL, NULL}, + {0, "foo", "*", NULL, NULL}, /* IDN variants of wildcards and registry controlled domains. */ - {1, "www.xn--poema-9qae5a.com.br", "*.xn--poema-9qae5a.com.br", NULL}, - {1, "test.example.xn--mgbaam7a8h", "*.example.xn--mgbaam7a8h", NULL}, + {1, "www.xn--poema-9qae5a.com.br", "*.xn--poema-9qae5a.com.br", NULL, NULL}, + {1, "test.example.xn--mgbaam7a8h", "*.example.xn--mgbaam7a8h", NULL, NULL}, /* {0, "xn--poema-9qae5a.com.br", "*.com.br", NULL}, */ - {0, "example.xn--mgbaam7a8h", "*.xn--mgbaam7a8h", NULL}, + {0, "example.xn--mgbaam7a8h", "*.xn--mgbaam7a8h", NULL, NULL}, /* Wildcards should be permissible for 'private' registry controlled domains. */ - {1, "www.appspot.com", "*.appspot.com", NULL}, - {1, "foo.s3.amazonaws.com", "*.s3.amazonaws.com", NULL}, + {1, "www.appspot.com", "*.appspot.com", NULL, NULL}, + {1, "foo.s3.amazonaws.com", "*.s3.amazonaws.com", NULL, NULL}, /* Multiple wildcards are not valid. */ - {0, "foo.example.com", "*.*.com", NULL}, - {0, "foo.bar.example.com", "*.bar.*.com", NULL}, + {0, "foo.example.com", "*.*.com", NULL, NULL}, + {0, "foo.bar.example.com", "*.bar.*.com", NULL, NULL}, /* Absolute vs relative DNS name tests. Although not explicitly specified in RFC 6125, absolute reference names (those ending in a .) should match either absolute or relative presented names. */ - {1, "foo.com", "foo.com.", NULL}, - {1, "foo.com.", "foo.com", NULL}, - {1, "foo.com.", "foo.com.", NULL}, - {1, "f", "f.", NULL}, - {1, "f.", "f", NULL}, - {1, "f.", "f.", NULL}, - {1, "www-3.bar.foo.com", "*.bar.foo.com.", NULL}, - {1, "www-3.bar.foo.com.", "*.bar.foo.com", NULL}, - {1, "www-3.bar.foo.com.", "*.bar.foo.com.", NULL}, - {0, ".", ".", NULL}, - {0, "example.com", "*.com.", NULL}, - {0, "example.com.", "*.com", NULL}, - {0, "example.com.", "*.com.", NULL}, - {0, "foo.", "*.", NULL}, - {0, "foo", "*.", NULL}, + {1, "foo.com", "foo.com.", NULL, NULL}, + {1, "foo.com.", "foo.com", NULL, NULL}, + {1, "foo.com.", "foo.com.", NULL, NULL}, + {1, "f", "f.", NULL, NULL}, + {1, "f.", "f", NULL, NULL}, + {1, "f.", "f.", NULL, NULL}, + {1, "www-3.bar.foo.com", "*.bar.foo.com.", NULL, NULL}, + {1, "www-3.bar.foo.com.", "*.bar.foo.com", NULL, NULL}, + {1, "www-3.bar.foo.com.", "*.bar.foo.com.", NULL, NULL}, + {0, ".", ".", NULL, NULL}, + {0, "example.com", "*.com.", NULL, NULL}, + {0, "example.com.", "*.com", NULL, NULL}, + {0, "example.com.", "*.com.", NULL, NULL}, + {0, "foo.", "*.", NULL, NULL}, + {0, "foo", "*.", NULL, NULL}, /* {0, "foo.co.uk", "*.co.uk.", NULL}, {0, "foo.co.uk.", "*.co.uk.", NULL}, */ /* An empty CN is OK. */ - {1, "test.foo.com", "", "test.foo.com"}, + {1, "test.foo.com", "", "test.foo.com", NULL}, /* An IP should not be used for the CN. */ - {0, "173.194.195.139", "173.194.195.139", NULL}, + {0, "173.194.195.139", "173.194.195.139", NULL, NULL}, + /* An IP can be used if the SAN IP is present */ + {1, "173.194.195.139", "foo.example.com", NULL, "173.194.195.139"}, + {0, "173.194.195.139", "foo.example.com", NULL, "8.8.8.8"}, + {0, "173.194.195.139", "foo.example.com", NULL, "8.8.8.8,8.8.4.4"}, + {1, "173.194.195.139", "foo.example.com", NULL, "8.8.8.8,173.194.195.139"}, + {0, "173.194.195.139", "foo.example.com", NULL, "173.194.195.13"}, }; typedef struct name_list { @@ -196,7 +205,7 @@ typedef struct { size_t name_count; char *buffer; name_list *names; -} parsed_dns_names; +} parsed_names; name_list *name_list_add(const char *n) { name_list *result = gpr_malloc(sizeof(name_list)); @@ -205,18 +214,18 @@ name_list *name_list_add(const char *n) { return result; } -static parsed_dns_names parse_dns_names(const char *dns_names_str) { - parsed_dns_names result; +static parsed_names parse_names(const char *names_str) { + parsed_names result; name_list *current_nl; size_t i; - memset(&result, 0, sizeof(parsed_dns_names)); - if (dns_names_str == 0) return result; + memset(&result, 0, sizeof(parsed_names)); + if (names_str == 0) return result; result.name_count = 1; - result.buffer = gpr_strdup(dns_names_str); + result.buffer = gpr_strdup(names_str); result.names = name_list_add(result.buffer); current_nl = result.names; - for (i = 0; i < strlen(dns_names_str); i++) { - if (dns_names_str[i] == ',') { + for (i = 0; i < strlen(names_str); i++) { + if (names_str[i] == ',') { result.buffer[i] = '\0'; result.name_count++; i++; @@ -227,7 +236,7 @@ static parsed_dns_names parse_dns_names(const char *dns_names_str) { return result; } -static void destruct_parsed_dns_names(parsed_dns_names *pdn) { +static void destruct_parsed_names(parsed_names *pdn) { name_list *nl = pdn->names; if (pdn->buffer != NULL) gpr_free(pdn->buffer); while (nl != NULL) { @@ -237,8 +246,8 @@ static void destruct_parsed_dns_names(parsed_dns_names *pdn) { } } -static char *processed_dns_name(const char *dns_name) { - char *result = gpr_strdup(dns_name); +static char *processed_name(const char *name) { + char *result = gpr_strdup(name); size_t i; for (i = 0; i < strlen(result); i++) { if (result[i] == '#') { @@ -253,31 +262,48 @@ static tsi_peer peer_from_cert_name_test_entry( size_t i; tsi_peer peer; name_list *nl; - parsed_dns_names dns_entries = parse_dns_names(entry->dns_names); + parsed_names dns_entries = parse_names(entry->dns_names); + parsed_names ip_entries = parse_names(entry->ip_names); nl = dns_entries.names; - GPR_ASSERT(tsi_construct_peer(1 + dns_entries.name_count, &peer) == TSI_OK); + GPR_ASSERT(tsi_construct_peer( + 1 + dns_entries.name_count + ip_entries.name_count, &peer) == + TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, entry->common_name, &peer.properties[0]) == TSI_OK); i = 1; while (nl != NULL) { - char *processed = processed_dns_name(nl->name); + char *processed = processed_name(nl->name); GPR_ASSERT(tsi_construct_string_peer_property( TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, processed, strlen(nl->name), &peer.properties[i++]) == TSI_OK); nl = nl->next; gpr_free(processed); } - destruct_parsed_dns_names(&dns_entries); + + nl = ip_entries.names; + while (nl != NULL) { + char *processed = processed_name(nl->name); + GPR_ASSERT(tsi_construct_string_peer_property( + TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, processed, + strlen(nl->name), &peer.properties[i++]) == TSI_OK); + nl = nl->next; + gpr_free(processed); + } + destruct_parsed_names(&dns_entries); + destruct_parsed_names(&ip_entries); return peer; } char *cert_name_test_entry_to_string(const cert_name_test_entry *entry) { char *s; - gpr_asprintf( - &s, "{ success = %s, host_name = %s, common_name = %s, dns_names = %s}", - entry->expected ? "true" : "false", entry->host_name, entry->common_name, - entry->dns_names != NULL ? entry->dns_names : ""); + gpr_asprintf(&s, + "{ success = %s, host_name = %s, common_name = %s, dns_names = " + "%s, ip_names = %s}", + entry->expected ? "true" : "false", entry->host_name, + entry->common_name, + entry->dns_names != NULL ? entry->dns_names : "", + entry->ip_names != NULL ? entry->ip_names : ""); return s; } diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index 8131a14ff76..47598183220 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -1371,11 +1371,12 @@ TEST_P(SecureEnd2endTest, ClientAuthContext) { if (GetParam().credentials_type == kTlsCredentialsType) { EXPECT_EQ("x509_subject_alternative_name", auth_ctx->GetPeerIdentityPropertyName()); - EXPECT_EQ(3u, auth_ctx->GetPeerIdentity().size()); + EXPECT_EQ(4u, auth_ctx->GetPeerIdentity().size()); EXPECT_EQ("*.test.google.fr", ToString(auth_ctx->GetPeerIdentity()[0])); EXPECT_EQ("waterzooi.test.google.be", ToString(auth_ctx->GetPeerIdentity()[1])); EXPECT_EQ("*.test.youtube.com", ToString(auth_ctx->GetPeerIdentity()[2])); + EXPECT_EQ("192.168.1.3", ToString(auth_ctx->GetPeerIdentity()[3])); } } From 27df68940577764f4a98a1972d9958005e0ef858 Mon Sep 17 00:00:00 2001 From: Paul Querna Date: Sun, 13 Mar 2016 13:34:27 -0700 Subject: [PATCH 55/60] Add note about refactor portability header --- src/core/tsi/ssl_transport_security.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c index 7d0c74bb878..d8f7b5b0e0f 100644 --- a/src/core/tsi/ssl_transport_security.c +++ b/src/core/tsi/ssl_transport_security.c @@ -37,6 +37,8 @@ #include #include + +/* TODO(jboeuf): refactor inet_ntop into a portability header. */ #ifdef GPR_WINSOCK_SOCKET #include #else From 4a9e7c4cb155ac3284bdca098227b1a78495a26f Mon Sep 17 00:00:00 2001 From: Paul Querna Date: Sun, 13 Mar 2016 14:00:11 -0700 Subject: [PATCH 56/60] Improve looks_like_ip_address for IPv6 addresses, and add tests --- src/core/tsi/ssl_transport_security.c | 7 +++++-- src/core/tsi/ssl_transport_security.h | 2 +- test/core/tsi/transport_security_test.c | 9 ++++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c index d8f7b5b0e0f..fcbd910f07f 100644 --- a/src/core/tsi/ssl_transport_security.c +++ b/src/core/tsi/ssl_transport_security.c @@ -206,13 +206,16 @@ static void ssl_info_callback(const SSL *ssl, int where, int ret) { } /* Returns 1 if name looks like an IP address, 0 otherwise. - This is a very rough heuristic as it does not handle IPV6 or things like: - 0300.0250.00.01, 0xC0.0Xa8.0x0.0x1, 000030052000001, 0xc0.052000001 */ + This is a very rough heuristic, and only handles IPv6 in hexadecimal form. */ static int looks_like_ip_address(const char *name) { size_t i; size_t dot_count = 0; size_t num_size = 0; for (i = 0; i < strlen(name); i++) { + if (name[i] == ':') { + /* IPv6 Address in hexadecimal form, : is not allowed in DNS names. */ + return 1; + } if (name[i] >= '0' && name[i] <= '9') { if (num_size > 3) return 0; num_size++; diff --git a/src/core/tsi/ssl_transport_security.h b/src/core/tsi/ssl_transport_security.h index b587d7ce31a..4909af4c470 100644 --- a/src/core/tsi/ssl_transport_security.h +++ b/src/core/tsi/ssl_transport_security.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/test/core/tsi/transport_security_test.c b/test/core/tsi/transport_security_test.c index f1e7f7f8a87..667d3f03497 100644 --- a/test/core/tsi/transport_security_test.c +++ b/test/core/tsi/transport_security_test.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -194,6 +194,13 @@ const cert_name_test_entry cert_name_test_entries[] = { {0, "173.194.195.139", "foo.example.com", NULL, "8.8.8.8,8.8.4.4"}, {1, "173.194.195.139", "foo.example.com", NULL, "8.8.8.8,173.194.195.139"}, {0, "173.194.195.139", "foo.example.com", NULL, "173.194.195.13"}, + {0, "2001:db8:a0b:12f0::1", "foo.example.com", NULL, "173.194.195.13"}, + {1, "2001:db8:a0b:12f0::1", "foo.example.com", NULL, + "2001:db8:a0b:12f0::1"}, + {0, "2001:db8:a0b:12f0::1", "foo.example.com", NULL, + "2001:db8:a0b:12f0::2"}, + {1, "2001:db8:a0b:12f0::1", "foo.example.com", NULL, + "2001:db8:a0b:12f0::2,2001:db8:a0b:12f0::1,8.8.8.8"}, }; typedef struct name_list { From f7cea976581a19a06eca5d54089920ead4f788ae Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Mon, 14 Mar 2016 17:01:15 -0700 Subject: [PATCH 57/60] Fix copyright --- test/core/support/thd_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/support/thd_test.c b/test/core/support/thd_test.c index 4b5e41eac78..0c176da2d3c 100644 --- a/test/core/support/thd_test.c +++ b/test/core/support/thd_test.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without From 7cfb396bf6c2194ca9c1bb6514cc57ce1403f3d8 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Mon, 14 Mar 2016 19:03:54 -0700 Subject: [PATCH 58/60] Make copyright not expire one year too soon. --- src/objective-c/RxLibrary/GRXWriteable.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/objective-c/RxLibrary/GRXWriteable.m b/src/objective-c/RxLibrary/GRXWriteable.m index c15ccb17fb9..028ba9b551a 100644 --- a/src/objective-c/RxLibrary/GRXWriteable.m +++ b/src/objective-c/RxLibrary/GRXWriteable.m @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without From 9db726812eceef6705c71fd1817b0658a00aa38f Mon Sep 17 00:00:00 2001 From: Paddy Byers Date: Thu, 10 Mar 2016 22:56:15 +0000 Subject: [PATCH 59/60] Use gpr_free() in gpr_free_aligned() --- src/core/support/alloc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/support/alloc.c b/src/core/support/alloc.c index 0a064b2c188..b99584bd200 100644 --- a/src/core/support/alloc.c +++ b/src/core/support/alloc.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -87,4 +87,4 @@ void *gpr_malloc_aligned(size_t size, size_t alignment_log) { return (void *)ret; } -void gpr_free_aligned(void *ptr) { free(((void **)ptr)[-1]); } +void gpr_free_aligned(void *ptr) { gpr_free(((void **)ptr)[-1]); } From cbd1bce45a605df97aa00a42b7568d08419253f9 Mon Sep 17 00:00:00 2001 From: Masood Malekghassemi Date: Tue, 15 Mar 2016 13:24:10 -0700 Subject: [PATCH 60/60] Fix two ways tests can hang Both have to do with the test runner's handling of the tests. With one it's the read thread somehow outliving the other threads (e.g. with ctrl-C). The other is due to a filled OS-level pipe's buffer causing a block while code is still holding the GIL in some gRPC core function. We can't empty the buffer from Python because the GIL is held, and the OS can't unblock because it's waiting for the buffer to get cleared: deadlock. --- src/python/grpcio/tests/_runner.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/python/grpcio/tests/_runner.py b/src/python/grpcio/tests/_runner.py index b0dbd92a495..32a31ce00e1 100644 --- a/src/python/grpcio/tests/_runner.py +++ b/src/python/grpcio/tests/_runner.py @@ -43,6 +43,13 @@ import uuid from tests import _loader from tests import _result +# This number needs to be large enough to outpace output on stdout and stderr +# from the gRPC core, otherwise we could end up in a potential deadlock. This +# stems from the OS waiting on someone to clear a filled pipe buffer while the +# GIL is held from a write to stderr from gRPC core, but said someone is in +# Python code thus necessitating GIL acquisition. +_READ_BYTES = 2**20 + class CapturePipe(object): """A context-manager pipe to redirect output to a byte array. @@ -76,6 +83,10 @@ class CapturePipe(object): flags = fcntl.fcntl(self._read_fd, fcntl.F_GETFL) fcntl.fcntl(self._read_fd, fcntl.F_SETFL, flags | os.O_NONBLOCK) self._read_thread = threading.Thread(target=self._read) + # If the user wants to exit from the Python program and hits ctrl-C and the + # read thread is somehow deadlocked with something else, the Python code may + # refuse to exit. This prevents that by making the read thread second-class. + self._read_thread.daemon = True self._read_thread.start() def stop(self): @@ -93,7 +104,7 @@ class CapturePipe(object): self.output = bytearray() while True: select.select([self._read_fd], [], []) - read_bytes = os.read(self._read_fd, 1024) + read_bytes = os.read(self._read_fd, _READ_BYTES) if read_bytes: self.output.extend(read_bytes) else: