From 883ae17420ff81bf311af0edb47abfd022c4b267 Mon Sep 17 00:00:00 2001 From: vjpai Date: Wed, 28 Jan 2015 16:46:11 -0800 Subject: [PATCH 01/28] Make this compile on mac by adding in header files currently used for posix, as opposed to non-existent header file. --- src/core/iomgr/wakeup_fd_nospecial.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/iomgr/wakeup_fd_nospecial.c b/src/core/iomgr/wakeup_fd_nospecial.c index 21e8074d50e..a7b69744e2b 100644 --- a/src/core/iomgr/wakeup_fd_nospecial.c +++ b/src/core/iomgr/wakeup_fd_nospecial.c @@ -37,11 +37,12 @@ */ #include +#include "src/core/iomgr/wakeup_fd_posix.h" +#include "src/core/iomgr/wakeup_fd_pipe.h" +#include #ifndef GPR_POSIX_HAS_SPECIAL_WAKEUP_FD -#include "src/core/iomgr/wakeup_fd.h" - static int check_availability_invalid(void) { return 0; } From 5ca9f921ae2a9b5d8b9111465c447c202e2944cb Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 3 Feb 2015 11:21:11 -0800 Subject: [PATCH 02/28] Changed PHP metadata representation to associative array of arrays of strings --- src/php/ext/grpc/call.c | 107 +++++++++++--------------- src/php/ext/grpc/call.h | 1 + src/php/tests/unit_tests/CallTest.php | 4 +- 3 files changed, 50 insertions(+), 62 deletions(-) diff --git a/src/php/ext/grpc/call.c b/src/php/ext/grpc/call.c index bd3490b3627..6e0ae590617 100644 --- a/src/php/ext/grpc/call.c +++ b/src/php/ext/grpc/call.c @@ -85,73 +85,25 @@ zval *grpc_call_create_metadata_array(int count, grpc_metadata *elements) { memcpy(str_val, elem->value, elem->value_length); if (zend_hash_find(array_hash, str_key, key_len, (void **)data) == SUCCESS) { - switch (Z_TYPE_P(*data)) { - case IS_STRING: - MAKE_STD_ZVAL(inner_array); - array_init(inner_array); - add_next_index_zval(inner_array, *data); - add_assoc_zval(array, str_key, inner_array); - break; - case IS_ARRAY: - inner_array = *data; - break; - default: - zend_throw_exception(zend_exception_get_default(), - "Metadata hash somehow contains wrong types.", - 1 TSRMLS_CC); + if (Z_TYPE_P(*data) != IS_ARRAY) { + zend_throw_exception(zend_exception_get_default(), + "Metadata hash somehow contains wrong types.", + 1 TSRMLS_CC); efree(str_key); efree(str_val); return NULL; } - add_next_index_stringl(inner_array, str_val, elem->value_length, false); + add_next_index_stringl(*data, str_val, elem->value_length, false); } else { - add_assoc_stringl(array, str_key, str_val, elem->value_length, false); + MAKE_STD_ZVAL(inner_array); + array_init(inner_array); + add_next_index_stringl(inner_array, str_val, elem->value_length, false); + add_assoc_zval(array, str_key, inner_array); } } return array; } -int php_grpc_call_add_metadata_array_walk(void *elem TSRMLS_DC, int num_args, - va_list args, - zend_hash_key *hash_key) { - grpc_call_error error_code; - zval **data = (zval **)elem; - grpc_metadata metadata; - grpc_call *call = va_arg(args, grpc_call *); - gpr_uint32 flags = va_arg(args, gpr_uint32); - const char *key; - HashTable *inner_hash; - /* We assume that either two args were passed, and we are in the recursive - case (and the second argument is the key), or one arg was passed and - hash_key is the string key. */ - if (num_args > 2) { - key = va_arg(args, const char *); - } else { - /* TODO(mlumish): If possible, check that hash_key is a string */ - key = hash_key->arKey; - } - switch (Z_TYPE_P(*data)) { - case IS_STRING: - metadata.key = (char *)key; - metadata.value = Z_STRVAL_P(*data); - metadata.value_length = Z_STRLEN_P(*data); - error_code = grpc_call_add_metadata_old(call, &metadata, 0u); - MAYBE_THROW_CALL_ERROR(add_metadata, error_code); - break; - case IS_ARRAY: - inner_hash = Z_ARRVAL_P(*data); - zend_hash_apply_with_arguments(inner_hash TSRMLS_CC, - php_grpc_call_add_metadata_array_walk, 3, - call, flags, key); - break; - default: - zend_throw_exception(zend_exception_get_default(), - "Metadata hash somehow contains wrong types.", - 1 TSRMLS_CC); - } - return ZEND_HASH_APPLY_KEEP; -} - /** * Constructs a new instance of the Call class. * @param Channel $channel The channel to associate the call with. Must not be @@ -204,8 +156,18 @@ PHP_METHOD(Call, __construct) { PHP_METHOD(Call, add_metadata) { wrapped_grpc_call *call = (wrapped_grpc_call *)zend_object_store_get_object(getThis() TSRMLS_CC); + grpc_metadata metadata; + grpc_call_error error_code; zval *array; + zval **inner_array; + zval **value; HashTable *array_hash; + HashPosition array_pointer; + HashTable *inner_array_hash; + HashPosition inner_array_pointer; + char *key; + uint key_len; + ulong index; long flags = 0; /* "a|l" == 1 array, 1 optional long */ if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "a|l", &array, &flags) == @@ -216,9 +178,34 @@ PHP_METHOD(Call, add_metadata) { return; } array_hash = Z_ARRVAL_P(array); - zend_hash_apply_with_arguments(array_hash TSRMLS_CC, - php_grpc_call_add_metadata_array_walk, 2, - call->wrapped, (gpr_uint32)flags); + for (zend_hash_internal_pointer_reset_ex(array_hash, &array_pointer); + zend_hash_get_current_data_ex(array_hash, (void**)&inner_array, + &array_pointer) == SUCCESS; + zend_hash_move_forward_ex(array_hash, &array_pointer)) { + if (zend_hash_get_current_key_ex(array_hash, &key, &key_len, &index, 0, + &array_pointer) != HASH_KEY_IS_STRING) { + zend_throw_exception(spl_ce_InvalidArgumentException, + "metadata keys must be strings", 1 TSRMLS_CC); + return; + } + inner_array_hash = Z_ARRVAL_P(array); + for (zend_hash_internal_pointer_reset_ex(inner_array_hash, + &inner_array_pointer); + zend_hash_get_current_data_ex(inner_array_hash, (void**)&value, + &inner_array_pointer) == SUCCESS; + zend_hash_move_forward_ex(inner_array_hash, &inner_array_pointer)) { + if (Z_TYPE_P(*value) != IS_STRING) { + zend_throw_exception(spl_ce_InvalidArgumentException, + "metadata values must be arrays of strings", + 1 TSRMLS_CC); + } + metadata.key = key; + metadata.value = Z_STRVAL_P(*value); + metadata.value_length = Z_STRLEN_P(*value); + error_code = grpc_call_add_metadata_old(call->wrapped, &metadata, 0u); + MAYBE_THROW_CALL_ERROR(add_metadata, error_code); + } + } } /** diff --git a/src/php/ext/grpc/call.h b/src/php/ext/grpc/call.h index 232c5d7cf29..ba1f1e797fb 100644 --- a/src/php/ext/grpc/call.h +++ b/src/php/ext/grpc/call.h @@ -19,6 +19,7 @@ zend_throw_exception(spl_ce_LogicException, \ #func_name " was called incorrectly", \ (long)error_code TSRMLS_CC); \ + return; \ } \ } while (0) diff --git a/src/php/tests/unit_tests/CallTest.php b/src/php/tests/unit_tests/CallTest.php index 795831cb65c..b6ce974e6e9 100755 --- a/src/php/tests/unit_tests/CallTest.php +++ b/src/php/tests/unit_tests/CallTest.php @@ -46,7 +46,7 @@ class CallTest extends PHPUnit_Framework_TestCase{ } public function testAddSingleMetadata() { - $this->call->add_metadata(['key' => 'value'], 0); + $this->call->add_metadata(['key' => ['value']], 0); /* Dummy assert: Checks that the previous call completed without error */ $this->assertTrue(true); } @@ -59,7 +59,7 @@ class CallTest extends PHPUnit_Framework_TestCase{ public function testAddSingleAndMultiValueMetadata() { $this->call->add_metadata( - ['key1' => 'value1', + ['key1' => ['value1'], 'key2' => ['value2', 'value3']], 0); /* Dummy assert: Checks that the previous call completed without error */ $this->assertTrue(true); From 4a0a394758d4f169173c221fe58284fee65970e4 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Tue, 3 Feb 2015 15:04:45 -0800 Subject: [PATCH 03/28] Fixing tsan errors in OpenSSL (#319) - Added cross-platform implementation of gpr_thd_currentid(); - OpenSSL still shows some TSAN errors on OPENSSL_cleanse which is inherently not thread-safe but this should not matter: see http://stackoverflow.com/questions/26433772/why-does-openssl-cleanse-look-so-complex-and-thread-unsafe --- include/grpc/support/thd.h | 13 +++----- include/grpc/support/thd_posix.h | 42 ------------------------- include/grpc/support/thd_win32.h | 44 --------------------------- src/core/support/thd_posix.c | 8 ++++- src/core/support/thd_win32.c | 4 +++ src/core/tsi/ssl_transport_security.c | 22 ++++++++++++++ 6 files changed, 38 insertions(+), 95 deletions(-) delete mode 100644 include/grpc/support/thd_posix.h delete mode 100644 include/grpc/support/thd_win32.h diff --git a/include/grpc/support/thd.h b/include/grpc/support/thd.h index 91e0c9f16f5..92d40b44752 100644 --- a/include/grpc/support/thd.h +++ b/include/grpc/support/thd.h @@ -44,18 +44,12 @@ #include -#if defined(GPR_POSIX_SYNC) -#include -#elif defined(GPR_WIN32) -#include -#else -#error could not determine platform for thd -#endif - #ifdef __cplusplus extern "C" { #endif +typedef gpr_uint64 gpr_thd_id; + /* Thread creation options. */ typedef struct { int flags; /* Flags below can be set here. Default value 0. */ @@ -72,6 +66,9 @@ int gpr_thd_new(gpr_thd_id *t, void (*thd_body)(void *arg), void *arg, /* Return a gpr_thd_options struct with all fields set to defaults. */ gpr_thd_options gpr_thd_options_default(void); +/* Returns the identifier of the current thread. */ +gpr_thd_id gpr_thd_currentid(void); + #ifdef __cplusplus } #endif diff --git a/include/grpc/support/thd_posix.h b/include/grpc/support/thd_posix.h deleted file mode 100644 index b688e45bc54..00000000000 --- a/include/grpc/support/thd_posix.h +++ /dev/null @@ -1,42 +0,0 @@ -/* - * - * Copyright 2014, 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. - * - */ - -#ifndef __GRPC_SUPPORT_THD_POSIX_H__ -#define __GRPC_SUPPORT_THD_POSIX_H__ -/* Posix variant of gpr_thd_platform.h. */ - -#include - -typedef pthread_t gpr_thd_id; - -#endif /* __GRPC_SUPPORT_THD_POSIX_H__ */ diff --git a/include/grpc/support/thd_win32.h b/include/grpc/support/thd_win32.h deleted file mode 100644 index b4ab3c7271f..00000000000 --- a/include/grpc/support/thd_win32.h +++ /dev/null @@ -1,44 +0,0 @@ -/* - * - * Copyright 2014, 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. - * - */ - -#ifndef __GRPC_SUPPORT_THD_WIN32_H__ -#define __GRPC_SUPPORT_THD_WIN32_H__ - -/* Win32 variant of gpr_thd_platform.h */ - -#include -#include - -typedef int gpr_thd_id; - -#endif /* __GRPC_SUPPORT_THD_WIN32_H__ */ diff --git a/src/core/support/thd_posix.c b/src/core/support/thd_posix.c index bac1d9c2203..74ca9424bbc 100644 --- a/src/core/support/thd_posix.c +++ b/src/core/support/thd_posix.c @@ -62,17 +62,19 @@ int gpr_thd_new(gpr_thd_id *t, void (*thd_body)(void *arg), void *arg, const gpr_thd_options *options) { int thread_started; pthread_attr_t attr; + pthread_t p; struct thd_arg *a = gpr_malloc(sizeof(*a)); a->body = thd_body; a->arg = arg; GPR_ASSERT(pthread_attr_init(&attr) == 0); GPR_ASSERT(pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED) == 0); - thread_started = (pthread_create(t, &attr, &thread_body, a) == 0); + thread_started = (pthread_create(&p, &attr, &thread_body, a) == 0); GPR_ASSERT(pthread_attr_destroy(&attr) == 0); if (!thread_started) { gpr_free(a); } + *t = (gpr_thd_id)p; return thread_started; } @@ -82,4 +84,8 @@ gpr_thd_options gpr_thd_options_default(void) { return options; } +gpr_thd_id gpr_thd_currentid(void) { + return (gpr_thd_id)pthread_self(); +} + #endif /* GPR_POSIX_SYNC */ diff --git a/src/core/support/thd_win32.c b/src/core/support/thd_win32.c index 1762f87f3cf..9378f91d211 100644 --- a/src/core/support/thd_win32.c +++ b/src/core/support/thd_win32.c @@ -77,4 +77,8 @@ gpr_thd_options gpr_thd_options_default(void) { return options; } +gpr_thd_id gpr_thd_currentid(void) { + return (gpr_thd_id)GetCurrentThreadId(); +} + #endif /* GPR_WIN32 */ diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c index 0f8cbccb625..e23421fc157 100644 --- a/src/core/tsi/ssl_transport_security.c +++ b/src/core/tsi/ssl_transport_security.c @@ -37,6 +37,7 @@ #include #include +#include #include #include "src/core/tsi/transport_security.h" @@ -103,11 +104,32 @@ typedef struct { /* --- Library Initialization. ---*/ static gpr_once init_openssl_once = GPR_ONCE_INIT; +static gpr_mu *openssl_mutexes = NULL; + +static void openssl_locking_cb(int mode, int type, const char* file, int line) { + if (mode & CRYPTO_LOCK) { + gpr_mu_lock(&openssl_mutexes[type]); + } else { + gpr_mu_unlock(&openssl_mutexes[type]); + } +} + +static unsigned long openssl_thread_id_cb(void) { + return (unsigned long)gpr_thd_currentid(); +} static void init_openssl(void) { + int i; SSL_library_init(); SSL_load_error_strings(); OpenSSL_add_all_algorithms(); + openssl_mutexes = malloc(CRYPTO_num_locks() * sizeof(gpr_mu)); + GPR_ASSERT(openssl_mutexes != NULL); + for (i = 0; i < CRYPTO_num_locks(); i++) { + gpr_mu_init(&openssl_mutexes[i]); + } + CRYPTO_set_locking_callback(openssl_locking_cb); + CRYPTO_set_id_callback(openssl_thread_id_cb); } /* --- Ssl utils. ---*/ From 57db88f1164314c3680a7a9c60dc36b47c8901f9 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Tue, 3 Feb 2015 16:29:31 -0800 Subject: [PATCH 04/28] Addressing nicolasnoble@ comments. --- src/core/support/thd_win32.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/support/thd_win32.c b/src/core/support/thd_win32.c index 9378f91d211..2ee14170484 100644 --- a/src/core/support/thd_win32.c +++ b/src/core/support/thd_win32.c @@ -58,16 +58,18 @@ static DWORD WINAPI thread_body(void *v) { int gpr_thd_new(gpr_thd_id *t, void (*thd_body)(void *arg), void *arg, const gpr_thd_options *options) { HANDLE handle; + DWORD thread_id; struct thd_arg *a = gpr_malloc(sizeof(*a)); a->body = thd_body; a->arg = arg; *t = 0; - handle = CreateThread(NULL, 64 * 1024, thread_body, a, 0, NULL); + handle = CreateThread(NULL, 64 * 1024, thread_body, a, 0, &thread_id); if (handle == NULL) { gpr_free(a); } else { CloseHandle(handle); /* threads are "detached" */ } + *t = (gpr_thd_id)thread_id; return handle != NULL; } From 5db18baab887d3a0157174244a2a501a637b0ba3 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 3 Feb 2015 17:31:19 -0800 Subject: [PATCH 05/28] Corrected error in metadata parser --- src/php/ext/grpc/call.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/php/ext/grpc/call.c b/src/php/ext/grpc/call.c index 6e0ae590617..3bc9ce2bead 100644 --- a/src/php/ext/grpc/call.c +++ b/src/php/ext/grpc/call.c @@ -188,7 +188,13 @@ PHP_METHOD(Call, add_metadata) { "metadata keys must be strings", 1 TSRMLS_CC); return; } - inner_array_hash = Z_ARRVAL_P(array); + if (Z_TYPE_P(*inner_array) != IS_ARRAY) { + zend_throw_exception(spl_ce_InvalidArgumentException, + "metadata values must be arrays", + 1 TSRMLS_CC); + return; + } + inner_array_hash = Z_ARRVAL_P(*inner_array); for (zend_hash_internal_pointer_reset_ex(inner_array_hash, &inner_array_pointer); zend_hash_get_current_data_ex(inner_array_hash, (void**)&value, @@ -198,6 +204,7 @@ PHP_METHOD(Call, add_metadata) { zend_throw_exception(spl_ce_InvalidArgumentException, "metadata values must be arrays of strings", 1 TSRMLS_CC); + return; } metadata.key = key; metadata.value = Z_STRVAL_P(*value); From d96db79b92e089fc1a18a0d11d06fffdcf892033 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 3 Feb 2015 17:44:50 -0800 Subject: [PATCH 06/28] Switched to binding servers to port 0 in tests --- src/php/ext/grpc/server.c | 4 ++-- src/php/tests/unit_tests/CallTest.php | 5 +++-- src/php/tests/unit_tests/EndToEndTest.php | 6 ++---- src/php/tests/unit_tests/SecureEndToEndTest.php | 5 ++--- src/php/tests/util/port_picker.php | 6 ------ 5 files changed, 9 insertions(+), 17 deletions(-) delete mode 100755 src/php/tests/util/port_picker.php diff --git a/src/php/ext/grpc/server.c b/src/php/ext/grpc/server.c index bc4fcf07c9c..47ea38db0c9 100644 --- a/src/php/ext/grpc/server.c +++ b/src/php/ext/grpc/server.c @@ -146,7 +146,7 @@ PHP_METHOD(Server, add_http2_port) { "add_http2_port expects a string", 1 TSRMLS_CC); return; } - RETURN_BOOL(grpc_server_add_http2_port(server->wrapped, addr)); + RETURN_LONG(grpc_server_add_http2_port(server->wrapped, addr)); } PHP_METHOD(Server, add_secure_http2_port) { @@ -161,7 +161,7 @@ PHP_METHOD(Server, add_secure_http2_port) { "add_http2_port expects a string", 1 TSRMLS_CC); return; } - RETURN_BOOL(grpc_server_add_secure_http2_port(server->wrapped, addr)); + RETURN_LONG(grpc_server_add_secure_http2_port(server->wrapped, addr)); } /** diff --git a/src/php/tests/unit_tests/CallTest.php b/src/php/tests/unit_tests/CallTest.php index 795831cb65c..ebf2ba34dda 100755 --- a/src/php/tests/unit_tests/CallTest.php +++ b/src/php/tests/unit_tests/CallTest.php @@ -1,16 +1,17 @@ add_http2_port('localhost:9001'); + self::$port = self::$server->add_http2_port('0.0.0.0:0'); } public function setUp() { $this->cq = new Grpc\CompletionQueue(); - $this->channel = new Grpc\Channel('localhost:9001', []); + $this->channel = new Grpc\Channel('localhost:' . self::$port, []); $this->call = new Grpc\Call($this->channel, '/foo', Grpc\Timeval::inf_future()); diff --git a/src/php/tests/unit_tests/EndToEndTest.php b/src/php/tests/unit_tests/EndToEndTest.php index a2d8029b047..05104c0e121 100755 --- a/src/php/tests/unit_tests/EndToEndTest.php +++ b/src/php/tests/unit_tests/EndToEndTest.php @@ -1,13 +1,11 @@ client_queue = new Grpc\CompletionQueue(); $this->server_queue = new Grpc\CompletionQueue(); $this->server = new Grpc\Server($this->server_queue, []); - $address = '127.0.0.1:' . getNewPort(); - $this->server->add_http2_port($address); - $this->channel = new Grpc\Channel($address, []); + $port = $this->server->add_http2_port('0.0.0.0:0'); + $this->channel = new Grpc\Channel('localhost:' . $port, []); } public function tearDown() { diff --git a/src/php/tests/unit_tests/SecureEndToEndTest.php b/src/php/tests/unit_tests/SecureEndToEndTest.php index 7ba4984bd86..5e95b11b44e 100755 --- a/src/php/tests/unit_tests/SecureEndToEndTest.php +++ b/src/php/tests/unit_tests/SecureEndToEndTest.php @@ -11,10 +11,9 @@ class SecureEndToEndTest extends PHPUnit_Framework_TestCase{ file_get_contents(dirname(__FILE__) . '/../data/server1.pem')); $this->server = new Grpc\Server($this->server_queue, ['credentials' => $server_credentials]); - $address = '127.0.0.1:' . getNewPort(); - $this->server->add_secure_http2_port($address); + $port = $this->server->add_secure_http2_port('0.0.0.0:0'); $this->channel = new Grpc\Channel( - $address, + 'localhost:' . $port, [ 'grpc.ssl_target_name_override' => 'foo.test.google.com', 'credentials' => $credentials diff --git a/src/php/tests/util/port_picker.php b/src/php/tests/util/port_picker.php deleted file mode 100755 index d869d8b0a49..00000000000 --- a/src/php/tests/util/port_picker.php +++ /dev/null @@ -1,6 +0,0 @@ - Date: Tue, 3 Feb 2015 18:16:23 -0800 Subject: [PATCH 07/28] Added cancel_after_first_response interop test --- src/php/tests/interop/interop_client.php | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/php/tests/interop/interop_client.php b/src/php/tests/interop/interop_client.php index d1f994a84b6..afae738c7e0 100755 --- a/src/php/tests/interop/interop_client.php +++ b/src/php/tests/interop/interop_client.php @@ -149,6 +149,25 @@ function pingPong($stub) { 'Call did not complete successfully'); } +function cancelAfterFirstResponse($stub) { + $call = $stub->StreamingInputCall(); + $request = new grpc\testing\StreamingOutputCallRequest(); + $request->setResponseType(grpc\testing\PayloadType::COMPRESSABLE); + $response_parameters = new grpc\testing\ResponseParameters(); + $response_parameters->setSize(31415); + $request->addResponseParameters($response_parameters); + $payload = new grpc\testing\Payload(); + $payload->setBody(str_repeat("\0", 27182)); + $request->setPayload($payload); + + $call->write($request); + $response = $call->read(); + + $call->cancel(); + hardAssert($call->getStatus()->code === Grpc\STATUS_CANCELLED, + 'Call status was not CANCELLED'); +} + $args = getopt('', array('server_host:', 'server_port:', 'test_case:')); if (!array_key_exists('server_host', $args) || !array_key_exists('server_port', $args) || @@ -187,4 +206,6 @@ switch($args['test_case']) { case 'ping_pong': pingPong($stub); break; + case 'cancel_after_first_response': + cancelAfterFirstResponse($stub); } \ No newline at end of file From 554fe351d30f59aee8c9c15338a5545f15375dd2 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 3 Feb 2015 18:18:28 -0800 Subject: [PATCH 08/28] Fixed error in new test --- src/php/tests/interop/interop_client.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/php/tests/interop/interop_client.php b/src/php/tests/interop/interop_client.php index afae738c7e0..5266e9a9fac 100755 --- a/src/php/tests/interop/interop_client.php +++ b/src/php/tests/interop/interop_client.php @@ -150,7 +150,7 @@ function pingPong($stub) { } function cancelAfterFirstResponse($stub) { - $call = $stub->StreamingInputCall(); + $call = $stub->FullDuplexCall(); $request = new grpc\testing\StreamingOutputCallRequest(); $request->setResponseType(grpc\testing\PayloadType::COMPRESSABLE); $response_parameters = new grpc\testing\ResponseParameters(); From e0bfcc9ffb1c6a770b4caefa92ba931b4e109560 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Tue, 3 Feb 2015 22:00:54 -0800 Subject: [PATCH 09/28] Removing removed files from build.json --- build.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/build.json b/build.json index 119e61263fa..a06eee666a0 100644 --- a/build.json +++ b/build.json @@ -220,8 +220,6 @@ "include/grpc/support/sync_posix.h", "include/grpc/support/sync_win32.h", "include/grpc/support/thd.h", - "include/grpc/support/thd_posix.h", - "include/grpc/support/thd_win32.h", "include/grpc/support/time.h", "include/grpc/support/time_posix.h", "include/grpc/support/time_win32.h", From fdbb1b039b156845249374d0a580b7457226b0ae Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Tue, 3 Feb 2015 22:01:58 -0800 Subject: [PATCH 10/28] ... And the project files... --- Makefile | 2 -- vsprojects/vs2013/gpr.vcxproj | 2 -- vsprojects/vs2013/gpr.vcxproj.filters | 6 ------ 3 files changed, 10 deletions(-) diff --git a/Makefile b/Makefile index a3f1aaf7c8e..bd5df721638 100644 --- a/Makefile +++ b/Makefile @@ -1251,8 +1251,6 @@ PUBLIC_HEADERS_C += \ include/grpc/support/sync_posix.h \ include/grpc/support/sync_win32.h \ include/grpc/support/thd.h \ - include/grpc/support/thd_posix.h \ - include/grpc/support/thd_win32.h \ include/grpc/support/time.h \ include/grpc/support/time_posix.h \ include/grpc/support/time_win32.h \ diff --git a/vsprojects/vs2013/gpr.vcxproj b/vsprojects/vs2013/gpr.vcxproj index f71b586aff4..c77a61d7829 100644 --- a/vsprojects/vs2013/gpr.vcxproj +++ b/vsprojects/vs2013/gpr.vcxproj @@ -91,8 +91,6 @@ - - diff --git a/vsprojects/vs2013/gpr.vcxproj.filters b/vsprojects/vs2013/gpr.vcxproj.filters index 013ed4b3c99..e385efcfb24 100644 --- a/vsprojects/vs2013/gpr.vcxproj.filters +++ b/vsprojects/vs2013/gpr.vcxproj.filters @@ -135,12 +135,6 @@ include\grpc\support - - include\grpc\support - - - include\grpc\support - include\grpc\support From b08dcfa77394bf14bf6dac76bc8701592a832a82 Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Wed, 4 Feb 2015 02:47:36 -0800 Subject: [PATCH 11/28] Update docker creation script to use the full boot disk size - automates the disk partitioning described in https://cloud.google.com/compute/docs/disks/persistent-disks#repartitionrootpd Removes an unnecessary semi-colon --- .../new_grpc_docker_builder_on_startup.sh | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/tools/gce_setup/new_grpc_docker_builder_on_startup.sh b/tools/gce_setup/new_grpc_docker_builder_on_startup.sh index 4b20a8be7d7..cfd05415a0e 100755 --- a/tools/gce_setup/new_grpc_docker_builder_on_startup.sh +++ b/tools/gce_setup/new_grpc_docker_builder_on_startup.sh @@ -41,7 +41,82 @@ _source_gs_script() { source $script_path } +# Args: +# $1: numerator +# $2: denominator +# $3: threshold (optional; defaults to $THRESHOLD) +# +# Returns: +# 1 if (numerator / denominator > threshold) +# 0 otherwise +_gce_disk_cmp_ratio() { + local DEFAULT_THRESHOLD="1.1" + local numer="${1}" + local denom="${2}" + local threshold="${3:-${DEFAULT_THRESHOLD}}" + + if `which python > /dev/null 2>&1`; then + python -c "print(1 if (1. * ${numer} / ${denom} > ${threshold}) else 0)" + else + echo "Can't find python; calculation not done." 1>&2 + return 1 + fi +} + +# Repartitions the disk or resizes the file system, depending on the current +# state of the partition table. +# +# Automates the process described in +# - https://cloud.google.com/compute/docs/disks/persistent-disks#repartitionrootpd +_gce_disk_maybe_resize_then_reboot() { + # Determine the size in blocks, of the whole disk and the first partition. + local dev_sda="$(fdisk -s /dev/sda)" + local dev_sda1="$(fdisk -s /dev/sda1)" + local dev_sda1_start="$(sudo fdisk -l /dev/sda | grep /dev/sda1 | sed -e 's/ \+/ /g' | cut -d' ' -f 3)" + + # Use fdisk to + # - first see if the partion 1 is using as much of the disk as it should + # - then to resize the partition if it's not + # + # fdisk(1) flags: + # -c: disable DOS compatibility mode + # -u: change display mode to sectors (from cylinders) + # + # fdisk(1) commands: + # d: delete partition (automatically selects the first one) + # n: new partition + # p: primary + # 1: partition number + # $dev_sda1_start: specify the value for the start sector, the default may be incorrect + # <1 blank lines>: accept the defaults for end sectors + # w: write partition table + if [ $(_gce_disk_cmp_ratio "${dev_sda}" "${dev_sda1}") -eq 1 ]; then + echo "$FUNCNAME: Updating the partition table to use full ${dev_sda} instead ${dev_sda1}" + cat < Date: Wed, 4 Feb 2015 09:48:53 -0800 Subject: [PATCH 12/28] Make gpr_timespec no longer be a typedef for struct timespec in posix The problem is that for the typedef to work we need _POSIX_C_SOURCE to be defined properly before any file that uses gpr_timespec includes anything. This is extremely fragile unless we change CFLAGS, which probably isn't worth doing for this. --- include/grpc/support/time.h | 4 ++-- include/grpc/support/time_posix.h | 5 ++++- src/core/support/sync_posix.c | 11 ++++++++++- src/core/support/time_posix.c | 25 +++++++++++++++++++++---- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/include/grpc/support/time.h b/include/grpc/support/time.h index 6327a2cffb6..70e4afdf6ba 100644 --- a/include/grpc/support/time.h +++ b/include/grpc/support/time.h @@ -34,8 +34,8 @@ #ifndef __GRPC_SUPPORT_TIME_H__ #define __GRPC_SUPPORT_TIME_H__ /* Time support. - We use gpr_timespec, which is typedefed to struct timespec on platforms which - have it. On some machines, absolute times may be in local time. */ + We use gpr_timespec, which is analogous to struct timespec. On some + machines, absolute times may be in local time. */ /* Platform specific header declares gpr_timespec. gpr_timespec contains: diff --git a/include/grpc/support/time_posix.h b/include/grpc/support/time_posix.h index 9ff6f7f4933..85dee5fc212 100644 --- a/include/grpc/support/time_posix.h +++ b/include/grpc/support/time_posix.h @@ -38,6 +38,9 @@ #include #include -typedef struct timespec gpr_timespec; +typedef struct gpr_timespec { + time_t tv_sec; + long tv_nsec; +} gpr_timespec; #endif /* __GRPC_SUPPORT_TIME_POSIX_H__ */ diff --git a/src/core/support/sync_posix.c b/src/core/support/sync_posix.c index 7f0e4a95a43..a28a4c6bf46 100644 --- a/src/core/support/sync_posix.c +++ b/src/core/support/sync_posix.c @@ -33,11 +33,17 @@ /* Posix gpr synchroization support code. */ +#if !defined _POSIX_C_SOURCE || _POSIX_C_SOURCE < 199309L +#undef _POSIX_C_SOURCE +#define _POSIX_C_SOURCE 199309L +#endif + #include #ifdef GPR_POSIX_SYNC #include +#include #include #include #include @@ -67,7 +73,10 @@ int gpr_cv_wait(gpr_cv *cv, gpr_mu *mu, gpr_timespec abs_deadline) { if (gpr_time_cmp(abs_deadline, gpr_inf_future) == 0) { err = pthread_cond_wait(cv, mu); } else { - err = pthread_cond_timedwait(cv, mu, &abs_deadline); + struct timespec abs_deadline_ts; + abs_deadline_ts.tv_sec = abs_deadline.tv_sec; + abs_deadline_ts.tv_nsec = abs_deadline.tv_nsec; + err = pthread_cond_timedwait(cv, mu, &abs_deadline_ts); } GPR_ASSERT(err == 0 || err == ETIMEDOUT || err == EAGAIN); return err == ETIMEDOUT; diff --git a/src/core/support/time_posix.c b/src/core/support/time_posix.c index 9e11f8a865d..7f0f028183e 100644 --- a/src/core/support/time_posix.c +++ b/src/core/support/time_posix.c @@ -34,7 +34,8 @@ /* Posix code for gpr time support. */ /* So we get nanosleep and clock_* */ -#ifndef _POSIX_C_SOURCE +#if !defined _POSIX_C_SOURCE || _POSIX_C_SOURCE < 199309L +#undef _POSIX_C_SOURCE #define _POSIX_C_SOURCE 199309L #endif @@ -47,11 +48,25 @@ #include #include +static struct timespec timespec_from_gpr(gpr_timespec gts) { + struct timespec rv; + rv.tv_sec = gts.tv_sec; + rv.tv_nsec = gts.tv_nsec; + return rv; +} + #if _POSIX_TIMERS > 0 +static gpr_timespec gpr_from_timespec(struct timespec ts) { + gpr_timespec rv; + rv.tv_sec = ts.tv_sec; + rv.tv_nsec = ts.tv_nsec; + return rv; +} + gpr_timespec gpr_now(void) { - gpr_timespec now; + struct timespec now; clock_gettime(CLOCK_REALTIME, &now); - return now; + return gpr_from_timespec(now); } #else /* For some reason Apple's OSes haven't implemented clock_gettime. */ @@ -69,6 +84,7 @@ gpr_timespec gpr_now(void) { void gpr_sleep_until(gpr_timespec until) { gpr_timespec now; gpr_timespec delta; + struct timespec delta_ts; for (;;) { /* We could simplify by using clock_nanosleep instead, but it might be @@ -79,7 +95,8 @@ void gpr_sleep_until(gpr_timespec until) { } delta = gpr_time_sub(until, now); - if (nanosleep(&delta, NULL) == 0) { + delta_ts = timespec_from_gpr(delta); + if (nanosleep(&delta_ts, NULL) == 0) { break; } } From 78b79920afbdc87284c49fc27358d2854ae8fe9c Mon Sep 17 00:00:00 2001 From: David Klempner Date: Wed, 4 Feb 2015 10:18:59 -0800 Subject: [PATCH 13/28] Fix up feature test macros Move all feature test macros to the start of the file and check that they aren't already defined or defined to a lower value than the file needs. Projects should be allowed to put these in CFLAGS and we shouldn't break when they do. --- src/core/iomgr/resolve_address.c | 2 ++ src/core/iomgr/socket_utils_linux.c | 2 ++ src/core/iomgr/socket_utils_posix.c | 1 - src/core/iomgr/tcp_server_posix.c | 6 +++++- src/core/support/log_linux.c | 6 ++++++ src/core/support/log_posix.c | 7 ++++++- src/core/support/string_posix.c | 3 ++- test/core/echo/echo_test.c | 3 +++ test/core/fling/fling_stream_test.c | 3 +++ test/core/fling/fling_test.c | 3 +++ 10 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/core/iomgr/resolve_address.c b/src/core/iomgr/resolve_address.c index 01681168ce4..575f884d91a 100644 --- a/src/core/iomgr/resolve_address.c +++ b/src/core/iomgr/resolve_address.c @@ -31,7 +31,9 @@ * */ +#ifndef _POSIX_SOURCE #define _POSIX_SOURCE +#endif #include "src/core/iomgr/sockaddr.h" #include "src/core/iomgr/resolve_address.h" diff --git a/src/core/iomgr/socket_utils_linux.c b/src/core/iomgr/socket_utils_linux.c index f971cb33bcc..7ef58940c24 100644 --- a/src/core/iomgr/socket_utils_linux.c +++ b/src/core/iomgr/socket_utils_linux.c @@ -31,7 +31,9 @@ * */ +#ifndef _GNU_SOURCE #define _GNU_SOURCE +#endif #include #ifdef GPR_LINUX diff --git a/src/core/iomgr/socket_utils_posix.c b/src/core/iomgr/socket_utils_posix.c index 06c5033d457..9184b2a47cf 100644 --- a/src/core/iomgr/socket_utils_posix.c +++ b/src/core/iomgr/socket_utils_posix.c @@ -35,7 +35,6 @@ #ifdef GPR_POSIX_SOCKETUTILS -#define _BSD_SOURCE #include "src/core/iomgr/socket_utils_posix.h" #include diff --git a/src/core/iomgr/tcp_server_posix.c b/src/core/iomgr/tcp_server_posix.c index d169d232718..091f0aab1a6 100644 --- a/src/core/iomgr/tcp_server_posix.c +++ b/src/core/iomgr/tcp_server_posix.c @@ -31,11 +31,15 @@ * */ +/* FIXME: "posix" files shouldn't be depending on _GNU_SOURCE */ +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif + #include #ifdef GPR_POSIX_SOCKET -#define _GNU_SOURCE #include "src/core/iomgr/tcp_server.h" #include diff --git a/src/core/support/log_linux.c b/src/core/support/log_linux.c index a0307e1a9a4..a64faa98bd9 100644 --- a/src/core/support/log_linux.c +++ b/src/core/support/log_linux.c @@ -31,8 +31,14 @@ * */ +#ifndef _POSIX_SOURCE #define _POSIX_SOURCE +#endif + +#ifndef _GNU_SOURCE #define _GNU_SOURCE +#endif + #include #ifdef GPR_LINUX diff --git a/src/core/support/log_posix.c b/src/core/support/log_posix.c index ab2d2e5a740..05f45de1308 100644 --- a/src/core/support/log_posix.c +++ b/src/core/support/log_posix.c @@ -31,11 +31,16 @@ * */ -#ifndef _POSIX_C_SOURCE +#if !defined _POSIX_C_SOURCE || _POSIX_C_SOURCE < 200112L +#undef _POSIX_C_SOURCE #define _POSIX_C_SOURCE 200112L #endif +/* FIXME: "posix" files probably shouldn't depend on _GNU_SOURCE */ +#ifndef _GNU_SOURCE #define _GNU_SOURCE +#endif + #include #if defined(GPR_POSIX_LOG) diff --git a/src/core/support/string_posix.c b/src/core/support/string_posix.c index 57832810ad3..a6bb8058e6c 100644 --- a/src/core/support/string_posix.c +++ b/src/core/support/string_posix.c @@ -33,7 +33,8 @@ /* Posix code for gpr snprintf support. */ -#ifndef _POSIX_C_SOURCE +#if !defined _POSIX_C_SOURCE || _POSIX_C_SOURCE < 200112L +#undef _POSIX_C_SOURCE #define _POSIX_C_SOURCE 200112L #endif diff --git a/test/core/echo/echo_test.c b/test/core/echo/echo_test.c index 83b83ab7ff7..5450dfbef56 100644 --- a/test/core/echo/echo_test.c +++ b/test/core/echo/echo_test.c @@ -31,7 +31,10 @@ * */ +#ifndef _POSIX_SOURCE #define _POSIX_SOURCE +#endif + #include #include #include diff --git a/test/core/fling/fling_stream_test.c b/test/core/fling/fling_stream_test.c index 7f52fb1bad1..1db2f1a7916 100644 --- a/test/core/fling/fling_stream_test.c +++ b/test/core/fling/fling_stream_test.c @@ -31,7 +31,10 @@ * */ +#ifndef _POSIX_SOURCE #define _POSIX_SOURCE +#endif + #include #include #include diff --git a/test/core/fling/fling_test.c b/test/core/fling/fling_test.c index b2272f20c8e..4f41a21aaaf 100644 --- a/test/core/fling/fling_test.c +++ b/test/core/fling/fling_test.c @@ -31,7 +31,10 @@ * */ +#ifndef _POSIX_SOURCE #define _POSIX_SOURCE +#endif + #include #include #include From dbc0f5940adca898862ff4c0791e5eb08b254045 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Wed, 4 Feb 2015 10:56:32 -0800 Subject: [PATCH 14/28] Add write buffer hint to C++ server handlers so that writes actually go out when expected. --- src/cpp/server/async_server_context.cc | 2 +- src/cpp/server/server_rpc_handler.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cpp/server/async_server_context.cc b/src/cpp/server/async_server_context.cc index 92958111c0c..8e03f7257c7 100644 --- a/src/cpp/server/async_server_context.cc +++ b/src/cpp/server/async_server_context.cc @@ -54,7 +54,7 @@ AsyncServerContext::~AsyncServerContext() { grpc_call_destroy(call_); } void AsyncServerContext::Accept(grpc_completion_queue *cq) { GPR_ASSERT(grpc_call_server_accept(call_, cq, this) == GRPC_CALL_OK); - GPR_ASSERT(grpc_call_server_end_initial_metadata(call_, 0) == GRPC_CALL_OK); + GPR_ASSERT(grpc_call_server_end_initial_metadata(call_, GRPC_WRITE_BUFFER_HINT) == GRPC_CALL_OK); } bool AsyncServerContext::StartRead(google::protobuf::Message *request) { diff --git a/src/cpp/server/server_rpc_handler.cc b/src/cpp/server/server_rpc_handler.cc index 061ac1c2f39..bf02de8b805 100644 --- a/src/cpp/server/server_rpc_handler.cc +++ b/src/cpp/server/server_rpc_handler.cc @@ -77,7 +77,7 @@ void ServerRpcHandler::StartRpc() { if (status.IsOk()) { // Send the response if we get an ok status. - async_server_context_->StartWrite(*response, 0); + async_server_context_->StartWrite(*response, GRPC_WRITE_BUFFER_HINT); type = WaitForNextEvent(); if (type != CompletionQueue::SERVER_WRITE_OK) { status = Status(StatusCode::INTERNAL, "Error writing response."); From 2d03e042e71e76734685215de8d5c6c32e94b3d0 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Wed, 4 Feb 2015 11:07:23 -0800 Subject: [PATCH 15/28] Revert "Make this compile on mac by adding in header files currently used for posix," This reverts commit 883ae17420ff81bf311af0edb47abfd022c4b267. --- src/core/iomgr/wakeup_fd_nospecial.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/iomgr/wakeup_fd_nospecial.c b/src/core/iomgr/wakeup_fd_nospecial.c index a7b69744e2b..21e8074d50e 100644 --- a/src/core/iomgr/wakeup_fd_nospecial.c +++ b/src/core/iomgr/wakeup_fd_nospecial.c @@ -37,12 +37,11 @@ */ #include -#include "src/core/iomgr/wakeup_fd_posix.h" -#include "src/core/iomgr/wakeup_fd_pipe.h" -#include #ifndef GPR_POSIX_HAS_SPECIAL_WAKEUP_FD +#include "src/core/iomgr/wakeup_fd.h" + static int check_availability_invalid(void) { return 0; } From d535bf3bc810e7ef7f3fd7fa651d97f498ad3aa4 Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Tue, 3 Feb 2015 18:40:03 -0800 Subject: [PATCH 16/28] Allow the ruby tests to have service account info --- tools/gce_setup/grpc_docker.sh | 96 ++++++++++++++++++------- tools/gce_setup/shared_startup_funcs.sh | 5 +- 2 files changed, 75 insertions(+), 26 deletions(-) diff --git a/tools/gce_setup/grpc_docker.sh b/tools/gce_setup/grpc_docker.sh index 2e7ed97a87f..44787b18e1b 100755 --- a/tools/gce_setup/grpc_docker.sh +++ b/tools/gce_setup/grpc_docker.sh @@ -822,7 +822,6 @@ grpc_interop_gen_ruby_cmd() { echo $the_cmd } - # constructs the full dockerized java interop test cmd. # # call-seq: @@ -832,12 +831,43 @@ grpc_cloud_prod_gen_ruby_cmd() { local cmd_prefix="sudo docker run grpc/ruby bin/bash -l -c" local test_script="/var/local/git/grpc/src/ruby/bin/interop/interop_client.rb" local test_script+=" --use_tls" - local gfe_flags=" --server_port=443 --server_host=grpc-test.sandbox.google.com --server_host_override=grpc-test.sandbox.google.com" + local gfe_flags=$(_grpc_prod_gfe_flags) local env_prefix="SSL_CERT_FILE=/cacerts/roots.pem" local the_cmd="$cmd_prefix '$env_prefix ruby $test_script $gfe_flags $@'" echo $the_cmd } +# constructs the full dockerized ruby service_account auth interop test cmd. +# +# call-seq: +# flags= .... # generic flags to include the command +# cmd=$($grpc_gen_test_cmd $flags) +grpc_cloud_prod_auth_service_account_creds_gen_ruby_cmd() { + local cmd_prefix="sudo docker run grpc/ruby bin/bash -l -c"; + local test_script="/var/local/git/grpc/src/ruby/bin/interop/interop_client.rb" + local test_script+=" --use_tls" + local gfe_flags=$(_grpc_prod_gfe_flags) + local added_gfe_flags=$(_grpc_svc_acc_test_flags) + local env_prefix="SSL_CERT_FILE=/cacerts/roots.pem" + local the_cmd="$cmd_prefix '$env_prefix ruby $test_script $gfe_flags $added_gfe_flag $@'" + echo $the_cmd +} + +# constructs the full dockerized ruby gce auth interop test cmd. +# +# call-seq: +# flags= .... # generic flags to include the command +# cmd=$($grpc_gen_test_cmd $flags) +grpc_cloud_prod_auth_compute_engine_creds_gen_ruby_cmd() { + local cmd_prefix="sudo docker run grpc/ruby bin/bash -l -c"; + local test_script="/var/local/git/grpc/src/ruby/bin/interop/interop_client.rb" + local test_script+=" --use_tls" + local gfe_flags=$(_grpc_prod_gfe_flags) + local added_gfe_flags=$(_grpc_gce_test_flags) + local env_prefix="SSL_CERT_FILE=/cacerts/roots.pem" + local the_cmd="$cmd_prefix '$env_prefix ruby $test_script $gfe_flags $added_gfe_flag $@'" + echo $the_cmd +} # constructs the full dockerized Go interop test cmd. # @@ -888,7 +918,7 @@ grpc_cloud_prod_gen_java_cmd() { local cmd_prefix="sudo docker run grpc/java"; local test_script="/var/local/git/grpc-java/run-test-client.sh"; local test_script+=" --use_tls=true" - local gfe_flags=" --server_port=443 --server_host=grpc-test.sandbox.google.com --server_host_override=grpc-test.sandbox.google.com" + local gfe_flags=$(_grpc_prod_gfe_flags) local the_cmd="$cmd_prefix $test_script $gfe_flags $@"; echo $the_cmd } @@ -909,19 +939,11 @@ grpc_interop_gen_php_cmd() { echo $the_cmd } -# constructs the full dockerized cpp interop test cmd. -# +# constructs the full dockerized node interop test cmd. # # call-seq: # flags= .... # generic flags to include the command # cmd=$($grpc_gen_test_cmd $flags) -grpc_interop_gen_cxx_cmd() { - local cmd_prefix="sudo docker run grpc/cxx"; - local test_script="/var/local/git/grpc/bins/opt/interop_client --enable_ssl"; - local the_cmd="$cmd_prefix $test_script $@"; - echo $the_cmd -} - grpc_interop_gen_node_cmd() { local cmd_prefix="sudo docker run grpc/node"; local test_script="/usr/bin/nodejs /var/local/git/grpc/src/node/interop/interop_client.js --use_tls=true"; @@ -931,46 +953,70 @@ grpc_interop_gen_node_cmd() { # constructs the full dockerized cpp interop test cmd. # +# call-seq: +# flags= .... # generic flags to include the command +# cmd=$($grpc_gen_test_cmd $flags) +grpc_interop_gen_cxx_cmd() { + local cmd_prefix="sudo docker run grpc/cxx"; + local test_script="/var/local/git/grpc/bins/opt/interop_client --enable_ssl"; + local the_cmd="$cmd_prefix $test_script $@"; + echo $the_cmd +} + +# constructs the full dockerized cpp gce=>prod interop test cmd. # # call-seq: # flags= .... # generic flags to include the command # cmd=$($grpc_gen_test_cmd $flags) grpc_cloud_prod_gen_cxx_cmd() { local cmd_prefix="sudo docker run grpc/cxx"; - local test_script="/var/local/git/grpc/bins/opt/interop_client --enable_ssl"; - local gfe_flags=" --use_prod_roots --server_port=443 --server_host=grpc-test.sandbox.google.com --server_host_override=grpc-test.sandbox.google.com" + local test_script="/var/local/git/grpc/bins/opt/interop_client --enable_ssl --use_prod_roots"; + local gfe_flags=$(_grpc_prod_gfe_flags) local the_cmd="$cmd_prefix $test_script $gfe_flags $@"; echo $the_cmd } -# constructs the full dockerized cpp interop test cmd. -# +# constructs the full dockerized cpp service_account auth interop test cmd. # # call-seq: # flags= .... # generic flags to include the command # cmd=$($grpc_gen_test_cmd $flags) grpc_cloud_prod_auth_service_account_creds_gen_cxx_cmd() { local cmd_prefix="sudo docker run grpc/cxx"; - local test_script="/var/local/git/grpc/bins/opt/interop_client --enable_ssl"; - local gfe_flags=" --use_prod_roots --server_port=443 --server_host=grpc-test.sandbox.google.com --server_host_override=grpc-test.sandbox.google.com" - local added_gfe_flags=" --service_account_key_file=/service_account/stubbyCloudTestingTest-7dd63462c60c.json --oauth_scope=https://www.googleapis.com/auth/xapi.zoo" + local test_script="/var/local/git/grpc/bins/opt/interop_client --enable_ssl --use_prod_roots"; + local gfe_flags=$(_grpc_prod_gfe_flags) + local added_gfe_flags=$(_grpc_svc_acc_test_flags) local the_cmd="$cmd_prefix $test_script $gfe_flags $added_gfe_flags $@"; echo $the_cmd } -# constructs the full dockerized cpp interop test cmd. -# +# constructs the full dockerized cpp gce auth interop test cmd. # # call-seq: # flags= .... # generic flags to include the command # cmd=$($grpc_gen_test_cmd $flags) grpc_cloud_prod_auth_compute_engine_creds_gen_cxx_cmd() { local cmd_prefix="sudo docker run grpc/cxx"; - local test_script="/var/local/git/grpc/bins/opt/interop_client --enable_ssl"; - local gfe_flags=" --use_prod_roots --server_port=443 --server_host=grpc-test.sandbox.google.com --server_host_override=grpc-test.sandbox.google.com" - local added_gfe_flags=" --default_service_account=155450119199-r5aaqa2vqoa9g5mv2m6s3m1l293rlmel@developer.gserviceaccount.com --oauth_scope=https://www.googleapis.com/auth/xapi.zoo" + local test_script="/var/local/git/grpc/bins/opt/interop_client --enable_ssl --use_prod_roots"; + local gfe_flags=$(_grpc_prod_gfe_flags) + local added_gfe_flags=$(_grpc_gce_test_flags) local the_cmd="$cmd_prefix $test_script $gfe_flags $added_gfe_flags $@"; echo $the_cmd } -# TODO(grpc-team): add grpc_interop_gen_xxx_cmd for python|nodejs +# outputs the flags passed to gfe tests +_grpc_prod_gfe_flags() { + echo " --server_port=443 --server_host=grpc-test.sandbox.google.com --server_host_override=grpc-test.sandbox.google.com" +} + +# outputs the flags passed to the service account auth tests +_grpc_svc_acc_test_flags() { + echo " --service_account_key_file=/service_account/stubbyCloudTestingTest-7dd63462c60c.json --oauth_scope=https://www.googleapis.com/auth/xapi.zoo" +} + +# outputs the flags passed to the gcloud auth tests +_grpc_gce_test_flags() { + echo " --default_service_account=155450119199-r5aaqa2vqoa9g5mv2m6s3m1l293rlmel@developer.gserviceaccount.com --oauth_scope=https://www.googleapis.com/auth/xapi.zoo" +} + +# TODO(grpc-team): add grpc_interop_gen_xxx_cmd for python diff --git a/tools/gce_setup/shared_startup_funcs.sh b/tools/gce_setup/shared_startup_funcs.sh index 3410a2a2c8a..eea940864da 100755 --- a/tools/gce_setup/shared_startup_funcs.sh +++ b/tools/gce_setup/shared_startup_funcs.sh @@ -372,7 +372,9 @@ grpc_dockerfile_install() { [[ -d $dockerfile_dir ]] || { echo "$FUNCNAME: not a valid dir: $dockerfile_dir"; return 1; } - # For specific base images, sync the ssh key into the .ssh dir in the dockerfile context + # For specific base images, sync private files. + # + # - the ssh key, ssh certs and/or service account info. [[ $image_label == "grpc/base" ]] && { grpc_docker_sync_github_key $dockerfile_dir/.ssh 'base_ssh_key' || return 1; } @@ -384,6 +386,7 @@ grpc_dockerfile_install() { } [[ $image_label == "grpc/ruby" ]] && { grpc_docker_sync_roots_pem $dockerfile_dir/cacerts || return 1; + grpc_docker_sync_service_account $dockerfile_dir/service_account || return 1; } [[ $image_label == "grpc/cxx" ]] && { grpc_docker_sync_service_account $dockerfile_dir/service_account || return 1; From a6ccffb8a4c72aa3a0d20b0cd95f5135d0e84d14 Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Tue, 3 Feb 2015 18:42:32 -0800 Subject: [PATCH 17/28] Updates the ruby dockerfile to add the service account from the docker context --- tools/dockerfile/grpc_ruby/Dockerfile | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/dockerfile/grpc_ruby/Dockerfile b/tools/dockerfile/grpc_ruby/Dockerfile index c84548c8802..47972e7effa 100644 --- a/tools/dockerfile/grpc_ruby/Dockerfile +++ b/tools/dockerfile/grpc_ruby/Dockerfile @@ -6,17 +6,19 @@ RUN cd /var/local/git/grpc \ && git pull --recurse-submodules \ && git submodule update --init --recursive -# TODO: remove this, once make install is fixed -RUN touch /var/local/git/grpc/include/grpc/support/string.h - # Build the C core. RUN make install_c -C /var/local/git/grpc # Build ruby gRPC and run its tests RUN /bin/bash -l -c 'cd /var/local/git/grpc/src/ruby && bundle && rake' -# Add a cacerts directory containing the Google root pem file, allowing the ruby client to access the production test instance +# Add a cacerts directory containing the Google root pem file, allowing the +# ruby client to access the production test instance ADD cacerts cacerts -# Specify the default command such that the interop server runs on its known testing port +# Add a service_account directory containing the auth creds file +ADD service_account service_account + +# Specify the default command such that the interop server runs on its known +# testing port CMD ["/bin/bash", "-l", "-c", "ruby /var/local/git/grpc/src/ruby/bin/interop/interop_server.rb --use_tls --port 8060"] From 5ebf88075643b0503e2a7ce7537c59b45b0f6394 Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Wed, 4 Feb 2015 03:55:19 -0800 Subject: [PATCH 18/28] Replaces grpc_launch_server with grpc_launch_servers --- tools/gce_setup/grpc_docker.sh | 80 +++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 36 deletions(-) diff --git a/tools/gce_setup/grpc_docker.sh b/tools/gce_setup/grpc_docker.sh index 2e7ed97a87f..913d11480b4 100755 --- a/tools/gce_setup/grpc_docker.sh +++ b/tools/gce_setup/grpc_docker.sh @@ -590,7 +590,7 @@ grpc_sync_images() { done } -grpc_launch_server_args() { +_grpc_launch_servers_args() { [[ -n $1 ]] && { # host host=$1 shift @@ -598,55 +598,63 @@ grpc_launch_server_args() { echo "$FUNCNAME: missing arg: host" 1>&2 return 1 } - - [[ -n $1 ]] && { # server_type - case $1 in - cxx) grpc_port=8010 ;; - go) grpc_port=8020 ;; - java) grpc_port=8030 ;; - node) grpc_port=8040 ;; - python) grpc_port=8050 ;; - ruby) grpc_port=8060 ;; - *) echo "bad server_type: $1" 1>&2; return 1 ;; - esac - docker_label="grpc/$1" - docker_name="grpc_interop_$1" - shift + [[ -n $1 ]] && { + servers="$@" } || { - echo "$FUNCNAME: missing arg: server_type" 1>&2 - return 1 + servers="cxx java go node ruby" + echo "$FUNCNAME: no servers specified, will launch defaults '$servers'" } } -# Launches a server on a docker instance. +# Launches servers on a docker instance. # # call-seq; -# grpc_launch_server +# grpc_launch_servers [server1 server2 ...] +# E.g +# grpc_launch_server grpc-docker-server ruby node # -# Runs the server_type on a GCE instance running docker with server_name -grpc_launch_server() { +# Restarts all the specified servers on the GCE instance +# If no servers are specified, it launches all known servers +grpc_launch_servers() { # declare vars local so that they don't pollute the shell environment # where they this func is used. local grpc_zone grpc_project dry_run # set by _grpc_set_project_and_zone - # set by grpc_launch_server_args - local docker_label docker_name host grpc_port + # set by grpc_launch_servers_args + local servers # set the project zone and check that all necessary args are provided - _grpc_set_project_and_zone -f grpc_launch_server_args "$@" || return 1 + _grpc_set_project_and_zone -f _grpc_launch_server_args "$@" || return 1 gce_has_instance $grpc_project $host || return 1; - cmd="sudo docker kill $docker_name > /dev/null 2>&1; " - cmd+="sudo docker rm $docker_name > /dev/null 2>&1; " - cmd+="sudo docker run -d --name $docker_name" - cmd+=" -p $grpc_port:$grpc_port $docker_label" - local project_opt="--project $grpc_project" - local zone_opt="--zone $grpc_zone" - local ssh_cmd="bash -l -c \"$cmd\"" - echo "will run:" - echo " $ssh_cmd" - echo "on $host" - [[ $dry_run == 1 ]] && return 0 # don't run the command on a dry run - gcloud compute $project_opt ssh $zone_opt $host --command "$cmd" + # launch each of the servers in turn + for server in $servers + do + local grpc_port + case $server in + cxx) grpc_port=8010 ;; + go) grpc_port=8020 ;; + java) grpc_port=8030 ;; + node) grpc_port=8040 ;; + python) grpc_port=8050 ;; + ruby) grpc_port=8060 ;; + *) echo "bad server_type: $1" 1>&2; return 1 ;; + esac + local docker_label="grpc/$server" + local docker_name="grpc_interop_$server" + + cmd="sudo docker kill $docker_name > /dev/null 2>&1; " + cmd+="sudo docker rm $docker_name > /dev/null 2>&1; " + cmd+="sudo docker run -d --name $docker_name" + cmd+=" -p $grpc_port:$grpc_port $docker_label" + local project_opt="--project $grpc_project" + local zone_opt="--zone $grpc_zone" + local ssh_cmd="bash -l -c \"$cmd\"" + echo "will run:" + echo " $ssh_cmd" + echo "on $host" + [[ $dry_run == 1 ]] && return 0 # don't run the command on a dry run + gcloud compute $project_opt ssh $zone_opt $host --command "$cmd" + done } # Runs a test command on a docker instance. From 5d6d005c9908603391f3e5d6e8ee22d4361bb311 Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Wed, 4 Feb 2015 03:55:54 -0800 Subject: [PATCH 19/28] Adds a grpc_show_servers command --- tools/gce_setup/grpc_docker.sh | 45 +++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/tools/gce_setup/grpc_docker.sh b/tools/gce_setup/grpc_docker.sh index 913d11480b4..09c28558a90 100755 --- a/tools/gce_setup/grpc_docker.sh +++ b/tools/gce_setup/grpc_docker.sh @@ -590,6 +590,45 @@ grpc_sync_images() { done } +_grpc_show_servers_args() { + [[ -n $1 ]] && { # host + host=$1 + shift + } || { + echo "$FUNCNAME: missing arg: host" 1>&2 + return 1 + } +} + + +# Shows servers on a docker instance. +# +# call-seq; +# grpc_show_servers +# E.g +# grpc_show_server grpc-docker-server +# +# Shows the grpc servers on the GCE instance +grpc_show_servers() { + # declare vars local so that they don't pollute the shell environment + # where they this func is used. + local grpc_zone grpc_project dry_run # set by _grpc_set_project_and_zone + # set by _grpc_show_servers + local host + + # set the project zone and check that all necessary args are provided + _grpc_set_project_and_zone -f _grpc_show_servers_args "$@" || return 1 + gce_has_instance $grpc_project $host || return 1; + + local cmd="sudo docker ps | grep grpc_" + local ssh_cmd="bash -l -c \"$cmd\"" + echo "will run:" + echo " $ssh_cmd" + echo "on $host" + [[ $dry_run == 1 ]] && continue # don't run the command on a dry run + gcloud compute $project_opt ssh $zone_opt $host --command "$cmd" +} + _grpc_launch_servers_args() { [[ -n $1 ]] && { # host host=$1 @@ -619,11 +658,11 @@ grpc_launch_servers() { # declare vars local so that they don't pollute the shell environment # where they this func is used. local grpc_zone grpc_project dry_run # set by _grpc_set_project_and_zone - # set by grpc_launch_servers_args - local servers + # set by _grpc_launch_servers_args + local host servers # set the project zone and check that all necessary args are provided - _grpc_set_project_and_zone -f _grpc_launch_server_args "$@" || return 1 + _grpc_set_project_and_zone -f _grpc_launch_servers_args "$@" || return 1 gce_has_instance $grpc_project $host || return 1; # launch each of the servers in turn From 3726098a2261024cf989f437a95b1f0d40409d89 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Wed, 4 Feb 2015 11:46:50 -0800 Subject: [PATCH 20/28] Fling client needs to have 4 invocations of event_finish to make this a real ping-pong test --- test/core/fling/client.c | 1 + 1 file changed, 1 insertion(+) diff --git a/test/core/fling/client.c b/test/core/fling/client.c index cd2efc3cd0d..a91dfba9b0e 100644 --- a/test/core/fling/client.c +++ b/test/core/fling/client.c @@ -65,6 +65,7 @@ static void step_ping_pong_request(void) { grpc_event_finish(grpc_completion_queue_next(cq, gpr_inf_future)); grpc_event_finish(grpc_completion_queue_next(cq, gpr_inf_future)); grpc_event_finish(grpc_completion_queue_next(cq, gpr_inf_future)); + grpc_event_finish(grpc_completion_queue_next(cq, gpr_inf_future)); grpc_call_destroy(call); call = NULL; } From c15622b95c8162cf981aa63caf8d764ab1718b09 Mon Sep 17 00:00:00 2001 From: David Klempner Date: Wed, 4 Feb 2015 12:02:17 -0800 Subject: [PATCH 21/28] Remove timeval functions They only had one caller, which could easily be converted to use timespec instead of timeval. --- include/grpc/support/time.h | 4 ---- src/core/support/time.c | 16 ---------------- src/node/ext/timeval.cc | 5 ++--- 3 files changed, 2 insertions(+), 23 deletions(-) diff --git a/include/grpc/support/time.h b/include/grpc/support/time.h index 6327a2cffb6..f8870153b1f 100644 --- a/include/grpc/support/time.h +++ b/include/grpc/support/time.h @@ -103,10 +103,6 @@ int gpr_time_similar(gpr_timespec a, gpr_timespec b, gpr_timespec threshold); /* Sleep until at least 'until' - an absolute timeout */ void gpr_sleep_until(gpr_timespec until); -struct timeval gpr_timeval_from_timespec(gpr_timespec t); - -gpr_timespec gpr_timespec_from_timeval(struct timeval t); - double gpr_timespec_to_micros(gpr_timespec t); #ifdef __cplusplus diff --git a/src/core/support/time.c b/src/core/support/time.c index 97243318fda..268a43c6775 100644 --- a/src/core/support/time.c +++ b/src/core/support/time.c @@ -234,22 +234,6 @@ int gpr_time_similar(gpr_timespec a, gpr_timespec b, gpr_timespec threshold) { } } -struct timeval gpr_timeval_from_timespec(gpr_timespec t) { - /* TODO(klempner): Consider whether this should round up, since it is likely - to be used for delays */ - struct timeval tv; - tv.tv_sec = t.tv_sec; - tv.tv_usec = t.tv_nsec / 1000; - return tv; -} - -gpr_timespec gpr_timespec_from_timeval(struct timeval t) { - gpr_timespec ts; - ts.tv_sec = t.tv_sec; - ts.tv_nsec = t.tv_usec * 1000; - return ts; -} - gpr_int32 gpr_time_to_millis(gpr_timespec t) { if (t.tv_sec >= 2147483) { if (t.tv_sec == 2147483 && t.tv_nsec < 648 * GPR_NS_PER_MS) { diff --git a/src/node/ext/timeval.cc b/src/node/ext/timeval.cc index 687e33576b4..20d52f0963e 100644 --- a/src/node/ext/timeval.cc +++ b/src/node/ext/timeval.cc @@ -56,9 +56,8 @@ double TimespecToMilliseconds(gpr_timespec timespec) { } else if (gpr_time_cmp(timespec, gpr_inf_past) == 0) { return -std::numeric_limits::infinity(); } else { - struct timeval time = gpr_timeval_from_timespec(timespec); - return (static_cast(time.tv_sec) * 1000 + - static_cast(time.tv_usec) / 1000); + return (static_cast(timespec.tv_sec) * 1000 + + static_cast(timespec.tv_nsec) / 1000000); } } From e2b2b1fb676eef687f0ea088aa74a13b52cbf755 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 4 Feb 2015 12:50:06 -0800 Subject: [PATCH 22/28] Fix check for whether we should write to prevent infinite loop --- src/core/surface/call.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/core/surface/call.c b/src/core/surface/call.c index 382909c8652..561c24e547e 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -445,17 +445,16 @@ static void finish_start_step(void *pc, grpc_op_error error) { static send_action choose_send_action(grpc_call *call) { switch (call->write_state) { case WRITE_STATE_INITIAL: - if (call->request_set[GRPC_IOREQ_SEND_INITIAL_METADATA] != - REQSET_EMPTY) { + if (is_op_live(call, GRPC_IOREQ_SEND_INITIAL_METADATA)) { call->write_state = WRITE_STATE_STARTED; return SEND_INITIAL_METADATA; } return SEND_NOTHING; case WRITE_STATE_STARTED: - if (call->request_set[GRPC_IOREQ_SEND_MESSAGE] != REQSET_EMPTY) { + if (is_op_live(call, GRPC_IOREQ_SEND_MESSAGE)) { return SEND_MESSAGE; } - if (call->request_set[GRPC_IOREQ_SEND_CLOSE] != REQSET_EMPTY) { + if (is_op_live(call, GRPC_IOREQ_SEND_CLOSE)) { call->write_state = WRITE_STATE_WRITE_CLOSED; finish_ioreq_op(call, GRPC_IOREQ_SEND_TRAILING_METADATA, GRPC_OP_OK); finish_ioreq_op(call, GRPC_IOREQ_SEND_STATUS, GRPC_OP_OK); From 0c61dc52a17b33fd11e2c85ee7797da517f01df2 Mon Sep 17 00:00:00 2001 From: David Klempner Date: Wed, 4 Feb 2015 13:24:04 -0800 Subject: [PATCH 23/28] Remove the platform specific time headers --- include/grpc/support/time.h | 21 ++++---------- include/grpc/support/time_posix.h | 46 ------------------------------- include/grpc/support/time_win32.h | 46 ------------------------------- 3 files changed, 6 insertions(+), 107 deletions(-) delete mode 100644 include/grpc/support/time_posix.h delete mode 100644 include/grpc/support/time_win32.h diff --git a/include/grpc/support/time.h b/include/grpc/support/time.h index 690fbc10e7d..9fb1d0bc97b 100644 --- a/include/grpc/support/time.h +++ b/include/grpc/support/time.h @@ -37,28 +37,19 @@ We use gpr_timespec, which is analogous to struct timespec. On some machines, absolute times may be in local time. */ -/* Platform specific header declares gpr_timespec. - gpr_timespec contains: - time_t tv_sec; // seconds since start of 1970 - int tv_nsec; // nanoseconds; always in 0..999999999; never negative. - */ - #include - -#if defined(GPR_POSIX_TIME) -#include -#elif defined(GPR_WIN32) -#include -#else -#error could not determine platform for time -#endif - #include +#include #ifdef __cplusplus extern "C" { #endif +typedef struct gpr_timespec { + time_t tv_sec; + int tv_nsec; +} gpr_timespec; + /* Time constants. */ extern const gpr_timespec gpr_time_0; /* The zero time interval. */ extern const gpr_timespec gpr_inf_future; /* The far future */ diff --git a/include/grpc/support/time_posix.h b/include/grpc/support/time_posix.h deleted file mode 100644 index 85dee5fc212..00000000000 --- a/include/grpc/support/time_posix.h +++ /dev/null @@ -1,46 +0,0 @@ -/* - * - * Copyright 2014, 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. - * - */ - -#ifndef __GRPC_SUPPORT_TIME_POSIX_H__ -#define __GRPC_SUPPORT_TIME_POSIX_H__ -/* Posix variant of gpr_time_platform.h */ - -#include -#include - -typedef struct gpr_timespec { - time_t tv_sec; - long tv_nsec; -} gpr_timespec; - -#endif /* __GRPC_SUPPORT_TIME_POSIX_H__ */ diff --git a/include/grpc/support/time_win32.h b/include/grpc/support/time_win32.h deleted file mode 100644 index e62ad64b8f5..00000000000 --- a/include/grpc/support/time_win32.h +++ /dev/null @@ -1,46 +0,0 @@ -/* - * - * Copyright 2014, 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. - * - */ - -#ifndef __GRPC_SUPPORT_TIME_WIN32_H__ -#define __GRPC_SUPPORT_TIME_WIN32_H__ -/* Win32 variant of gpr_time_platform.h */ - -#include -#include - -typedef struct gpr_timespec { - time_t tv_sec; - long tv_nsec; -} gpr_timespec; - -#endif /* __GRPC_SUPPORT_TIME_WIN32_H__ */ From e5437de181fb0ccea7978c6dfa735169ad65cc69 Mon Sep 17 00:00:00 2001 From: David Klempner Date: Wed, 4 Feb 2015 14:06:03 -0800 Subject: [PATCH 24/28] Add a missing mdstr_unref This fixes most of the asan reported leaks. --- src/core/surface/call.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/surface/call.c b/src/core/surface/call.c index 5a24264ccec..2b6f042eb99 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -318,6 +318,7 @@ grpc_call_error grpc_call_cancel_with_status(grpc_call *c, maybe_set_status_code(c, status); if (details) { maybe_set_status_details(c, details); + grpc_mdstr_unref(details); } gpr_mu_unlock(&c->read_mu); return grpc_call_cancel(c); From 5ea99bb81cf34ed721e915bfabd9b974e361e382 Mon Sep 17 00:00:00 2001 From: Nicolas Noble Date: Wed, 4 Feb 2015 14:13:09 -0800 Subject: [PATCH 25/28] Let the http2 transport issue a read request before pumping bytes into it. --- src/core/transport/chttp2_transport.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/core/transport/chttp2_transport.c b/src/core/transport/chttp2_transport.c index 48a10058331..6e5095d87f5 100644 --- a/src/core/transport/chttp2_transport.c +++ b/src/core/transport/chttp2_transport.c @@ -328,6 +328,9 @@ static void maybe_start_some_streams(transport *t); static void become_skip_parser(transport *t); +static void recv_data(void *tp, gpr_slice *slices, size_t nslices, + grpc_endpoint_cb_status error); + /* * CONSTRUCTION/DESTRUCTION/REFCOUNTING */ @@ -382,8 +385,8 @@ static void ref_transport(transport *t) { gpr_ref(&t->refs); } static void init_transport(transport *t, grpc_transport_setup_callback setup, void *arg, const grpc_channel_args *channel_args, - grpc_endpoint *ep, grpc_mdctx *mdctx, - int is_client) { + grpc_endpoint *ep, gpr_slice *slices, size_t nslices, + grpc_mdctx *mdctx, int is_client) { size_t i; int j; grpc_transport_setup_result sr; @@ -422,6 +425,7 @@ static void init_transport(transport *t, grpc_transport_setup_callback setup, gpr_slice_buffer_init(&t->outbuf); gpr_slice_buffer_init(&t->qbuf); grpc_sopb_init(&t->nuke_later_sopb); + grpc_chttp2_hpack_parser_init(&t->hpack_parser, t->metadata_context); if (is_client) { gpr_slice_buffer_add(&t->qbuf, gpr_slice_from_copied_string(CLIENT_CONNECT_STRING)); @@ -476,12 +480,14 @@ static void init_transport(transport *t, grpc_transport_setup_callback setup, ref_transport(t); gpr_mu_unlock(&t->mu); + ref_transport(t); + recv_data(t, slices, nslices, GRPC_ENDPOINT_CB_OK); + sr = setup(arg, &t->base, t->metadata_context); lock(t); t->cb = sr.callbacks; t->cb_user_data = sr.user_data; - grpc_chttp2_hpack_parser_init(&t->hpack_parser, t->metadata_context); t->calling_back = 0; gpr_cv_broadcast(&t->cv); unlock(t); @@ -1769,7 +1775,6 @@ void grpc_create_chttp2_transport(grpc_transport_setup_callback setup, size_t nslices, grpc_mdctx *mdctx, int is_client) { transport *t = gpr_malloc(sizeof(transport)); - init_transport(t, setup, arg, channel_args, ep, mdctx, is_client); - ref_transport(t); - recv_data(t, slices, nslices, GRPC_ENDPOINT_CB_OK); + init_transport(t, setup, arg, channel_args, ep, slices, nslices, mdctx, + is_client); } From 6393dd36f1a3d2c0c1125f65e0c8329e1385e0b6 Mon Sep 17 00:00:00 2001 From: Nicolas Noble Date: Wed, 4 Feb 2015 14:23:39 -0800 Subject: [PATCH 26/28] Fixing build.json by removing files that are no longer present. --- Makefile | 2 -- build.json | 2 -- vsprojects/vs2013/gpr.vcxproj | 2 -- vsprojects/vs2013/gpr.vcxproj.filters | 6 ------ 4 files changed, 12 deletions(-) diff --git a/Makefile b/Makefile index 846d9772d4e..d0b5e34f3e5 100644 --- a/Makefile +++ b/Makefile @@ -1272,8 +1272,6 @@ PUBLIC_HEADERS_C += \ include/grpc/support/sync_win32.h \ include/grpc/support/thd.h \ include/grpc/support/time.h \ - include/grpc/support/time_posix.h \ - include/grpc/support/time_win32.h \ include/grpc/support/useful.h \ LIBGPR_OBJS = $(addprefix objs/$(CONFIG)/, $(addsuffix .o, $(basename $(LIBGPR_SRC)))) diff --git a/build.json b/build.json index 6fa3495aa30..d8a295f35c3 100644 --- a/build.json +++ b/build.json @@ -221,8 +221,6 @@ "include/grpc/support/sync_win32.h", "include/grpc/support/thd.h", "include/grpc/support/time.h", - "include/grpc/support/time_posix.h", - "include/grpc/support/time_win32.h", "include/grpc/support/useful.h" ], "headers": [ diff --git a/vsprojects/vs2013/gpr.vcxproj b/vsprojects/vs2013/gpr.vcxproj index c77a61d7829..0d429ab43de 100644 --- a/vsprojects/vs2013/gpr.vcxproj +++ b/vsprojects/vs2013/gpr.vcxproj @@ -92,8 +92,6 @@ - - diff --git a/vsprojects/vs2013/gpr.vcxproj.filters b/vsprojects/vs2013/gpr.vcxproj.filters index e385efcfb24..a992558654b 100644 --- a/vsprojects/vs2013/gpr.vcxproj.filters +++ b/vsprojects/vs2013/gpr.vcxproj.filters @@ -138,12 +138,6 @@ include\grpc\support - - include\grpc\support - - - include\grpc\support - include\grpc\support From 05fce429e2a6503a839ac2a7f6854dafc6d188e9 Mon Sep 17 00:00:00 2001 From: David Klempner Date: Wed, 4 Feb 2015 14:40:42 -0800 Subject: [PATCH 27/28] Fix a memory leak and a gpr_strdup/free mismatch in json_test --- test/core/json/json_test.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/core/json/json_test.c b/test/core/json/json_test.c index 11659a57161..6d0227ad39b 100644 --- a/test/core/json/json_test.c +++ b/test/core/json/json_test.c @@ -151,7 +151,7 @@ static void test_pairs() { GPR_ASSERT(!json); } - free(scratchpad); + gpr_free(scratchpad); } } @@ -166,6 +166,7 @@ static void test_atypical() { grpc_json_destroy(json->child); json->child = brother; grpc_json_destroy(json); + gpr_free(scratchpad); } int main(int argc, char **argv) { From 1d0302d03df9d5bde57fb326c5df3f6de43ddd6f Mon Sep 17 00:00:00 2001 From: David Klempner Date: Wed, 4 Feb 2015 16:08:01 -0800 Subject: [PATCH 28/28] Add a tsan suppression file with OPENSSL_cleanse and use it in run_tests --- tools/run_tests/run_tests.py | 5 +++-- tools/tsan_suppressions.txt | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 tools/tsan_suppressions.txt diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 280c3f05cb9..cb54c0db82c 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -108,10 +108,11 @@ class PythonLanguage(object): _CONFIGS = { 'dbg': SimpleConfig('dbg'), 'opt': SimpleConfig('opt'), - 'tsan': SimpleConfig('tsan'), + 'tsan': SimpleConfig('tsan', environ={ + 'TSAN_OPTIONS': 'suppressions=tools/tsan_suppressions.txt'}), 'msan': SimpleConfig('msan'), 'asan': SimpleConfig('asan', environ={ - 'ASAN_OPTIONS': 'detect_leaks=1:color=always'}), + 'ASAN_OPTIONS': 'detect_leaks=1:color=always:suppressions=tools/tsan_suppressions.txt'}), 'gcov': SimpleConfig('gcov'), 'memcheck': ValgrindConfig('valgrind', 'memcheck'), 'helgrind': ValgrindConfig('dbg', 'helgrind') diff --git a/tools/tsan_suppressions.txt b/tools/tsan_suppressions.txt new file mode 100644 index 00000000000..23d57f9fd1f --- /dev/null +++ b/tools/tsan_suppressions.txt @@ -0,0 +1,2 @@ +# OPENSSL_cleanse does racy access to a global +race:OPENSSL_cleanse