From 80fa15c15121a7d0ec020dec8bfa3697a96058b6 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 16:10:49 -0800 Subject: [PATCH 01/26] Moving prototype from Google to GitHub I'd started some prototyping work on this change before the move to GitHub; this change restores things. --- include/grpc/grpc.h | 5 + src/core/surface/byte_buffer.c | 10 + src/core/surface/call.c | 149 ++++++++---- test/core/end2end/cq_verifier.c | 11 +- test/core/end2end/cq_verifier.h | 1 - test/core/end2end/dualstack_socket_test.c | 9 +- .../core/end2end/dualstack_socket_test.c.orig | 213 ++++++++++++++++++ test/core/end2end/no_server_test.c | 3 +- test/core/end2end/tests/cancel_after_accept.c | 4 +- .../cancel_after_accept_and_writes_closed.c | 4 +- test/core/end2end/tests/cancel_after_invoke.c | 4 +- .../core/end2end/tests/cancel_before_invoke.c | 3 +- test/core/end2end/tests/disappearing_server.c | 5 +- ..._server_shutdown_finishes_inflight_calls.c | 4 +- .../core/end2end/tests/invoke_large_request.c | 4 +- .../end2end/tests/max_concurrent_streams.c | 23 +- test/core/end2end/tests/ping_pong_streaming.c | 3 +- ...esponse_with_binary_metadata_and_payload.c | 4 +- ...quest_response_with_metadata_and_payload.c | 4 +- .../tests/request_response_with_payload.c | 4 +- ...ponse_with_trailing_metadata_and_payload.c | 4 +- .../tests/request_with_large_metadata.c | 4 +- .../core/end2end/tests/request_with_payload.c | 4 +- .../end2end/tests/simple_delayed_request.c | 4 +- test/core/end2end/tests/simple_request.c | 8 +- test/core/end2end/tests/thread_stress.c | 37 ++- .../writes_done_hangs_with_pending_read.c | 4 +- test/core/surface/lame_client_test.c | 3 +- 28 files changed, 387 insertions(+), 148 deletions(-) create mode 100644 test/core/end2end/dualstack_socket_test.c.orig diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index 323a944f93f..f479fb8a409 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -158,6 +158,7 @@ typedef struct grpc_byte_buffer grpc_byte_buffer; /* Sample helpers to obtain byte buffers (these will certainly move place */ grpc_byte_buffer *grpc_byte_buffer_create(gpr_slice *slices, size_t nslices); +grpc_byte_buffer *grpc_byte_buffer_copy(grpc_byte_buffer *bb); size_t grpc_byte_buffer_length(grpc_byte_buffer *bb); void grpc_byte_buffer_destroy(grpc_byte_buffer *byte_buffer); @@ -319,6 +320,10 @@ grpc_call_error grpc_call_add_metadata(grpc_call *call, grpc_metadata *metadata, Produces a GRPC_FINISHED event with finished_tag when the call has been completed (there may be other events for the call pending at this time) */ +grpc_call_error grpc_call_invoke(grpc_call *call, + grpc_completion_queue *cq, + void *metadata_read_tag, + void *finished_tag, gpr_uint32 flags); grpc_call_error grpc_call_start_invoke(grpc_call *call, grpc_completion_queue *cq, void *invoke_accepted_tag, diff --git a/src/core/surface/byte_buffer.c b/src/core/surface/byte_buffer.c index 27a6c6e33d5..40876228945 100644 --- a/src/core/surface/byte_buffer.c +++ b/src/core/surface/byte_buffer.c @@ -49,6 +49,16 @@ grpc_byte_buffer *grpc_byte_buffer_create(gpr_slice *slices, size_t nslices) { return bb; } +grpc_byte_buffer *grpc_byte_buffer_copy(grpc_byte_buffer *bb) { + switch (bb->type) { + case GRPC_BB_SLICE_BUFFER: + return grpc_byte_buffer_create(bb->data.slice_buffer.slices, bb->data.slice_buffer.count); + } + gpr_log(GPR_INFO, "should never get here"); + abort(); + return NULL; +} + void grpc_byte_buffer_destroy(grpc_byte_buffer *bb) { switch (bb->type) { case GRPC_BB_SLICE_BUFFER: diff --git a/src/core/surface/call.c b/src/core/surface/call.c index 9c5f5064eba..f6d93bd9573 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -173,11 +173,14 @@ struct grpc_call { /* protects variables in this section */ gpr_mu read_mu; + gpr_uint8 received_start; + gpr_uint8 start_ok; gpr_uint8 reads_done; gpr_uint8 received_finish; gpr_uint8 received_metadata; gpr_uint8 have_read; gpr_uint8 have_alarm; + gpr_uint8 pending_writes_done; /* The current outstanding read message tag (only valid if have_read == 1) */ void *read_tag; void *metadata_tag; @@ -189,6 +192,8 @@ struct grpc_call { /* The current outstanding send message/context/invoke/end tag (only valid if have_write == 1) */ void *write_tag; + grpc_byte_buffer *pending_write; + gpr_uint32 pending_write_flags; /* The final status of the call */ grpc_status_code status_code; @@ -230,6 +235,9 @@ grpc_call *grpc_call_create(grpc_channel *channel, call->status_details = NULL; call->received_finish = 0; call->reads_done = 0; + call->received_start = 0; + call->pending_write = NULL; + call->pending_writes_done = 0; grpc_metadata_buffer_init(&call->incoming_metadata); gpr_ref_init(&call->internal_refcount, 1); grpc_call_stack_init(channel_stack, server_transport_data, @@ -330,16 +338,6 @@ grpc_call_error grpc_call_add_metadata(grpc_call *call, grpc_metadata *metadata, return GRPC_CALL_OK; } -static void done_invoke(void *user_data, grpc_op_error error) { - grpc_call *call = user_data; - void *tag = call->write_tag; - - GPR_ASSERT(call->have_write); - call->have_write = 0; - call->write_tag = INVALID_TAG; - grpc_cq_end_invoke_accepted(call->cq, tag, call, NULL, NULL, error); -} - static void finish_call(grpc_call *call) { size_t count; grpc_metadata *elements; @@ -359,6 +357,88 @@ grpc_call_error grpc_call_start_invoke(grpc_call *call, void *invoke_accepted_tag, void *metadata_read_tag, void *finished_tag, gpr_uint32 flags) { + grpc_call_error err = grpc_call_invoke(call, cq, metadata_read_tag, finished_tag, flags); + if (err == GRPC_CALL_OK) { + grpc_cq_begin_op(call->cq, call, GRPC_INVOKE_ACCEPTED); + grpc_cq_end_invoke_accepted(call->cq, invoke_accepted_tag, call, do_nothing, NULL, GRPC_OP_OK); + } + return err; +} + +static void done_write(void *user_data, grpc_op_error error) { + grpc_call *call = user_data; + void *tag = call->write_tag; + + GPR_ASSERT(call->have_write); + call->have_write = 0; + call->write_tag = INVALID_TAG; + grpc_cq_end_write_accepted(call->cq, tag, call, NULL, NULL, error); +} + +static void done_writes_done(void *user_data, grpc_op_error error) { + grpc_call *call = user_data; + void *tag = call->write_tag; + + GPR_ASSERT(call->have_write); + call->have_write = 0; + call->write_tag = INVALID_TAG; + grpc_cq_end_finish_accepted(call->cq, tag, call, NULL, NULL, error); +} + +static void call_started(void *user_data, grpc_op_error error) { + grpc_call *call = user_data; + grpc_call_element *elem; + grpc_byte_buffer *pending_write = NULL; + gpr_uint32 pending_write_flags = 0; + gpr_uint8 pending_writes_done = 0; + int ok; + grpc_call_op op; + + gpr_mu_lock(&call->read_mu); + GPR_ASSERT(!call->received_start); + call->received_start = 1; + ok = call->start_ok = (error == GRPC_OP_OK); + pending_write = call->pending_write; + pending_write_flags = call->pending_write_flags; + pending_writes_done = call->pending_writes_done; + gpr_mu_unlock(&call->read_mu); + + if (pending_write) { + if (ok) { + op.type = GRPC_SEND_MESSAGE; + op.dir = GRPC_CALL_DOWN; + op.flags = pending_write_flags; + op.done_cb = done_write; + op.user_data = call; + op.data.message = pending_write; + + elem = CALL_ELEM_FROM_CALL(call, 0); + elem->filter->call_op(elem, NULL, &op); + } else { + done_write(call, error); + } + grpc_byte_buffer_destroy(pending_write); + } + if (pending_writes_done) { + if (ok) { + op.type = GRPC_SEND_FINISH; + op.dir = GRPC_CALL_DOWN; + op.flags = 0; + op.done_cb = done_writes_done; + op.user_data = call; + + elem = CALL_ELEM_FROM_CALL(call, 0); + elem->filter->call_op(elem, NULL, &op); + } else { + done_writes_done(call, error); + } + } +} + +grpc_call_error grpc_call_invoke(grpc_call *call, + grpc_completion_queue *cq, + void *metadata_read_tag, + void *finished_tag, gpr_uint32 flags) { grpc_call_element *elem; grpc_call_op op; @@ -390,7 +470,6 @@ grpc_call_error grpc_call_start_invoke(grpc_call *call, /* inform the completion queue of an incoming operation */ grpc_cq_begin_op(cq, call, GRPC_FINISHED); grpc_cq_begin_op(cq, call, GRPC_CLIENT_METADATA_READ); - grpc_cq_begin_op(cq, call, GRPC_INVOKE_ACCEPTED); gpr_mu_lock(&call->read_mu); @@ -401,8 +480,6 @@ grpc_call_error grpc_call_start_invoke(grpc_call *call, if (call->received_finish) { /* handle early cancellation */ - grpc_cq_end_invoke_accepted(call->cq, invoke_accepted_tag, call, NULL, NULL, - GRPC_OP_ERROR); grpc_cq_end_client_metadata_read(call->cq, metadata_read_tag, call, NULL, NULL, 0, NULL); finish_call(call); @@ -412,18 +489,15 @@ grpc_call_error grpc_call_start_invoke(grpc_call *call, return GRPC_CALL_OK; } - call->write_tag = invoke_accepted_tag; call->metadata_tag = metadata_read_tag; - call->have_write = 1; - gpr_mu_unlock(&call->read_mu); /* call down the filter stack */ op.type = GRPC_SEND_START; op.dir = GRPC_CALL_DOWN; op.flags = flags; - op.done_cb = done_invoke; + op.done_cb = call_started; op.data.start.pollset = grpc_cq_pollset(cq); op.user_data = call; @@ -516,26 +590,6 @@ grpc_call_error grpc_call_accept(grpc_call *call, grpc_completion_queue *cq, return GRPC_CALL_OK; } -static void done_writes_done(void *user_data, grpc_op_error error) { - grpc_call *call = user_data; - void *tag = call->write_tag; - - GPR_ASSERT(call->have_write); - call->have_write = 0; - call->write_tag = INVALID_TAG; - grpc_cq_end_finish_accepted(call->cq, tag, call, NULL, NULL, error); -} - -static void done_write(void *user_data, grpc_op_error error) { - grpc_call *call = user_data; - void *tag = call->write_tag; - - GPR_ASSERT(call->have_write); - call->have_write = 0; - call->write_tag = INVALID_TAG; - grpc_cq_end_write_accepted(call->cq, tag, call, NULL, NULL, error); -} - void grpc_call_client_initial_metadata_complete( grpc_call_element *surface_element) { grpc_call *call = grpc_call_from_top_element(surface_element); @@ -635,8 +689,6 @@ grpc_call_error grpc_call_start_write(grpc_call *call, grpc_cq_begin_op(call->cq, call, GRPC_WRITE_ACCEPTED); - /* for now we do no buffering, so a NULL byte_buffer can have no impact - on our behavior -- succeed immediately */ /* TODO(ctiller): if flags & GRPC_WRITE_BUFFER_HINT == 0, this indicates a flush, and that flush should be propogated down from here */ if (byte_buffer == NULL) { @@ -647,6 +699,15 @@ grpc_call_error grpc_call_start_write(grpc_call *call, call->write_tag = tag; call->have_write = 1; + gpr_mu_lock(&call->read_mu); + if (!call->received_start) { + call->pending_write = grpc_byte_buffer_copy(byte_buffer); + call->pending_write_flags = flags; + + gpr_mu_unlock(&call->read_mu); + } else { + gpr_mu_unlock(&call->read_mu); + op.type = GRPC_SEND_MESSAGE; op.dir = GRPC_CALL_DOWN; op.flags = flags; @@ -656,6 +717,7 @@ grpc_call_error grpc_call_start_write(grpc_call *call, elem = CALL_ELEM_FROM_CALL(call, 0); elem->filter->call_op(elem, NULL, &op); + } return GRPC_CALL_OK; } @@ -687,6 +749,14 @@ grpc_call_error grpc_call_writes_done(grpc_call *call, void *tag) { call->write_tag = tag; call->have_write = 1; + gpr_mu_lock(&call->read_mu); + if (!call->received_start) { + call->pending_writes_done = 1; + + gpr_mu_unlock(&call->read_mu); + } else { + gpr_mu_unlock(&call->read_mu); + op.type = GRPC_SEND_FINISH; op.dir = GRPC_CALL_DOWN; op.flags = 0; @@ -695,6 +765,7 @@ grpc_call_error grpc_call_writes_done(grpc_call *call, void *tag) { elem = CALL_ELEM_FROM_CALL(call, 0); elem->filter->call_op(elem, NULL, &op); + } return GRPC_CALL_OK; } diff --git a/test/core/end2end/cq_verifier.c b/test/core/end2end/cq_verifier.c index e5b7304743f..ba446c116af 100644 --- a/test/core/end2end/cq_verifier.c +++ b/test/core/end2end/cq_verifier.c @@ -70,7 +70,6 @@ typedef struct expectation { union { grpc_op_error finish_accepted; grpc_op_error write_accepted; - grpc_op_error invoke_accepted; struct { const char *method; const char *host; @@ -182,7 +181,7 @@ static void verify_matches(expectation *e, grpc_event *ev) { GPR_ASSERT(e->data.write_accepted == ev->data.write_accepted); break; case GRPC_INVOKE_ACCEPTED: - GPR_ASSERT(e->data.invoke_accepted == ev->data.invoke_accepted); + abort(); break; case GRPC_SERVER_RPC_NEW: GPR_ASSERT(string_equivalent(e->data.server_rpc_new.method, @@ -268,8 +267,7 @@ static size_t expectation_to_string(char *out, expectation *e) { return sprintf(out, "GRPC_WRITE_ACCEPTED result=%d", e->data.write_accepted); case GRPC_INVOKE_ACCEPTED: - return sprintf(out, "GRPC_INVOKE_ACCEPTED result=%d", - e->data.invoke_accepted); + return sprintf(out, "GRPC_INVOKE_ACCEPTED"); case GRPC_SERVER_RPC_NEW: timeout = gpr_time_sub(e->data.server_rpc_new.deadline, gpr_now()); return sprintf(out, "GRPC_SERVER_RPC_NEW method=%s host=%s timeout=%fsec", @@ -414,11 +412,6 @@ static metadata *metadata_from_args(va_list args) { } } -void cq_expect_invoke_accepted(cq_verifier *v, void *tag, - grpc_op_error result) { - add(v, GRPC_INVOKE_ACCEPTED, tag)->data.invoke_accepted = result; -} - void cq_expect_write_accepted(cq_verifier *v, void *tag, grpc_op_error result) { add(v, GRPC_WRITE_ACCEPTED, tag)->data.write_accepted = result; } diff --git a/test/core/end2end/cq_verifier.h b/test/core/end2end/cq_verifier.h index 9711e0067c1..73d5782a335 100644 --- a/test/core/end2end/cq_verifier.h +++ b/test/core/end2end/cq_verifier.h @@ -56,7 +56,6 @@ void cq_verify_empty(cq_verifier *v); Any functions taking ... expect a NULL terminated list of key/value pairs (each pair using two parameter slots) of metadata that MUST be present in the event. */ -void cq_expect_invoke_accepted(cq_verifier *v, void *tag, grpc_op_error result); void cq_expect_write_accepted(cq_verifier *v, void *tag, grpc_op_error result); void cq_expect_finish_accepted(cq_verifier *v, void *tag, grpc_op_error result); void cq_expect_read(cq_verifier *v, void *tag, gpr_slice bytes); diff --git a/test/core/end2end/dualstack_socket_test.c b/test/core/end2end/dualstack_socket_test.c index b443caa2a67..23ffddc1467 100644 --- a/test/core/end2end/dualstack_socket_test.c +++ b/test/core/end2end/dualstack_socket_test.c @@ -107,13 +107,10 @@ void test_connect(const char *server_host, const char *client_host, int port, GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, client_cq, tag(1), tag(2), tag(3), 0)); + grpc_call_invoke(c, client_cq, tag(2), tag(3), 0)); + GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c, tag(4))); if (expect_ok) { /* Check for a successful request. */ - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); - - GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c, tag(4))); cq_expect_finish_accepted(v_client, tag(4), GRPC_OP_OK); cq_verify(v_client); @@ -142,10 +139,10 @@ void test_connect(const char *server_host, const char *client_host, int port, grpc_call_destroy(s); } else { /* Check for a failed connection. */ - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_ERROR); cq_expect_client_metadata_read(v_client, tag(2), NULL); cq_expect_finished_with_status(v_client, tag(3), GRPC_STATUS_CANCELLED, NULL, NULL); + cq_expect_finish_accepted(v_client, tag(4), GRPC_OP_ERROR); cq_verify(v_client); grpc_call_destroy(c); diff --git a/test/core/end2end/dualstack_socket_test.c.orig b/test/core/end2end/dualstack_socket_test.c.orig new file mode 100644 index 00000000000..b443caa2a67 --- /dev/null +++ b/test/core/end2end/dualstack_socket_test.c.orig @@ -0,0 +1,213 @@ +/* + * + * 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. + * + */ + +#include "src/core/iomgr/socket_utils_posix.h" +#include +#include +#include +#include +#include "test/core/end2end/cq_verifier.h" +#include "test/core/util/port.h" +#include "test/core/util/test_config.h" + +/* This test exercises IPv4, IPv6, and dualstack sockets in various ways. */ + +static void *tag(gpr_intptr i) { return (void *)i; } + +static gpr_timespec ms_from_now(int ms) { + return gpr_time_add(gpr_now(), gpr_time_from_micros(GPR_US_PER_MS * ms)); +} + +static void drain_cq(grpc_completion_queue *cq) { + grpc_event *ev; + grpc_completion_type type; + do { + ev = grpc_completion_queue_next(cq, ms_from_now(5000)); + GPR_ASSERT(ev); + type = ev->type; + grpc_event_finish(ev); + gpr_log(GPR_INFO, "Drained event type %d", type); + } while (type != GRPC_QUEUE_SHUTDOWN); +} + +void test_connect(const char *server_host, const char *client_host, int port, + int expect_ok) { + char *client_hostport; + char *server_hostport; + grpc_channel *client; + grpc_server *server; + grpc_completion_queue *client_cq; + grpc_completion_queue *server_cq; + grpc_call *c; + grpc_call *s; + cq_verifier *v_client; + cq_verifier *v_server; + gpr_timespec deadline; + + gpr_join_host_port(&client_hostport, client_host, port); + gpr_join_host_port(&server_hostport, server_host, port); + gpr_log(GPR_INFO, "Testing with server=%s client=%s (expecting %s)", + server_hostport, client_hostport, expect_ok ? "success" : "failure"); + + /* Create server. */ + server_cq = grpc_completion_queue_create(); + server = grpc_server_create(server_cq, NULL); + GPR_ASSERT(grpc_server_add_http2_port(server, server_hostport)); + grpc_server_start(server); + gpr_free(server_hostport); + v_server = cq_verifier_create(server_cq); + + /* Create client. */ + client_cq = grpc_completion_queue_create(); + client = grpc_channel_create(client_hostport, NULL); + gpr_free(client_hostport); + v_client = cq_verifier_create(client_cq); + + if (expect_ok) { + /* Normal deadline, shouldn't be reached. */ + deadline = ms_from_now(60000); + } else { + /* Give up faster when failure is expected. + BUG: Setting this to 1000 reveals a memory leak (b/18608927). */ + deadline = ms_from_now(1500); + } + + /* Send a trivial request. */ + c = grpc_channel_create_call(client, "/foo", "test.google.com", deadline); + GPR_ASSERT(c); + + GPR_ASSERT(GRPC_CALL_OK == + grpc_call_start_invoke(c, client_cq, tag(1), tag(2), tag(3), 0)); + if (expect_ok) { + /* Check for a successful request. */ + cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); + cq_verify(v_client); + + GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c, tag(4))); + cq_expect_finish_accepted(v_client, tag(4), GRPC_OP_OK); + cq_verify(v_client); + + GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(server, tag(100))); + cq_expect_server_rpc_new(v_server, &s, tag(100), "/foo", "test.google.com", + deadline, NULL); + cq_verify(v_server); + + GPR_ASSERT(GRPC_CALL_OK == grpc_call_accept(s, server_cq, tag(102), 0)); + cq_expect_client_metadata_read(v_client, tag(2), NULL); + cq_verify(v_client); + + GPR_ASSERT(GRPC_CALL_OK == + grpc_call_start_write_status(s, GRPC_STATUS_UNIMPLEMENTED, "xyz", + tag(5))); + cq_expect_finished_with_status(v_client, tag(3), GRPC_STATUS_UNIMPLEMENTED, + "xyz", NULL); + cq_verify(v_client); + + cq_expect_finish_accepted(v_server, tag(5), GRPC_OP_OK); + cq_verify(v_server); + cq_expect_finished(v_server, tag(102), NULL); + cq_verify(v_server); + + grpc_call_destroy(c); + grpc_call_destroy(s); + } else { + /* Check for a failed connection. */ + cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_ERROR); + cq_expect_client_metadata_read(v_client, tag(2), NULL); + cq_expect_finished_with_status(v_client, tag(3), GRPC_STATUS_CANCELLED, + NULL, NULL); + cq_verify(v_client); + + grpc_call_destroy(c); + } + + cq_verifier_destroy(v_client); + cq_verifier_destroy(v_server); + + /* Destroy client. */ + grpc_channel_destroy(client); + grpc_completion_queue_shutdown(client_cq); + drain_cq(client_cq); + grpc_completion_queue_destroy(client_cq); + + /* Destroy server. */ + grpc_server_shutdown(server); + grpc_server_destroy(server); + grpc_completion_queue_shutdown(server_cq); + drain_cq(server_cq); + grpc_completion_queue_destroy(server_cq); +} + +int main(int argc, char **argv) { + int do_ipv6 = 1; + int i; + int port = grpc_pick_unused_port_or_die(); + + grpc_test_init(argc, argv); + grpc_init(); + + if (!grpc_ipv6_loopback_available()) { + gpr_log(GPR_INFO, "Can't bind to ::1. Skipping IPv6 tests."); + do_ipv6 = 0; + } + + for (i = 0; i <= 1; i++) { + /* For coverage, test with and without dualstack sockets. */ + grpc_forbid_dualstack_sockets_for_testing = i; + + /* :: and 0.0.0.0 are handled identically. */ + test_connect("::", "127.0.0.1", port, 1); + test_connect("::", "::ffff:127.0.0.1", port, 1); + test_connect("::", "localhost", port, 1); + test_connect("0.0.0.0", "127.0.0.1", port, 1); + test_connect("0.0.0.0", "::ffff:127.0.0.1", port, 1); + test_connect("0.0.0.0", "localhost", port, 1); + if (do_ipv6) { + test_connect("::", "::1", port, 1); + test_connect("0.0.0.0", "::1", port, 1); + } + + /* These only work when the families agree. */ + test_connect("127.0.0.1", "127.0.0.1", port, 1); + if (do_ipv6) { + test_connect("::1", "::1", port, 1); + test_connect("::1", "127.0.0.1", port, 0); + test_connect("127.0.0.1", "::1", port, 0); + } + + } + + grpc_shutdown(); + + return 0; +} diff --git a/test/core/end2end/no_server_test.c b/test/core/end2end/no_server_test.c index ba6349c109b..0c251ab467a 100644 --- a/test/core/end2end/no_server_test.c +++ b/test/core/end2end/no_server_test.c @@ -57,10 +57,9 @@ int main(int argc, char **argv) { /* create a call, channel to a non existant server */ chan = grpc_channel_create("nonexistant:54321", NULL); call = grpc_channel_create_call(chan, "/foo", "nonexistant", deadline); - GPR_ASSERT(grpc_call_start_invoke(call, cq, tag(1), tag(2), tag(3), 0) == + GPR_ASSERT(grpc_call_invoke(call, cq, tag(2), tag(3), 0) == GRPC_CALL_OK); /* verify that all tags get completed */ - cq_expect_invoke_accepted(cqv, tag(1), GRPC_OP_ERROR); cq_expect_client_metadata_read(cqv, tag(2), NULL); cq_expect_finished_with_status(cqv, tag(3), GRPC_STATUS_CANCELLED, NULL, NULL); diff --git a/test/core/end2end/tests/cancel_after_accept.c b/test/core/end2end/tests/cancel_after_accept.c index 3b570401ef9..5ec13b937c6 100644 --- a/test/core/end2end/tests/cancel_after_accept.c +++ b/test/core/end2end/tests/cancel_after_accept.c @@ -124,9 +124,7 @@ static void test_cancel_after_accept(grpc_end2end_test_config config, GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, tag(100))); cq_expect_server_rpc_new(v_server, &s, tag(100), "/foo", "test.google.com", diff --git a/test/core/end2end/tests/cancel_after_accept_and_writes_closed.c b/test/core/end2end/tests/cancel_after_accept_and_writes_closed.c index e20b0ec0b24..19d240471e3 100644 --- a/test/core/end2end/tests/cancel_after_accept_and_writes_closed.c +++ b/test/core/end2end/tests/cancel_after_accept_and_writes_closed.c @@ -124,9 +124,7 @@ static void test_cancel_after_accept_and_writes_closed( GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, tag(100))); cq_expect_server_rpc_new(v_server, &s, tag(100), "/foo", "test.google.com", diff --git a/test/core/end2end/tests/cancel_after_invoke.c b/test/core/end2end/tests/cancel_after_invoke.c index 4fab9f7effa..ad0dd0c9ec9 100644 --- a/test/core/end2end/tests/cancel_after_invoke.c +++ b/test/core/end2end/tests/cancel_after_invoke.c @@ -122,9 +122,7 @@ static void test_cancel_after_invoke(grpc_end2end_test_config config, GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == call_cancel(c)); diff --git a/test/core/end2end/tests/cancel_before_invoke.c b/test/core/end2end/tests/cancel_before_invoke.c index 2d8d465b249..e523fa2f762 100644 --- a/test/core/end2end/tests/cancel_before_invoke.c +++ b/test/core/end2end/tests/cancel_before_invoke.c @@ -119,8 +119,7 @@ static void test_cancel_before_invoke(grpc_end2end_test_config config) { GPR_ASSERT(GRPC_CALL_OK == grpc_call_cancel(c)); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_ERROR); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); cq_expect_client_metadata_read(v_client, tag(2), NULL); cq_expect_finished_with_status(v_client, tag(3), GRPC_STATUS_CANCELLED, NULL, NULL); diff --git a/test/core/end2end/tests/disappearing_server.c b/test/core/end2end/tests/disappearing_server.c index c421798fc76..1cbb15f32e4 100644 --- a/test/core/end2end/tests/disappearing_server.c +++ b/test/core/end2end/tests/disappearing_server.c @@ -100,11 +100,8 @@ static void do_request_and_shutdown_server(grpc_end2end_test_fixture *f, c = grpc_channel_create_call(f->client, "/foo", "test.google.com", deadline); GPR_ASSERT(c); - GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_invoke(c, f->client_cq, tag(1), + GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(c, f->client_cq, tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - - cq_verify(v_client); GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c, tag(4))); cq_expect_finish_accepted(v_client, tag(4), GRPC_OP_OK); diff --git a/test/core/end2end/tests/early_server_shutdown_finishes_inflight_calls.c b/test/core/end2end/tests/early_server_shutdown_finishes_inflight_calls.c index 3855a7a3de4..824d9eba88a 100644 --- a/test/core/end2end/tests/early_server_shutdown_finishes_inflight_calls.c +++ b/test/core/end2end/tests/early_server_shutdown_finishes_inflight_calls.c @@ -115,9 +115,7 @@ static void test_early_server_shutdown_finishes_inflight_calls( GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c, tag(4))); cq_expect_finish_accepted(v_client, tag(4), GRPC_OP_OK); diff --git a/test/core/end2end/tests/invoke_large_request.c b/test/core/end2end/tests/invoke_large_request.c index bad86fb9dcd..bacecde01bb 100644 --- a/test/core/end2end/tests/invoke_large_request.c +++ b/test/core/end2end/tests/invoke_large_request.c @@ -126,9 +126,7 @@ static void test_invoke_large_request(grpc_end2end_test_config config) { GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_write(c, request_payload, tag(4), 0)); diff --git a/test/core/end2end/tests/max_concurrent_streams.c b/test/core/end2end/tests/max_concurrent_streams.c index a418d1b15f3..e2f30d07789 100644 --- a/test/core/end2end/tests/max_concurrent_streams.c +++ b/test/core/end2end/tests/max_concurrent_streams.c @@ -113,9 +113,7 @@ static void simple_request_body(grpc_end2end_test_fixture f) { GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c, tag(4))); cq_expect_finish_accepted(v_client, tag(4), GRPC_OP_OK); @@ -191,14 +189,17 @@ static void test_max_concurrent_streams(grpc_end2end_test_config config) { GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, tag(100))); - GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_invoke(c1, f.client_cq, tag(300), + GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(c1, f.client_cq, tag(301), tag(302), 0)); - GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_invoke(c2, f.client_cq, tag(400), + GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(c2, f.client_cq, tag(401), tag(402), 0)); + GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c1, tag(303))); + GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c2, tag(303))); + ev = grpc_completion_queue_next( f.client_cq, gpr_time_add(gpr_now(), gpr_time_from_seconds(10))); GPR_ASSERT(ev); - GPR_ASSERT(ev->type == GRPC_INVOKE_ACCEPTED); + GPR_ASSERT(ev->type == GRPC_FINISH_ACCEPTED); GPR_ASSERT(ev->data.invoke_accepted == GRPC_OP_OK); /* The /alpha or /beta calls started above could be invoked (but NOT both); * check this here */ @@ -206,11 +207,6 @@ static void test_max_concurrent_streams(grpc_end2end_test_config config) { live_call_obj = live_call == 300 ? c1 : c2; grpc_event_finish(ev); - GPR_ASSERT(GRPC_CALL_OK == - grpc_call_writes_done(live_call_obj, tag(live_call + 3))); - cq_expect_finish_accepted(v_client, tag(live_call + 3), GRPC_OP_OK); - cq_verify(v_client); - cq_expect_server_rpc_new(v_server, &s1, tag(100), live_call == 300 ? "/alpha" : "/beta", "test.google.com", deadline, NULL); @@ -232,11 +228,6 @@ static void test_max_concurrent_streams(grpc_end2end_test_config config) { GRPC_STATUS_UNIMPLEMENTED, "xyz", NULL); live_call = (live_call == 300) ? 400 : 300; live_call_obj = live_call == 300 ? c1 : c2; - cq_expect_invoke_accepted(v_client, tag(live_call), GRPC_OP_OK); - cq_verify(v_client); - - GPR_ASSERT(GRPC_CALL_OK == - grpc_call_writes_done(live_call_obj, tag(live_call + 3))); cq_expect_finish_accepted(v_client, tag(live_call + 3), GRPC_OP_OK); cq_verify(v_client); diff --git a/test/core/end2end/tests/ping_pong_streaming.c b/test/core/end2end/tests/ping_pong_streaming.c index 02a05e8e0c7..1801d34a2e2 100644 --- a/test/core/end2end/tests/ping_pong_streaming.c +++ b/test/core/end2end/tests/ping_pong_streaming.c @@ -122,8 +122,7 @@ static void test_pingpong_streaming(grpc_end2end_test_config config, GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, tag(100))); diff --git a/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c b/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c index 8a0eea8072f..c6048bf96a3 100644 --- a/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c +++ b/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c @@ -145,9 +145,7 @@ static void test_request_response_with_metadata_and_payload( GPR_ASSERT(GRPC_CALL_OK == grpc_call_add_metadata(c, &meta2, 0)); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_write(c, request_payload, tag(4), 0)); diff --git a/test/core/end2end/tests/request_response_with_metadata_and_payload.c b/test/core/end2end/tests/request_response_with_metadata_and_payload.c index 77e490710c7..0eebebeb002 100644 --- a/test/core/end2end/tests/request_response_with_metadata_and_payload.c +++ b/test/core/end2end/tests/request_response_with_metadata_and_payload.c @@ -136,9 +136,7 @@ static void test_request_response_with_metadata_and_payload( GPR_ASSERT(GRPC_CALL_OK == grpc_call_add_metadata(c, &meta2, 0)); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_write(c, request_payload, tag(4), 0)); diff --git a/test/core/end2end/tests/request_response_with_payload.c b/test/core/end2end/tests/request_response_with_payload.c index deb61a7a8b9..b2c39e8d6d3 100644 --- a/test/core/end2end/tests/request_response_with_payload.c +++ b/test/core/end2end/tests/request_response_with_payload.c @@ -125,9 +125,7 @@ static void request_response_with_payload(grpc_end2end_test_fixture f) { GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_write(c, request_payload, tag(4), 0)); diff --git a/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c b/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c index 95e268441ba..8fbe514bf37 100644 --- a/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c +++ b/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c @@ -138,9 +138,7 @@ static void test_request_response_with_metadata_and_payload( GPR_ASSERT(GRPC_CALL_OK == grpc_call_add_metadata(c, &meta2, 0)); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_write(c, request_payload, tag(4), 0)); diff --git a/test/core/end2end/tests/request_with_large_metadata.c b/test/core/end2end/tests/request_with_large_metadata.c index 26b165625e1..3c634a396de 100644 --- a/test/core/end2end/tests/request_with_large_metadata.c +++ b/test/core/end2end/tests/request_with_large_metadata.c @@ -128,9 +128,7 @@ static void test_request_with_large_metadata(grpc_end2end_test_config config) { GPR_ASSERT(GRPC_CALL_OK == grpc_call_add_metadata(c, &meta, 0)); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); cq_expect_server_rpc_new(v_server, &s, tag(100), "/foo", "test.google.com", deadline, "key", meta.value, NULL); diff --git a/test/core/end2end/tests/request_with_payload.c b/test/core/end2end/tests/request_with_payload.c index 602ade6f901..db10fac6f5a 100644 --- a/test/core/end2end/tests/request_with_payload.c +++ b/test/core/end2end/tests/request_with_payload.c @@ -122,9 +122,7 @@ static void test_invoke_request_with_payload(grpc_end2end_test_config config) { GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, tag(100))); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_write(c, payload, tag(4), 0)); /* destroy byte buffer early to ensure async code keeps track of its contents diff --git a/test/core/end2end/tests/simple_delayed_request.c b/test/core/end2end/tests/simple_delayed_request.c index b90316c32eb..c1b528cc3f2 100644 --- a/test/core/end2end/tests/simple_delayed_request.c +++ b/test/core/end2end/tests/simple_delayed_request.c @@ -106,10 +106,8 @@ static void simple_delayed_request_body(grpc_end2end_test_config config, c = grpc_channel_create_call(f->client, "/foo", "test.google.com", deadline); GPR_ASSERT(c); - GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_invoke(c, f->client_cq, tag(1), + GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(c, f->client_cq, tag(2), tag(3), 0)); - gpr_sleep_until(gpr_time_add(gpr_now(), gpr_time_from_micros(delay_us))); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); config.init_server(f, server_args); diff --git a/test/core/end2end/tests/simple_request.c b/test/core/end2end/tests/simple_request.c index 3511f276e66..58d80230903 100644 --- a/test/core/end2end/tests/simple_request.c +++ b/test/core/end2end/tests/simple_request.c @@ -113,9 +113,7 @@ static void simple_request_body(grpc_end2end_test_fixture f) { GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c, tag(4))); cq_expect_finish_accepted(v_client, tag(4), GRPC_OP_OK); @@ -160,9 +158,7 @@ static void simple_request_body2(grpc_end2end_test_fixture f) { GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c, tag(4))); cq_expect_finish_accepted(v_client, tag(4), GRPC_OP_OK); diff --git a/test/core/end2end/tests/thread_stress.c b/test/core/end2end/tests/thread_stress.c index 4100b0e35d6..65a7930c1c7 100644 --- a/test/core/end2end/tests/thread_stress.c +++ b/test/core/end2end/tests/thread_stress.c @@ -106,25 +106,31 @@ static void drain_cq(int client, grpc_completion_queue *cq) { /* Kick off a new request - assumes g_mu taken */ static void start_request() { + gpr_slice slice = gpr_slice_malloc(100); + grpc_byte_buffer *buf; grpc_call *call = grpc_channel_create_call( g_fixture.client, "/Foo", "test.google.com", g_test_end_time); + + memset(GPR_SLICE_START_PTR(slice), 1, GPR_SLICE_LENGTH(slice)); + buf = grpc_byte_buffer_create(&slice, 1); + gpr_slice_unref(slice); + g_active_requests++; GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_invoke(call, g_fixture.client_cq, NULL, NULL, NULL, 0)); + GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_read(call, NULL)); + GPR_ASSERT(GRPC_CALL_OK == + grpc_call_start_write(call, buf, NULL, 0)); + + grpc_byte_buffer_destroy(buf); } /* Async client: handle sending requests, reading responses, and starting new requests when old ones finish */ static void client_thread(void *p) { - int id = (gpr_intptr)p; + gpr_intptr id = (gpr_intptr)p; grpc_event *ev; - gpr_slice slice = gpr_slice_malloc(100); - grpc_byte_buffer *buf; char *estr; - memset(GPR_SLICE_START_PTR(slice), id, GPR_SLICE_LENGTH(slice)); - - buf = grpc_byte_buffer_create(&slice, 1); - gpr_slice_unref(slice); for (;;) { ev = grpc_completion_queue_next(g_fixture.client_cq, n_seconds_time(1)); @@ -135,14 +141,6 @@ static void client_thread(void *p) { gpr_log(GPR_ERROR, "unexpected event: %s", estr); gpr_free(estr); break; - case GRPC_INVOKE_ACCEPTED: - /* better not keep going if the invoke failed */ - if (ev->data.invoke_accepted == GRPC_OP_OK) { - GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_read(ev->call, NULL)); - GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_write(ev->call, buf, NULL, 0)); - } - break; case GRPC_READ: break; case GRPC_WRITE_ACCEPTED: @@ -173,7 +171,6 @@ static void client_thread(void *p) { gpr_mu_unlock(&g_mu); } - grpc_byte_buffer_destroy(buf); gpr_event_set(&g_client_done[id], (void *)1); } @@ -196,17 +193,17 @@ static void maybe_end_server_call(grpc_call *call, gpr_refcount *rc) { static void server_thread(void *p) { int id = (gpr_intptr)p; - grpc_event *ev; gpr_slice slice = gpr_slice_malloc(100); grpc_byte_buffer *buf; + grpc_event *ev; char *estr; - memset(GPR_SLICE_START_PTR(slice), id, GPR_SLICE_LENGTH(slice)); - - request_server_call(); + memset(GPR_SLICE_START_PTR(slice), 1, GPR_SLICE_LENGTH(slice)); buf = grpc_byte_buffer_create(&slice, 1); gpr_slice_unref(slice); + request_server_call(); + for (;;) { ev = grpc_completion_queue_next(g_fixture.server_cq, n_seconds_time(1)); if (ev) { diff --git a/test/core/end2end/tests/writes_done_hangs_with_pending_read.c b/test/core/end2end/tests/writes_done_hangs_with_pending_read.c index 2241519e894..d959111e43d 100644 --- a/test/core/end2end/tests/writes_done_hangs_with_pending_read.c +++ b/test/core/end2end/tests/writes_done_hangs_with_pending_read.c @@ -128,9 +128,7 @@ static void test_writes_done_hangs_with_pending_read( GPR_ASSERT(c); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_write(c, request_payload, tag(4), 0)); diff --git a/test/core/surface/lame_client_test.c b/test/core/surface/lame_client_test.c index 0520a39ea2f..11d5e4a4951 100644 --- a/test/core/surface/lame_client_test.c +++ b/test/core/surface/lame_client_test.c @@ -63,10 +63,9 @@ int main(int argc, char **argv) { /* and invoke the call */ GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(call, cq, tag(1), tag(2), tag(3), 0)); + grpc_call_invoke(call, cq, tag(2), tag(3), 0)); /* the call should immediately fail */ - cq_expect_invoke_accepted(cqv, tag(1), GRPC_OP_ERROR); cq_expect_client_metadata_read(cqv, tag(2), NULL); cq_expect_finished(cqv, tag(3), NULL); cq_verify(cqv); From 40fc7a66323d74b4303d582345807c4577584a77 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 16:11:58 -0800 Subject: [PATCH 02/26] clang-format codebase --- include/grpc/grpc.h | 7 ++- src/core/surface/byte_buffer.c | 3 +- src/core/surface/call.c | 53 ++++++++++--------- test/core/end2end/dualstack_socket_test.c | 3 +- test/core/end2end/no_server_test.c | 3 +- test/core/end2end/tests/disappearing_server.c | 4 +- .../end2end/tests/max_concurrent_streams.c | 8 +-- .../end2end/tests/simple_delayed_request.c | 4 +- test/core/end2end/tests/thread_stress.c | 5 +- test/core/surface/lame_client_test.c | 3 +- 10 files changed, 45 insertions(+), 48 deletions(-) diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index cc7ed4a9fbc..e51a1668e8b 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -320,10 +320,9 @@ grpc_call_error grpc_call_add_metadata(grpc_call *call, grpc_metadata *metadata, Produces a GRPC_FINISHED event with finished_tag when the call has been completed (there may be other events for the call pending at this time) */ -grpc_call_error grpc_call_invoke(grpc_call *call, - grpc_completion_queue *cq, - void *metadata_read_tag, - void *finished_tag, gpr_uint32 flags); +grpc_call_error grpc_call_invoke(grpc_call *call, grpc_completion_queue *cq, + void *metadata_read_tag, void *finished_tag, + gpr_uint32 flags); grpc_call_error grpc_call_start_invoke(grpc_call *call, grpc_completion_queue *cq, void *invoke_accepted_tag, diff --git a/src/core/surface/byte_buffer.c b/src/core/surface/byte_buffer.c index 40876228945..d1be41074d3 100644 --- a/src/core/surface/byte_buffer.c +++ b/src/core/surface/byte_buffer.c @@ -52,7 +52,8 @@ grpc_byte_buffer *grpc_byte_buffer_create(gpr_slice *slices, size_t nslices) { grpc_byte_buffer *grpc_byte_buffer_copy(grpc_byte_buffer *bb) { switch (bb->type) { case GRPC_BB_SLICE_BUFFER: - return grpc_byte_buffer_create(bb->data.slice_buffer.slices, bb->data.slice_buffer.count); + return grpc_byte_buffer_create(bb->data.slice_buffer.slices, + bb->data.slice_buffer.count); } gpr_log(GPR_INFO, "should never get here"); abort(); diff --git a/src/core/surface/call.c b/src/core/surface/call.c index 297d9587eb5..262fbe381ac 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -357,10 +357,12 @@ grpc_call_error grpc_call_start_invoke(grpc_call *call, void *invoke_accepted_tag, void *metadata_read_tag, void *finished_tag, gpr_uint32 flags) { - grpc_call_error err = grpc_call_invoke(call, cq, metadata_read_tag, finished_tag, flags); + grpc_call_error err = + grpc_call_invoke(call, cq, metadata_read_tag, finished_tag, flags); if (err == GRPC_CALL_OK) { grpc_cq_begin_op(call->cq, call, GRPC_INVOKE_ACCEPTED); - grpc_cq_end_invoke_accepted(call->cq, invoke_accepted_tag, call, do_nothing, NULL, GRPC_OP_OK); + grpc_cq_end_invoke_accepted(call->cq, invoke_accepted_tag, call, do_nothing, + NULL, GRPC_OP_OK); } return err; } @@ -421,11 +423,11 @@ static void call_started(void *user_data, grpc_op_error error) { } if (pending_writes_done) { if (ok) { - op.type = GRPC_SEND_FINISH; - op.dir = GRPC_CALL_DOWN; - op.flags = 0; - op.done_cb = done_writes_done; - op.user_data = call; + op.type = GRPC_SEND_FINISH; + op.dir = GRPC_CALL_DOWN; + op.flags = 0; + op.done_cb = done_writes_done; + op.user_data = call; elem = CALL_ELEM_FROM_CALL(call, 0); elem->filter->call_op(elem, NULL, &op); @@ -435,10 +437,9 @@ static void call_started(void *user_data, grpc_op_error error) { } } -grpc_call_error grpc_call_invoke(grpc_call *call, - grpc_completion_queue *cq, - void *metadata_read_tag, - void *finished_tag, gpr_uint32 flags) { +grpc_call_error grpc_call_invoke(grpc_call *call, grpc_completion_queue *cq, + void *metadata_read_tag, void *finished_tag, + gpr_uint32 flags) { grpc_call_element *elem; grpc_call_op op; @@ -708,15 +709,15 @@ grpc_call_error grpc_call_start_write(grpc_call *call, } else { gpr_mu_unlock(&call->read_mu); - op.type = GRPC_SEND_MESSAGE; - op.dir = GRPC_CALL_DOWN; - op.flags = flags; - op.done_cb = done_write; - op.user_data = call; - op.data.message = byte_buffer; + op.type = GRPC_SEND_MESSAGE; + op.dir = GRPC_CALL_DOWN; + op.flags = flags; + op.done_cb = done_write; + op.user_data = call; + op.data.message = byte_buffer; - elem = CALL_ELEM_FROM_CALL(call, 0); - elem->filter->call_op(elem, NULL, &op); + elem = CALL_ELEM_FROM_CALL(call, 0); + elem->filter->call_op(elem, NULL, &op); } return GRPC_CALL_OK; @@ -757,14 +758,14 @@ grpc_call_error grpc_call_writes_done(grpc_call *call, void *tag) { } else { gpr_mu_unlock(&call->read_mu); - op.type = GRPC_SEND_FINISH; - op.dir = GRPC_CALL_DOWN; - op.flags = 0; - op.done_cb = done_writes_done; - op.user_data = call; + op.type = GRPC_SEND_FINISH; + op.dir = GRPC_CALL_DOWN; + op.flags = 0; + op.done_cb = done_writes_done; + op.user_data = call; - elem = CALL_ELEM_FROM_CALL(call, 0); - elem->filter->call_op(elem, NULL, &op); + elem = CALL_ELEM_FROM_CALL(call, 0); + elem->filter->call_op(elem, NULL, &op); } return GRPC_CALL_OK; diff --git a/test/core/end2end/dualstack_socket_test.c b/test/core/end2end/dualstack_socket_test.c index ac656d3719b..b765d3d43ea 100644 --- a/test/core/end2end/dualstack_socket_test.c +++ b/test/core/end2end/dualstack_socket_test.c @@ -115,8 +115,7 @@ void test_connect(const char *server_host, const char *client_host, int port, c = grpc_channel_create_call(client, "/foo", "test.google.com", deadline); GPR_ASSERT(c); - GPR_ASSERT(GRPC_CALL_OK == - grpc_call_invoke(c, client_cq, tag(2), tag(3), 0)); + GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(c, client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c, tag(4))); if (expect_ok) { /* Check for a successful request. */ diff --git a/test/core/end2end/no_server_test.c b/test/core/end2end/no_server_test.c index 0c251ab467a..2c432456ce4 100644 --- a/test/core/end2end/no_server_test.c +++ b/test/core/end2end/no_server_test.c @@ -57,8 +57,7 @@ int main(int argc, char **argv) { /* create a call, channel to a non existant server */ chan = grpc_channel_create("nonexistant:54321", NULL); call = grpc_channel_create_call(chan, "/foo", "nonexistant", deadline); - GPR_ASSERT(grpc_call_invoke(call, cq, tag(2), tag(3), 0) == - GRPC_CALL_OK); + GPR_ASSERT(grpc_call_invoke(call, cq, tag(2), tag(3), 0) == GRPC_CALL_OK); /* verify that all tags get completed */ cq_expect_client_metadata_read(cqv, tag(2), NULL); cq_expect_finished_with_status(cqv, tag(3), GRPC_STATUS_CANCELLED, NULL, diff --git a/test/core/end2end/tests/disappearing_server.c b/test/core/end2end/tests/disappearing_server.c index 1cbb15f32e4..d30e9eae5ff 100644 --- a/test/core/end2end/tests/disappearing_server.c +++ b/test/core/end2end/tests/disappearing_server.c @@ -100,8 +100,8 @@ static void do_request_and_shutdown_server(grpc_end2end_test_fixture *f, c = grpc_channel_create_call(f->client, "/foo", "test.google.com", deadline); GPR_ASSERT(c); - GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(c, f->client_cq, - tag(2), tag(3), 0)); + GPR_ASSERT(GRPC_CALL_OK == + grpc_call_invoke(c, f->client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c, tag(4))); cq_expect_finish_accepted(v_client, tag(4), GRPC_OP_OK); diff --git a/test/core/end2end/tests/max_concurrent_streams.c b/test/core/end2end/tests/max_concurrent_streams.c index e2f30d07789..6e5cb1316b0 100644 --- a/test/core/end2end/tests/max_concurrent_streams.c +++ b/test/core/end2end/tests/max_concurrent_streams.c @@ -189,10 +189,10 @@ static void test_max_concurrent_streams(grpc_end2end_test_config config) { GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, tag(100))); - GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(c1, f.client_cq, - tag(301), tag(302), 0)); - GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(c2, f.client_cq, - tag(401), tag(402), 0)); + GPR_ASSERT(GRPC_CALL_OK == + grpc_call_invoke(c1, f.client_cq, tag(301), tag(302), 0)); + GPR_ASSERT(GRPC_CALL_OK == + grpc_call_invoke(c2, f.client_cq, tag(401), tag(402), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c1, tag(303))); GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c2, tag(303))); diff --git a/test/core/end2end/tests/simple_delayed_request.c b/test/core/end2end/tests/simple_delayed_request.c index c1b528cc3f2..6e2e16714ab 100644 --- a/test/core/end2end/tests/simple_delayed_request.c +++ b/test/core/end2end/tests/simple_delayed_request.c @@ -106,8 +106,8 @@ static void simple_delayed_request_body(grpc_end2end_test_config config, c = grpc_channel_create_call(f->client, "/foo", "test.google.com", deadline); GPR_ASSERT(c); - GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(c, f->client_cq, - tag(2), tag(3), 0)); + GPR_ASSERT(GRPC_CALL_OK == + grpc_call_invoke(c, f->client_cq, tag(2), tag(3), 0)); config.init_server(f, server_args); diff --git a/test/core/end2end/tests/thread_stress.c b/test/core/end2end/tests/thread_stress.c index 65a7930c1c7..4ce85df22e0 100644 --- a/test/core/end2end/tests/thread_stress.c +++ b/test/core/end2end/tests/thread_stress.c @@ -118,9 +118,8 @@ static void start_request() { g_active_requests++; GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_invoke(call, g_fixture.client_cq, NULL, NULL, NULL, 0)); - GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_read(call, NULL)); - GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_write(call, buf, NULL, 0)); + GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_read(call, NULL)); + GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_write(call, buf, NULL, 0)); grpc_byte_buffer_destroy(buf); } diff --git a/test/core/surface/lame_client_test.c b/test/core/surface/lame_client_test.c index 11d5e4a4951..9b9f0202d6d 100644 --- a/test/core/surface/lame_client_test.c +++ b/test/core/surface/lame_client_test.c @@ -62,8 +62,7 @@ int main(int argc, char **argv) { GPR_ASSERT(GRPC_CALL_OK == grpc_call_add_metadata(call, &md, 0)); /* and invoke the call */ - GPR_ASSERT(GRPC_CALL_OK == - grpc_call_invoke(call, cq, tag(2), tag(3), 0)); + GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(call, cq, tag(2), tag(3), 0)); /* the call should immediately fail */ cq_expect_client_metadata_read(cqv, tag(2), NULL); From cbbac9b10db599c9c78d5c94846333125215ca85 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 16:13:43 -0800 Subject: [PATCH 03/26] Compile fix --- test/core/end2end/tests/max_concurrent_streams.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/core/end2end/tests/max_concurrent_streams.c b/test/core/end2end/tests/max_concurrent_streams.c index 6e5cb1316b0..57e271834c0 100644 --- a/test/core/end2end/tests/max_concurrent_streams.c +++ b/test/core/end2end/tests/max_concurrent_streams.c @@ -155,7 +155,6 @@ static void test_max_concurrent_streams(grpc_end2end_test_config config) { grpc_call *s1; grpc_call *s2; int live_call; - grpc_call *live_call_obj; gpr_timespec deadline; cq_verifier *v_client; cq_verifier *v_server; @@ -204,7 +203,6 @@ static void test_max_concurrent_streams(grpc_end2end_test_config config) { /* The /alpha or /beta calls started above could be invoked (but NOT both); * check this here */ live_call = (int)(gpr_intptr)ev->tag; - live_call_obj = live_call == 300 ? c1 : c2; grpc_event_finish(ev); cq_expect_server_rpc_new(v_server, &s1, tag(100), @@ -227,7 +225,6 @@ static void test_max_concurrent_streams(grpc_end2end_test_config config) { cq_expect_finished_with_status(v_client, tag(live_call + 2), GRPC_STATUS_UNIMPLEMENTED, "xyz", NULL); live_call = (live_call == 300) ? 400 : 300; - live_call_obj = live_call == 300 ? c1 : c2; cq_expect_finish_accepted(v_client, tag(live_call + 3), GRPC_OP_OK); cq_verify(v_client); From 2163bad186eeeedac73e3a7b1a9b735e1dc6dd18 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 16:35:41 -0800 Subject: [PATCH 04/26] Calls are always started on the server. By the time we call accept, we have proof that we've received the start of a call, so we should set this bit to one. --- 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 262fbe381ac..6313fbce028 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -531,6 +531,7 @@ grpc_call_error grpc_call_server_accept(grpc_call *call, call->state = CALL_BOUNDCQ; call->cq = cq; call->finished_tag = finished_tag; + call->received_start = 1; if (prq_is_empty(&call->prq) && call->received_finish) { finish_call(call); From 97f7fca5ffe17565d020118a1c8b7db032cc70fb Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 16:36:31 -0800 Subject: [PATCH 05/26] Remove calls to grpc_call_start_invoke --- test/core/end2end/tests/census_simple_request.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/core/end2end/tests/census_simple_request.c b/test/core/end2end/tests/census_simple_request.c index 64c0d12ca4a..95bbefe381c 100644 --- a/test/core/end2end/tests/census_simple_request.c +++ b/test/core/end2end/tests/census_simple_request.c @@ -109,9 +109,7 @@ static void test_body(grpc_end2end_test_fixture f) { GPR_ASSERT(c); tag(1); GPR_ASSERT(GRPC_CALL_OK == - grpc_call_start_invoke(c, f.client_cq, tag(1), tag(2), tag(3), 0)); - cq_expect_invoke_accepted(v_client, tag(1), GRPC_OP_OK); - cq_verify(v_client); + grpc_call_invoke(c, f.client_cq, tag(2), tag(3), 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_writes_done(c, tag(4))); cq_expect_finish_accepted(v_client, tag(4), GRPC_OP_OK); From 9c0307bd4fe5528f05b301d19ad2c72472b26b3f Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 16:49:24 -0800 Subject: [PATCH 06/26] Update test to grpc_call_invoke --- test/core/end2end/tests/thread_stress.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/core/end2end/tests/thread_stress.c b/test/core/end2end/tests/thread_stress.c index 4ce85df22e0..36da4950bbc 100644 --- a/test/core/end2end/tests/thread_stress.c +++ b/test/core/end2end/tests/thread_stress.c @@ -116,8 +116,8 @@ static void start_request() { gpr_slice_unref(slice); g_active_requests++; - GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_invoke(call, g_fixture.client_cq, - NULL, NULL, NULL, 0)); + GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(call, g_fixture.client_cq, + NULL, NULL, 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_read(call, NULL)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_write(call, buf, NULL, 0)); From fc84e7b28e4a506393381f874708fee5f5411c91 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 16:49:44 -0800 Subject: [PATCH 07/26] Fix tag matching bug --- test/core/end2end/tests/max_concurrent_streams.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/core/end2end/tests/max_concurrent_streams.c b/test/core/end2end/tests/max_concurrent_streams.c index 57e271834c0..5a0d458d209 100644 --- a/test/core/end2end/tests/max_concurrent_streams.c +++ b/test/core/end2end/tests/max_concurrent_streams.c @@ -202,7 +202,8 @@ static void test_max_concurrent_streams(grpc_end2end_test_config config) { GPR_ASSERT(ev->data.invoke_accepted == GRPC_OP_OK); /* The /alpha or /beta calls started above could be invoked (but NOT both); * check this here */ - live_call = (int)(gpr_intptr)ev->tag; + /* We'll get tag 303 or 403, we want 300, 400 */ + live_call = ((int)(gpr_intptr)ev->tag) - 3; grpc_event_finish(ev); cq_expect_server_rpc_new(v_server, &s1, tag(100), From 44974e6bd8f7498d19e258bc99a15c04bc79735d Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 16:50:09 -0800 Subject: [PATCH 08/26] Only request more data once stream is connected. Otherwise we can cause segfaults down in the client_channel. --- src/core/surface/call.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/surface/call.c b/src/core/surface/call.c index 6313fbce028..bb4a0e77c7f 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -654,7 +654,7 @@ grpc_call_error grpc_call_start_read(grpc_call *call, void *tag) { } else { call->read_tag = tag; call->have_read = 1; - request_more = 1; + request_more = call->received_start; } } else if (prq_is_empty(&call->prq) && call->received_finish) { finish_call(call); From f93d53e458743d561a93cb11541ca3b28580a3d8 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 17:01:34 -0800 Subject: [PATCH 09/26] Starting to fix C++ --- src/cpp/client/channel.cc | 12 ++---------- src/cpp/stream/stream_context.cc | 9 +-------- src/cpp/stream/stream_context.h | 1 - 3 files changed, 3 insertions(+), 19 deletions(-) diff --git a/src/cpp/client/channel.cc b/src/cpp/client/channel.cc index ddda8c22d6d..223151932ca 100644 --- a/src/cpp/client/channel.cc +++ b/src/cpp/client/channel.cc @@ -104,7 +104,6 @@ Status Channel::StartBlockingRpc(const RpcMethod& method, context->set_call(call); grpc_event* ev; void* finished_tag = reinterpret_cast(call); - void* invoke_tag = reinterpret_cast(call) + 1; void* metadata_read_tag = reinterpret_cast(call) + 2; void* write_tag = reinterpret_cast(call) + 3; void* halfclose_tag = reinterpret_cast(call) + 4; @@ -115,19 +114,12 @@ Status Channel::StartBlockingRpc(const RpcMethod& method, // add_metadata from context // // invoke - GPR_ASSERT(grpc_call_start_invoke(call, cq, invoke_tag, metadata_read_tag, + GPR_ASSERT(grpc_call_invoke(call, cq, metadata_read_tag, finished_tag, GRPC_WRITE_BUFFER_HINT) == GRPC_CALL_OK); - ev = grpc_completion_queue_pluck(cq, invoke_tag, gpr_inf_future); - bool success = ev->data.invoke_accepted == GRPC_OP_OK; - grpc_event_finish(ev); - if (!success) { - GetFinalStatus(cq, finished_tag, &status); - return status; - } // write request grpc_byte_buffer* write_buffer = nullptr; - success = SerializeProto(request, &write_buffer); + bool success = SerializeProto(request, &write_buffer); if (!success) { grpc_call_cancel(call); status = diff --git a/src/cpp/stream/stream_context.cc b/src/cpp/stream/stream_context.cc index 7936a30dfd7..720f3e27fe3 100644 --- a/src/cpp/stream/stream_context.cc +++ b/src/cpp/stream/stream_context.cc @@ -80,17 +80,10 @@ void StreamContext::Start(bool buffered) { if (is_client_) { // TODO(yangg) handle metadata send path int flag = buffered ? GRPC_WRITE_BUFFER_HINT : 0; - grpc_call_error error = grpc_call_start_invoke(call(), cq(), invoke_tag(), + grpc_call_error error = grpc_call_invoke(call(), cq(), client_metadata_read_tag(), finished_tag(), flag); GPR_ASSERT(GRPC_CALL_OK == error); - grpc_event* invoke_ev = - grpc_completion_queue_pluck(cq(), invoke_tag(), gpr_inf_future); - if (invoke_ev->data.invoke_accepted != GRPC_OP_OK) { - peer_halfclosed_ = true; - self_halfclosed_ = true; - } - grpc_event_finish(invoke_ev); } else { // TODO(yangg) metadata needs to be added before accept // TODO(yangg) correctly set flag to accept diff --git a/src/cpp/stream/stream_context.h b/src/cpp/stream/stream_context.h index f70fe6daa34..c1a8d8ae522 100644 --- a/src/cpp/stream/stream_context.h +++ b/src/cpp/stream/stream_context.h @@ -76,7 +76,6 @@ class StreamContext : public StreamContextInterface { void* read_tag() { return reinterpret_cast(this) + 1; } void* write_tag() { return reinterpret_cast(this) + 2; } void* halfclose_tag() { return reinterpret_cast(this) + 3; } - void* invoke_tag() { return reinterpret_cast(this) + 4; } void* client_metadata_read_tag() { return reinterpret_cast(this) + 5; } grpc_call* call() { return call_; } grpc_completion_queue* cq() { return cq_; } From bf444937c613370fb88acbc4bceefba1772cc3c1 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 17:13:08 -0800 Subject: [PATCH 10/26] Fix ordering problem leading to flaky test --- test/core/end2end/tests/max_concurrent_streams.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/end2end/tests/max_concurrent_streams.c b/test/core/end2end/tests/max_concurrent_streams.c index 5a0d458d209..e58b31a7cc3 100644 --- a/test/core/end2end/tests/max_concurrent_streams.c +++ b/test/core/end2end/tests/max_concurrent_streams.c @@ -225,8 +225,8 @@ static void test_max_concurrent_streams(grpc_end2end_test_config config) { /* first request is finished, we should be able to start the second */ cq_expect_finished_with_status(v_client, tag(live_call + 2), GRPC_STATUS_UNIMPLEMENTED, "xyz", NULL); - live_call = (live_call == 300) ? 400 : 300; cq_expect_finish_accepted(v_client, tag(live_call + 3), GRPC_OP_OK); + live_call = (live_call == 300) ? 400 : 300; cq_verify(v_client); GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, tag(200))); From 5c35008ea1108f84e51071cc12b2a2ae0b7f3d98 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 17:26:27 -0800 Subject: [PATCH 11/26] Unify signal handling in C --- test/core/iomgr/tcp_posix_test.c | 3 --- test/core/transport/chttp2_transport_end2end_test.c | 3 --- test/core/util/test_config.c | 3 +++ 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/test/core/iomgr/tcp_posix_test.c b/test/core/iomgr/tcp_posix_test.c index 7fd2567cec5..6af3ded98b0 100644 --- a/test/core/iomgr/tcp_posix_test.c +++ b/test/core/iomgr/tcp_posix_test.c @@ -36,7 +36,6 @@ #include #include #include -#include #include #include #include @@ -491,8 +490,6 @@ static grpc_endpoint_test_config configs[] = { int main(int argc, char **argv) { grpc_test_init(argc, argv); grpc_iomgr_init(); - /* disable SIGPIPE */ - signal(SIGPIPE, SIG_IGN); run_tests(); grpc_endpoint_tests(configs[0]); grpc_iomgr_shutdown(); diff --git a/test/core/transport/chttp2_transport_end2end_test.c b/test/core/transport/chttp2_transport_end2end_test.c index 30d2a174401..8b0f9aa25b5 100644 --- a/test/core/transport/chttp2_transport_end2end_test.c +++ b/test/core/transport/chttp2_transport_end2end_test.c @@ -107,9 +107,6 @@ grpc_transport_test_config fixture_configs[] = { int main(int argc, char **argv) { size_t i; - /* disable SIGPIPE */ - signal(SIGPIPE, SIG_IGN); - grpc_test_init(argc, argv); grpc_iomgr_init(); diff --git a/test/core/util/test_config.c b/test/core/util/test_config.c index fc5de9bbefb..1a8f1a5f496 100644 --- a/test/core/util/test_config.c +++ b/test/core/util/test_config.c @@ -34,9 +34,12 @@ #include "test/core/util/test_config.h" #include +#include #include void grpc_test_init(int argc, char **argv) { + /* disable SIGPIPE */ + signal(SIGPIPE, SIG_IGN); /* seed rng with pid, so we don't end up with the same random numbers as a concurrently running test binary */ srand(getpid()); From 0a00a08c53c3017f1d6c3224c94b84b9c56ab197 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 17:26:46 -0800 Subject: [PATCH 12/26] Use C signal handling in C++ end2end --- test/cpp/end2end/end2end_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index e01a6efe82a..3a1da68e47f 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -34,6 +34,7 @@ #include #include +#include "test/core/util/test_config.h" #include "test/cpp/util/echo_duplicate.pb.h" #include "test/cpp/util/echo.pb.h" #include "src/cpp/util/time.h" @@ -435,6 +436,7 @@ TEST_F(End2endTest, BadCredentials) { } // namespace grpc int main(int argc, char** argv) { + grpc_test_init(argc, argv); grpc_init(); ::testing::InitGoogleTest(&argc, argv); int result = RUN_ALL_TESTS(); From cc19464dd5d51f0f38771e997838f9ad9a9c06ff Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 13 Jan 2015 17:45:03 -0800 Subject: [PATCH 13/26] Use the first received status as authoritative. So that later cancellations do not clobber status. --- src/core/surface/call.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/core/surface/call.c b/src/core/surface/call.c index bb4a0e77c7f..ae6fa6336b2 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -181,6 +181,7 @@ struct grpc_call { gpr_uint8 have_read; gpr_uint8 have_alarm; gpr_uint8 pending_writes_done; + gpr_uint8 got_status_code; /* The current outstanding read message tag (only valid if have_read == 1) */ void *read_tag; void *metadata_tag; @@ -230,6 +231,7 @@ grpc_call *grpc_call_create(grpc_channel *channel, call->have_write = 0; call->have_alarm = 0; call->received_metadata = 0; + call->got_status_code = 0; call->status_code = server_transport_data != NULL ? GRPC_STATUS_OK : GRPC_STATUS_UNKNOWN; call->status_details = NULL; @@ -872,15 +874,18 @@ void grpc_call_recv_metadata(grpc_call_element *elem, grpc_call_op *op) { grpc_call *call = CALL_FROM_TOP_ELEM(elem); grpc_mdelem *md = op->data.metadata; grpc_mdstr *key = md->key; + gpr_log(GPR_DEBUG, "call %p got metadata %s %s", call, grpc_mdstr_as_c_string(md->key), grpc_mdstr_as_c_string(md->value)); if (key == grpc_channel_get_status_string(call->channel)) { - call->status_code = decode_status(md); + if (!call->got_status_code) { + call->status_code = decode_status(md); + call->got_status_code = 1; + } grpc_mdelem_unref(md); op->done_cb(op->user_data, GRPC_OP_OK); } else if (key == grpc_channel_get_message_string(call->channel)) { - if (call->status_details) { - grpc_mdstr_unref(call->status_details); + if (!call->status_details) { + call->status_details = grpc_mdstr_ref(md->value); } - call->status_details = grpc_mdstr_ref(md->value); grpc_mdelem_unref(md); op->done_cb(op->user_data, GRPC_OP_OK); } else { From e4fcd6aa0e3f1d1c759f55cb72966693d8fa774b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 11:33:03 -0800 Subject: [PATCH 14/26] Build all targets for a config at once --- tools/run_tests/run_tests.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index bb25b38e570..84193a1337c 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -75,10 +75,8 @@ def _build_and_run(check_cancelled): if not jobset.run( (['make', '-j', '%d' % (multiprocessing.cpu_count() + 1), - target, - 'CONFIG=%s' % cfg] - for cfg in build_configs - for target in _MAKE_TEST_TARGETS), + 'CONFIG=%s' % cfg] + _MAKE_TEST_TARGETS + for cfg in build_configs), check_cancelled, maxjobs=1): sys.exit(1) From 2ba0dacbde45ed3a491d78134244b3175c556494 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 11:49:12 -0800 Subject: [PATCH 15/26] Update C++ code to set status via the C api. This prevents mismatches from breaking tests. --- include/grpc++/stream.h | 6 ++-- include/grpc++/stream_context_interface.h | 2 +- include/grpc/grpc.h | 8 +++++ src/core/surface/call.c | 33 +++++++++++++++----- src/cpp/stream/stream_context.cc | 38 ++++++++--------------- src/cpp/stream/stream_context.h | 4 +-- third_party/libevent | 1 + 7 files changed, 54 insertions(+), 38 deletions(-) create mode 160000 third_party/libevent diff --git a/include/grpc++/stream.h b/include/grpc++/stream.h index 49f88a6f135..b8982f4d93d 100644 --- a/include/grpc++/stream.h +++ b/include/grpc++/stream.h @@ -96,7 +96,7 @@ class ClientReader : public ClientStreamingInterface, virtual bool Read(R* msg) { return context_->Read(msg); } - virtual void Cancel() { context_->FinishStream(Status::Cancelled, true); } + virtual void Cancel() { context_->Cancel(); } virtual const Status& Wait() { return context_->Wait(); } @@ -122,7 +122,7 @@ class ClientWriter : public ClientStreamingInterface, virtual void WritesDone() { context_->Write(nullptr, true); } - virtual void Cancel() { context_->FinishStream(Status::Cancelled, true); } + virtual void Cancel() { context_->Cancel(); } // Read the final response and wait for the final status. virtual const Status& Wait() { @@ -165,7 +165,7 @@ class ClientReaderWriter : public ClientStreamingInterface, virtual void WritesDone() { context_->Write(nullptr, true); } - virtual void Cancel() { context_->FinishStream(Status::Cancelled, true); } + virtual void Cancel() { context_->Cancel(); } virtual const Status& Wait() { return context_->Wait(); } diff --git a/include/grpc++/stream_context_interface.h b/include/grpc++/stream_context_interface.h index 535c0048e65..a84119800b7 100644 --- a/include/grpc++/stream_context_interface.h +++ b/include/grpc++/stream_context_interface.h @@ -53,7 +53,7 @@ class StreamContextInterface { virtual bool Read(google::protobuf::Message* msg) = 0; virtual bool Write(const google::protobuf::Message* msg, bool is_last) = 0; virtual const Status& Wait() = 0; - virtual void FinishStream(const Status& status, bool send) = 0; + virtual void Cancel() = 0; virtual google::protobuf::Message* request() = 0; virtual google::protobuf::Message* response() = 0; diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index e51a1668e8b..53e0d056305 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -369,6 +369,14 @@ grpc_call_error grpc_call_server_end_initial_metadata(grpc_call *call, Can be called multiple times, from any thread. */ grpc_call_error grpc_call_cancel(grpc_call *call); +/* Called by clients to cancel an RPC on the server. + Can be called multiple times, from any thread. + If a status has not been received for the call, set it to the status code + and description passed in. + Importantly, this function does not send status nor description to the + remote endpoint. */ +grpc_call_error grpc_call_cancel_with_status(grpc_call *call, grpc_status_code status, const char *description); + /* Queue a byte buffer for writing. flags is a bit-field combination of the write flags defined above. A write with byte_buffer null is allowed, and will not send any bytes on the diff --git a/src/core/surface/call.c b/src/core/surface/call.c index ae6fa6336b2..87e3de88e2d 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -278,6 +278,19 @@ void grpc_call_destroy(grpc_call *c) { grpc_call_internal_unref(c); } +static void maybe_set_status_code(grpc_call *call, gpr_uint32 status) { + if (!call->got_status_code) { + call->status_code = status; + call->got_status_code = 1; + } +} + +static void maybe_set_status_details(grpc_call *call, grpc_mdstr *status) { + if (!call->status_details) { + call->status_details = grpc_mdstr_ref(status); + } +} + grpc_call_error grpc_call_cancel(grpc_call *c) { grpc_call_element *elem; grpc_call_op op; @@ -294,6 +307,17 @@ grpc_call_error grpc_call_cancel(grpc_call *c) { return GRPC_CALL_OK; } +grpc_call_error grpc_call_cancel_with_status(grpc_call *c, grpc_status_code status, const char *description) { + grpc_mdstr *details = description? grpc_mdstr_from_string(c->metadata_context, description) : NULL; + gpr_mu_lock(&c->read_mu); + maybe_set_status_code(c, status); + if (details) { + maybe_set_status_details(c, details); + } + gpr_mu_unlock(&c->read_mu); + return grpc_call_cancel(c); +} + void grpc_call_execute_op(grpc_call *call, grpc_call_op *op) { grpc_call_element *elem; GPR_ASSERT(op->dir == GRPC_CALL_DOWN); @@ -876,16 +900,11 @@ void grpc_call_recv_metadata(grpc_call_element *elem, grpc_call_op *op) { grpc_mdstr *key = md->key; gpr_log(GPR_DEBUG, "call %p got metadata %s %s", call, grpc_mdstr_as_c_string(md->key), grpc_mdstr_as_c_string(md->value)); if (key == grpc_channel_get_status_string(call->channel)) { - if (!call->got_status_code) { - call->status_code = decode_status(md); - call->got_status_code = 1; - } + maybe_set_status_code(call, decode_status(md)); grpc_mdelem_unref(md); op->done_cb(op->user_data, GRPC_OP_OK); } else if (key == grpc_channel_get_message_string(call->channel)) { - if (!call->status_details) { - call->status_details = grpc_mdstr_ref(md->value); - } + maybe_set_status_details(call, md->value); grpc_mdelem_unref(md); op->done_cb(op->user_data, GRPC_OP_OK); } else { diff --git a/src/cpp/stream/stream_context.cc b/src/cpp/stream/stream_context.cc index 720f3e27fe3..aec226b23c0 100644 --- a/src/cpp/stream/stream_context.cc +++ b/src/cpp/stream/stream_context.cc @@ -105,9 +105,7 @@ bool StreamContext::Read(google::protobuf::Message* msg) { if (read_ev->data.read) { if (!DeserializeProto(read_ev->data.read, msg)) { ret = false; - FinishStream( - Status(StatusCode::DATA_LOSS, "Failed to parse incoming proto"), - true); + grpc_call_cancel_with_status(call(), GRPC_STATUS_DATA_LOSS, "Failed to parse incoming proto"); } } else { ret = false; @@ -125,9 +123,7 @@ bool StreamContext::Write(const google::protobuf::Message* msg, bool is_last) { if (msg) { grpc_byte_buffer* out_buf = nullptr; if (!SerializeProto(*msg, &out_buf)) { - FinishStream(Status(StatusCode::INVALID_ARGUMENT, - "Failed to serialize outgoing proto"), - true); + grpc_call_cancel_with_status(call(), GRPC_STATUS_INVALID_ARGUMENT, "Failed to serialize outgoing proto"); return false; } int flag = is_last ? GRPC_WRITE_BUFFER_HINT : 0; @@ -165,29 +161,21 @@ const Status& StreamContext::Wait() { grpc_event_finish(metadata_ev); // TODO(yangg) protect states by a mutex, including other places. if (!self_halfclosed_ || !peer_halfclosed_) { - FinishStream(Status::Cancelled, true); - } else { - grpc_event* finish_ev = - grpc_completion_queue_pluck(cq(), finished_tag(), gpr_inf_future); - GPR_ASSERT(finish_ev->type == GRPC_FINISHED); - final_status_ = Status( - static_cast(finish_ev->data.finished.status), - finish_ev->data.finished.details ? finish_ev->data.finished.details - : ""); - grpc_event_finish(finish_ev); - } - return final_status_; -} - -void StreamContext::FinishStream(const Status& status, bool send) { - if (send) { - grpc_call_cancel(call()); - } + Cancel(); + } grpc_event* finish_ev = grpc_completion_queue_pluck(cq(), finished_tag(), gpr_inf_future); GPR_ASSERT(finish_ev->type == GRPC_FINISHED); + final_status_ = Status( + static_cast(finish_ev->data.finished.status), + finish_ev->data.finished.details ? finish_ev->data.finished.details + : ""); grpc_event_finish(finish_ev); - final_status_ = status; + return final_status_; +} + +void StreamContext::Cancel() { + grpc_call_cancel(call()); } } // namespace grpc diff --git a/src/cpp/stream/stream_context.h b/src/cpp/stream/stream_context.h index c1a8d8ae522..d6bd7a23708 100644 --- a/src/cpp/stream/stream_context.h +++ b/src/cpp/stream/stream_context.h @@ -48,7 +48,7 @@ namespace grpc { class ClientContext; class RpcMethod; -class StreamContext : public StreamContextInterface { +class StreamContext final : public StreamContextInterface { public: StreamContext(const RpcMethod& method, ClientContext* context, const google::protobuf::Message* request, @@ -63,7 +63,7 @@ class StreamContext : public StreamContextInterface { bool Read(google::protobuf::Message* msg) override; bool Write(const google::protobuf::Message* msg, bool is_last) override; const Status& Wait() override; - void FinishStream(const Status& status, bool send) override; + void Cancel() override; google::protobuf::Message* request() override { return request_; } google::protobuf::Message* response() override { return result_; } diff --git a/third_party/libevent b/third_party/libevent new file mode 160000 index 00000000000..f7d92c63928 --- /dev/null +++ b/third_party/libevent @@ -0,0 +1 @@ +Subproject commit f7d92c63928a1460f3d99b9bc418bd3b686a0dca From 3f8bd048af276a40805e36a179a76af2e60f8399 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 12:48:54 -0800 Subject: [PATCH 16/26] Fix return behavior of run_tools.py If not running in forever mode, and a test fails, fail run_tests.py also. If running in forever mode and make fails, wait for the next run. --- tools/run_tests/run_tests.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 84193a1337c..428f6c41b4b 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -78,17 +78,20 @@ def _build_and_run(check_cancelled): 'CONFIG=%s' % cfg] + _MAKE_TEST_TARGETS for cfg in build_configs), check_cancelled, maxjobs=1): - sys.exit(1) + return 1 # run all the tests - jobset.run(( + if not jobset.run(( config.run_command(x) for config in run_configs for filt in filters for x in itertools.chain.from_iterable(itertools.repeat( glob.glob('bins/%s/%s_test' % ( config.build_config, filt)), - runs_per_test))), check_cancelled) + runs_per_test))), check_cancelled): + return 2 + + return 0 if forever: @@ -100,5 +103,5 @@ if forever: while not have_files_changed(): time.sleep(1) else: - _build_and_run(lambda: False) + sys.exit(_build_and_run(lambda: False)) From 09b637538f4e7e65d6d3994929f386a809317c21 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 13:02:09 -0800 Subject: [PATCH 17/26] clang-format --- include/grpc/grpc.h | 8 +++++--- src/core/surface/call.c | 17 +++++++++++------ src/cpp/client/channel.cc | 5 ++--- src/cpp/stream/stream_context.cc | 20 +++++++++----------- test/core/end2end/tests/thread_stress.c | 4 ++-- 5 files changed, 29 insertions(+), 25 deletions(-) diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index 53e0d056305..8ab3d258732 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -370,12 +370,14 @@ grpc_call_error grpc_call_server_end_initial_metadata(grpc_call *call, grpc_call_error grpc_call_cancel(grpc_call *call); /* Called by clients to cancel an RPC on the server. - Can be called multiple times, from any thread. + Can be called multiple times, from any thread. If a status has not been received for the call, set it to the status code - and description passed in. + and description passed in. Importantly, this function does not send status nor description to the remote endpoint. */ -grpc_call_error grpc_call_cancel_with_status(grpc_call *call, grpc_status_code status, const char *description); +grpc_call_error grpc_call_cancel_with_status(grpc_call *call, + grpc_status_code status, + const char *description); /* Queue a byte buffer for writing. flags is a bit-field combination of the write flags defined above. diff --git a/src/core/surface/call.c b/src/core/surface/call.c index 87e3de88e2d..24c1567db92 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -286,9 +286,9 @@ static void maybe_set_status_code(grpc_call *call, gpr_uint32 status) { } static void maybe_set_status_details(grpc_call *call, grpc_mdstr *status) { - if (!call->status_details) { - call->status_details = grpc_mdstr_ref(status); - } + if (!call->status_details) { + call->status_details = grpc_mdstr_ref(status); + } } grpc_call_error grpc_call_cancel(grpc_call *c) { @@ -307,8 +307,12 @@ grpc_call_error grpc_call_cancel(grpc_call *c) { return GRPC_CALL_OK; } -grpc_call_error grpc_call_cancel_with_status(grpc_call *c, grpc_status_code status, const char *description) { - grpc_mdstr *details = description? grpc_mdstr_from_string(c->metadata_context, description) : NULL; +grpc_call_error grpc_call_cancel_with_status(grpc_call *c, + grpc_status_code status, + const char *description) { + grpc_mdstr *details = + description ? grpc_mdstr_from_string(c->metadata_context, description) + : NULL; gpr_mu_lock(&c->read_mu); maybe_set_status_code(c, status); if (details) { @@ -898,7 +902,8 @@ void grpc_call_recv_metadata(grpc_call_element *elem, grpc_call_op *op) { grpc_call *call = CALL_FROM_TOP_ELEM(elem); grpc_mdelem *md = op->data.metadata; grpc_mdstr *key = md->key; - gpr_log(GPR_DEBUG, "call %p got metadata %s %s", call, grpc_mdstr_as_c_string(md->key), grpc_mdstr_as_c_string(md->value)); + gpr_log(GPR_DEBUG, "call %p got metadata %s %s", call, + grpc_mdstr_as_c_string(md->key), grpc_mdstr_as_c_string(md->value)); if (key == grpc_channel_get_status_string(call->channel)) { maybe_set_status_code(call, decode_status(md)); grpc_mdelem_unref(md); diff --git a/src/cpp/client/channel.cc b/src/cpp/client/channel.cc index 223151932ca..f476f77a49e 100644 --- a/src/cpp/client/channel.cc +++ b/src/cpp/client/channel.cc @@ -114,9 +114,8 @@ Status Channel::StartBlockingRpc(const RpcMethod& method, // add_metadata from context // // invoke - GPR_ASSERT(grpc_call_invoke(call, cq, metadata_read_tag, - finished_tag, - GRPC_WRITE_BUFFER_HINT) == GRPC_CALL_OK); + GPR_ASSERT(grpc_call_invoke(call, cq, metadata_read_tag, finished_tag, + GRPC_WRITE_BUFFER_HINT) == GRPC_CALL_OK); // write request grpc_byte_buffer* write_buffer = nullptr; bool success = SerializeProto(request, &write_buffer); diff --git a/src/cpp/stream/stream_context.cc b/src/cpp/stream/stream_context.cc index aec226b23c0..ebe71594c02 100644 --- a/src/cpp/stream/stream_context.cc +++ b/src/cpp/stream/stream_context.cc @@ -80,9 +80,8 @@ void StreamContext::Start(bool buffered) { if (is_client_) { // TODO(yangg) handle metadata send path int flag = buffered ? GRPC_WRITE_BUFFER_HINT : 0; - grpc_call_error error = grpc_call_invoke(call(), cq(), - client_metadata_read_tag(), - finished_tag(), flag); + grpc_call_error error = grpc_call_invoke( + call(), cq(), client_metadata_read_tag(), finished_tag(), flag); GPR_ASSERT(GRPC_CALL_OK == error); } else { // TODO(yangg) metadata needs to be added before accept @@ -105,7 +104,8 @@ bool StreamContext::Read(google::protobuf::Message* msg) { if (read_ev->data.read) { if (!DeserializeProto(read_ev->data.read, msg)) { ret = false; - grpc_call_cancel_with_status(call(), GRPC_STATUS_DATA_LOSS, "Failed to parse incoming proto"); + grpc_call_cancel_with_status(call(), GRPC_STATUS_DATA_LOSS, + "Failed to parse incoming proto"); } } else { ret = false; @@ -123,7 +123,8 @@ bool StreamContext::Write(const google::protobuf::Message* msg, bool is_last) { if (msg) { grpc_byte_buffer* out_buf = nullptr; if (!SerializeProto(*msg, &out_buf)) { - grpc_call_cancel_with_status(call(), GRPC_STATUS_INVALID_ARGUMENT, "Failed to serialize outgoing proto"); + grpc_call_cancel_with_status(call(), GRPC_STATUS_INVALID_ARGUMENT, + "Failed to serialize outgoing proto"); return false; } int flag = is_last ? GRPC_WRITE_BUFFER_HINT : 0; @@ -162,20 +163,17 @@ const Status& StreamContext::Wait() { // TODO(yangg) protect states by a mutex, including other places. if (!self_halfclosed_ || !peer_halfclosed_) { Cancel(); - } + } grpc_event* finish_ev = grpc_completion_queue_pluck(cq(), finished_tag(), gpr_inf_future); GPR_ASSERT(finish_ev->type == GRPC_FINISHED); final_status_ = Status( static_cast(finish_ev->data.finished.status), - finish_ev->data.finished.details ? finish_ev->data.finished.details - : ""); + finish_ev->data.finished.details ? finish_ev->data.finished.details : ""); grpc_event_finish(finish_ev); return final_status_; } -void StreamContext::Cancel() { - grpc_call_cancel(call()); -} +void StreamContext::Cancel() { grpc_call_cancel(call()); } } // namespace grpc diff --git a/test/core/end2end/tests/thread_stress.c b/test/core/end2end/tests/thread_stress.c index 36da4950bbc..3b571ebd084 100644 --- a/test/core/end2end/tests/thread_stress.c +++ b/test/core/end2end/tests/thread_stress.c @@ -116,8 +116,8 @@ static void start_request() { gpr_slice_unref(slice); g_active_requests++; - GPR_ASSERT(GRPC_CALL_OK == grpc_call_invoke(call, g_fixture.client_cq, - NULL, NULL, 0)); + GPR_ASSERT(GRPC_CALL_OK == + grpc_call_invoke(call, g_fixture.client_cq, NULL, NULL, 0)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_read(call, NULL)); GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_write(call, buf, NULL, 0)); From 92eb29a7b743d90abcdb77631b629214feee2352 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 14:12:17 -0800 Subject: [PATCH 18/26] Remove unused private field --- test/cpp/qps/server.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/cpp/qps/server.cc b/test/cpp/qps/server.cc index b2a4cde59fa..eb810b8d559 100644 --- a/test/cpp/qps/server.cc +++ b/test/cpp/qps/server.cc @@ -80,9 +80,6 @@ bool SetPayload(PayloadType type, int size, Payload* payload) { } class TestServiceImpl : public TestService::Service { - private: - int num_rpcs; - public: Status CollectServerStats(ServerContext* context, const StatsRequest*, ServerStats* response) { From 0d65c784db24aaa5b3db7996e971b1898cec3bf1 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 14:24:31 -0800 Subject: [PATCH 19/26] Filter out ssl tests in the sans: they dont work yet --- tools/run_tests/run_tests.py | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 428f6c41b4b..2c27cd777a2 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -29,13 +29,23 @@ class ValgrindConfig(object): return ['valgrind', binary] +# SanConfig: compile with CONFIG=config, filter out incompatible binaries +class SanConfig(object): + def __init__(self, config): + self.build_config = config + + def run_command(self, binary): + if '_ssl_' in binary: + return None + return [binary] + # different configurations we can run under _CONFIGS = { 'dbg': SimpleConfig('dbg'), 'opt': SimpleConfig('opt'), - 'tsan': SimpleConfig('tsan'), - 'msan': SimpleConfig('msan'), - 'asan': SimpleConfig('asan'), + 'tsan': SanConfig('tsan'), + 'msan': SanConfig('msan'), + 'asan': SanConfig('asan'), 'gcov': SimpleConfig('gcov'), 'valgrind': ValgrindConfig('dbg'), } @@ -81,14 +91,15 @@ def _build_and_run(check_cancelled): return 1 # run all the tests - if not jobset.run(( - config.run_command(x) - for config in run_configs - for filt in filters - for x in itertools.chain.from_iterable(itertools.repeat( - glob.glob('bins/%s/%s_test' % ( - config.build_config, filt)), - runs_per_test))), check_cancelled): + if not jobset.run(itertools.ifilter( + lambda x: x is not None, ( + config.run_command(x) + for config in run_configs + for filt in filters + for x in itertools.chain.from_iterable(itertools.repeat( + glob.glob('bins/%s/%s_test' % ( + config.build_config, filt)), + runs_per_test)))), check_cancelled): return 2 return 0 From 1371abd306e33f87b8016a587e3decc73b3fafdf Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 15:58:45 -0800 Subject: [PATCH 20/26] Fix refcounting bug --- src/core/surface/call.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/surface/call.c b/src/core/surface/call.c index 24c1567db92..6f8e0d5db7c 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -465,6 +465,8 @@ static void call_started(void *user_data, grpc_op_error error) { done_writes_done(call, error); } } + + grpc_call_internal_unref(call); } grpc_call_error grpc_call_invoke(grpc_call *call, grpc_completion_queue *cq, @@ -531,6 +533,7 @@ grpc_call_error grpc_call_invoke(grpc_call *call, grpc_completion_queue *cq, op.done_cb = call_started; op.data.start.pollset = grpc_cq_pollset(cq); op.user_data = call; + grpc_call_internal_ref(call); elem = CALL_ELEM_FROM_CALL(call, 0); elem->filter->call_op(elem, NULL, &op); From d63b7896cc6509924548da2236bcfe77140bdb3e Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 15:59:44 -0800 Subject: [PATCH 21/26] Helgrind support for run_tests.py Also allow maxjobs to be tweaked based upon which configs are being run, to prevent memory starvation. --- tools/run_tests/run_tests.py | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 2c27cd777a2..eee6f0136dd 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -15,6 +15,7 @@ import watch_dirs class SimpleConfig(object): def __init__(self, config): self.build_config = config + self.maxjobs = 32 * multiprocessing.cpu_count() def run_command(self, binary): return [binary] @@ -22,17 +23,20 @@ class SimpleConfig(object): # ValgrindConfig: compile with some CONFIG=config, but use valgrind to run class ValgrindConfig(object): - def __init__(self, config): + def __init__(self, config, tool): self.build_config = config + self.tool = tool + self.maxjobs = 4 * multiprocessing.cpu_count() def run_command(self, binary): - return ['valgrind', binary] + return ['valgrind', binary, '--tool=%s' % self.tool] # SanConfig: compile with CONFIG=config, filter out incompatible binaries class SanConfig(object): def __init__(self, config): self.build_config = config + self.maxjobs = 16 * multiprocessing.cpu_count() def run_command(self, binary): if '_ssl_' in binary: @@ -47,7 +51,8 @@ _CONFIGS = { 'msan': SanConfig('msan'), 'asan': SanConfig('asan'), 'gcov': SimpleConfig('gcov'), - 'valgrind': ValgrindConfig('dbg'), + 'memcheck': ValgrindConfig('dbg', 'memcheck'), + 'helgrind': ValgrindConfig('dbg', 'helgrind') } @@ -91,15 +96,18 @@ def _build_and_run(check_cancelled): return 1 # run all the tests - if not jobset.run(itertools.ifilter( - lambda x: x is not None, ( - config.run_command(x) - for config in run_configs - for filt in filters - for x in itertools.chain.from_iterable(itertools.repeat( - glob.glob('bins/%s/%s_test' % ( - config.build_config, filt)), - runs_per_test)))), check_cancelled): + if not jobset.run( + itertools.ifilter( + lambda x: x is not None, ( + config.run_command(x) + for config in run_configs + for filt in filters + for x in itertools.chain.from_iterable(itertools.repeat( + glob.glob('bins/%s/%s_test' % ( + config.build_config, filt)), + runs_per_test)))), + check_cancelled, + maxjobs=min(c.maxjobs for c in run_configs)): return 2 return 0 From 240d02c0760cdc67345c972b18d43aa1bfea9ccf Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 16:16:52 -0800 Subject: [PATCH 22/26] Echo test uses grpc_call_invoke --- test/core/echo/client.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/core/echo/client.c b/test/core/echo/client.c index 1905863e117..5581cd7582b 100644 --- a/test/core/echo/client.c +++ b/test/core/echo/client.c @@ -79,11 +79,8 @@ int main(int argc, char **argv) { GPR_ASSERT(argc == 2); channel = grpc_channel_create(argv[1], NULL); call = grpc_channel_create_call(channel, "/foo", "localhost", gpr_inf_future); - GPR_ASSERT(grpc_call_start_invoke(call, cq, (void *)1, (void *)1, (void *)1, + GPR_ASSERT(grpc_call_invoke(call, cq, (void *)1, (void *)1, 0) == GRPC_CALL_OK); - ev = grpc_completion_queue_next(cq, gpr_inf_future); - GPR_ASSERT(ev->data.invoke_accepted == GRPC_OP_OK); - grpc_event_finish(ev); start_write_next_slice(call, bytes_written, WRITE_SLICE_LENGTH); bytes_written += WRITE_SLICE_LENGTH; From ca757968110a4ed0004a85e2357a2f277a0a54a6 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 16:17:05 -0800 Subject: [PATCH 23/26] Fling test uses grpc_call_invoke --- test/core/fling/client.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/core/fling/client.c b/test/core/fling/client.c index cc661c34c5f..068565edc20 100644 --- a/test/core/fling/client.c +++ b/test/core/fling/client.c @@ -55,9 +55,8 @@ static void init_ping_pong_request() {} static void step_ping_pong_request() { call = grpc_channel_create_call(channel, "/Reflector/reflectUnary", "localhost", gpr_inf_future); - GPR_ASSERT(grpc_call_start_invoke(call, cq, (void *)1, (void *)1, (void *)1, + GPR_ASSERT(grpc_call_invoke(call, cq, (void *)1, (void *)1, GRPC_WRITE_BUFFER_HINT) == GRPC_CALL_OK); - grpc_event_finish(grpc_completion_queue_next(cq, gpr_inf_future)); GPR_ASSERT(grpc_call_start_write(call, the_buffer, (void *)1, GRPC_WRITE_BUFFER_HINT) == GRPC_CALL_OK); grpc_event_finish(grpc_completion_queue_next(cq, gpr_inf_future)); @@ -66,7 +65,6 @@ static void step_ping_pong_request() { 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; } @@ -74,10 +72,9 @@ static void step_ping_pong_request() { static void init_ping_pong_stream() { call = grpc_channel_create_call(channel, "/Reflector/reflectStream", "localhost", gpr_inf_future); - GPR_ASSERT(grpc_call_start_invoke(call, cq, (void *)1, (void *)1, (void *)1, + GPR_ASSERT(grpc_call_invoke(call, cq, (void *)1, (void *)1, 0) == GRPC_CALL_OK); grpc_event_finish(grpc_completion_queue_next(cq, gpr_inf_future)); - grpc_event_finish(grpc_completion_queue_next(cq, gpr_inf_future)); } static void step_ping_pong_stream() { From 808632ed4b9757c79bfbb8917ec79ca4a4a614d0 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 16:17:21 -0800 Subject: [PATCH 24/26] Remove grpc_call_start_invoke --- include/grpc/grpc.h | 7 ------- src/core/surface/call.c | 15 --------------- 2 files changed, 22 deletions(-) diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index 8ab3d258732..f9a28e6f69b 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -313,8 +313,6 @@ grpc_call_error grpc_call_add_metadata(grpc_call *call, grpc_metadata *metadata, flags is a bit-field combination of the write flags defined above. REQUIRES: Can be called at most once per call. Can only be called on the client. - Produces a GRPC_INVOKE_ACCEPTED event with invoke_accepted_tag when the - call has been invoked (meaning bytes can start flowing to the wire). Produces a GRPC_CLIENT_METADATA_READ event with metadata_read_tag when the servers initial metadata has been read. Produces a GRPC_FINISHED event with finished_tag when the call has been @@ -323,11 +321,6 @@ grpc_call_error grpc_call_add_metadata(grpc_call *call, grpc_metadata *metadata, grpc_call_error grpc_call_invoke(grpc_call *call, grpc_completion_queue *cq, void *metadata_read_tag, void *finished_tag, gpr_uint32 flags); -grpc_call_error grpc_call_start_invoke(grpc_call *call, - grpc_completion_queue *cq, - void *invoke_accepted_tag, - void *metadata_read_tag, - void *finished_tag, gpr_uint32 flags); /* DEPRECATED: users should use grpc_call_server_accept, and grpc_call_server_end_initial_metadata instead now. diff --git a/src/core/surface/call.c b/src/core/surface/call.c index 6f8e0d5db7c..eb39044e939 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -382,21 +382,6 @@ static void finish_call(grpc_call *call) { elements, count); } -grpc_call_error grpc_call_start_invoke(grpc_call *call, - grpc_completion_queue *cq, - void *invoke_accepted_tag, - void *metadata_read_tag, - void *finished_tag, gpr_uint32 flags) { - grpc_call_error err = - grpc_call_invoke(call, cq, metadata_read_tag, finished_tag, flags); - if (err == GRPC_CALL_OK) { - grpc_cq_begin_op(call->cq, call, GRPC_INVOKE_ACCEPTED); - grpc_cq_end_invoke_accepted(call->cq, invoke_accepted_tag, call, do_nothing, - NULL, GRPC_OP_OK); - } - return err; -} - static void done_write(void *user_data, grpc_op_error error) { grpc_call *call = user_data; void *tag = call->write_tag; From 408c7383cc56cf9531657ea17b1f9fdc5c41891b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 16:25:47 -0800 Subject: [PATCH 25/26] clang-format --- test/core/echo/client.c | 4 ++-- test/core/fling/client.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/core/echo/client.c b/test/core/echo/client.c index 5581cd7582b..2ad29df53ca 100644 --- a/test/core/echo/client.c +++ b/test/core/echo/client.c @@ -79,8 +79,8 @@ int main(int argc, char **argv) { GPR_ASSERT(argc == 2); channel = grpc_channel_create(argv[1], NULL); call = grpc_channel_create_call(channel, "/foo", "localhost", gpr_inf_future); - GPR_ASSERT(grpc_call_invoke(call, cq, (void *)1, (void *)1, - 0) == GRPC_CALL_OK); + GPR_ASSERT(grpc_call_invoke(call, cq, (void *)1, (void *)1, 0) == + GRPC_CALL_OK); start_write_next_slice(call, bytes_written, WRITE_SLICE_LENGTH); bytes_written += WRITE_SLICE_LENGTH; diff --git a/test/core/fling/client.c b/test/core/fling/client.c index 068565edc20..0e21a064017 100644 --- a/test/core/fling/client.c +++ b/test/core/fling/client.c @@ -56,7 +56,7 @@ static void step_ping_pong_request() { call = grpc_channel_create_call(channel, "/Reflector/reflectUnary", "localhost", gpr_inf_future); GPR_ASSERT(grpc_call_invoke(call, cq, (void *)1, (void *)1, - GRPC_WRITE_BUFFER_HINT) == GRPC_CALL_OK); + GRPC_WRITE_BUFFER_HINT) == GRPC_CALL_OK); GPR_ASSERT(grpc_call_start_write(call, the_buffer, (void *)1, GRPC_WRITE_BUFFER_HINT) == GRPC_CALL_OK); grpc_event_finish(grpc_completion_queue_next(cq, gpr_inf_future)); @@ -72,8 +72,8 @@ static void step_ping_pong_request() { static void init_ping_pong_stream() { call = grpc_channel_create_call(channel, "/Reflector/reflectStream", "localhost", gpr_inf_future); - GPR_ASSERT(grpc_call_invoke(call, cq, (void *)1, (void *)1, - 0) == GRPC_CALL_OK); + GPR_ASSERT(grpc_call_invoke(call, cq, (void *)1, (void *)1, 0) == + GRPC_CALL_OK); grpc_event_finish(grpc_completion_queue_next(cq, gpr_inf_future)); } From 0fc50596c7803c2b33ed08d7c72c109f1ae76497 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jan 2015 17:49:59 -0800 Subject: [PATCH 26/26] Initialize start_ok --- 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 eb39044e939..d8a34cf68dc 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -232,6 +232,7 @@ grpc_call *grpc_call_create(grpc_channel *channel, call->have_alarm = 0; call->received_metadata = 0; call->got_status_code = 0; + call->start_ok = 0; call->status_code = server_transport_data != NULL ? GRPC_STATUS_OK : GRPC_STATUS_UNKNOWN; call->status_details = NULL;