From d6731628bcedbe150d4f7cd09656bd506979b00f Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 3 Feb 2015 22:44:13 -0800 Subject: [PATCH 01/11] Cleanup documentation --- src/core/surface/call.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/surface/call.c b/src/core/surface/call.c index f3b76364467..8221401a70b 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -163,11 +163,12 @@ struct grpc_call { a request, and is valid iff request_set[op] <= GRPC_IOREQ_OP_COUNT. The set fields are as per the request type specified by op. - Finally, one element of masters[op] is set per active _group_ of ioreq + Finally, one element of masters is set per active _set_ of ioreq operations. It describes work left outstanding, result status, and what work to perform upon operation completion. As one ioreq of each op type can be active at once, by convention we choose the first element - of a the group to be the master. This allows constant time allocation + of the group to be the master -- ie the master of in-progress operation + op is masters[request_set[op]]. This allows constant time allocation and a strong upper bound of a count of masters to be calculated. */ gpr_uint8 request_set[GRPC_IOREQ_OP_COUNT]; grpc_ioreq_data request_data[GRPC_IOREQ_OP_COUNT]; From 9b60fa3acd9b48441f8f0a2062cfd2df7ebeeb8b Mon Sep 17 00:00:00 2001 From: David Klempner Date: Wed, 4 Feb 2015 09:48:53 -0800 Subject: [PATCH 02/11] 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 03/11] 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 c15622b95c8162cf981aa63caf8d764ab1718b09 Mon Sep 17 00:00:00 2001 From: David Klempner Date: Wed, 4 Feb 2015 12:02:17 -0800 Subject: [PATCH 04/11] 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 05/11] 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 06/11] 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 07/11] 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 08/11] 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 09/11] 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 10/11] 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 11/11] 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