From 1d5aca5de05d16968f0f1e208a9aa10d7c06caac Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Sun, 14 Jun 2015 14:42:04 -0700 Subject: Added flags support to grpc_op. Which includes its propagation to grpc_ioreq and validation mechanisms for checking that only known bits are set ot in). Also added an internal flag (GRPC_WRITE_INTERNAL_COMPRESS) mask for its use in signaling compressed messages. --- src/core/surface/call.c | 41 ++++++++++++++++++++++++++++++++++++++++- src/core/surface/call.h | 1 + 2 files changed, 41 insertions(+), 1 deletion(-) (limited to 'src/core/surface') diff --git a/src/core/surface/call.c b/src/core/surface/call.c index cead5e08dc..4b0a4c82f0 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -185,6 +185,7 @@ struct grpc_call { and a strong upper bound of a count of masters to be calculated. */ gpr_uint8 request_set[GRPC_IOREQ_OP_COUNT]; grpc_ioreq_data request_data[GRPC_IOREQ_OP_COUNT]; + gpr_uint32 request_flags[GRPC_IOREQ_OP_COUNT]; reqinfo_master masters[GRPC_IOREQ_OP_COUNT]; /* Dynamic array of ioreq's that have completed: the count of @@ -228,6 +229,7 @@ struct grpc_call { gpr_slice_buffer incoming_message; gpr_uint32 incoming_message_length; + gpr_uint32 incoming_message_flags; grpc_iomgr_closure destroy_closure; }; @@ -670,6 +672,7 @@ static int begin_message(grpc_call *call, grpc_begin_message msg) { } else if (msg.length > 0) { call->reading_message = 1; call->incoming_message_length = msg.length; + call->incoming_message_flags = msg.flags; return 1; } else { finish_message(call); @@ -818,6 +821,7 @@ static void copy_byte_buffer_to_stream_ops(grpc_byte_buffer *byte_buffer, static int fill_send_ops(grpc_call *call, grpc_transport_op *op) { grpc_ioreq_data data; + gpr_uint32 flags; grpc_metadata_batch mdb; size_t i; GPR_ASSERT(op->send_ops == NULL); @@ -844,8 +848,9 @@ static int fill_send_ops(grpc_call *call, grpc_transport_op *op) { case WRITE_STATE_STARTED: if (is_op_live(call, GRPC_IOREQ_SEND_MESSAGE)) { data = call->request_data[GRPC_IOREQ_SEND_MESSAGE]; + flags = call->request_flags[GRPC_IOREQ_SEND_MESSAGE]; grpc_sopb_add_begin_message( - &call->send_ops, grpc_byte_buffer_length(data.send_message), 0); + &call->send_ops, grpc_byte_buffer_length(data.send_message), flags); copy_byte_buffer_to_stream_ops(data.send_message, &call->send_ops); op->send_ops = &call->send_ops; call->last_send_contains |= 1 << GRPC_IOREQ_SEND_MESSAGE; @@ -979,6 +984,7 @@ static grpc_call_error start_ioreq(grpc_call *call, const grpc_ioreq *reqs, have_ops |= 1u << op; call->request_data[op] = data; + call->request_flags[op] = reqs[i].flags; call->request_set[op] = set; } @@ -1189,6 +1195,14 @@ static void finish_batch_with_close(grpc_call *call, int success, void *tag) { grpc_cq_end_op(call->cq, tag, call, 1); } +static int are_write_flags_valid(gpr_uint32 flags) { + /* check that only bits in GRPC_WRITE_(INTERNAL?)_USED_MASK are set */ + const gpr_uint32 allowed_write_positions = + (GRPC_WRITE_USED_MASK | GRPC_WRITE_INTERNAL_USED_MASK); + const gpr_uint32 invalid_positions = ~allowed_write_positions; + return !(flags & invalid_positions); +} + grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, size_t nops, void *tag) { grpc_ioreq reqs[GRPC_IOREQ_OP_COUNT]; @@ -1211,30 +1225,43 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, op = &ops[in]; switch (op->op) { case GRPC_OP_SEND_INITIAL_METADATA: + /* Flag validation: currently allow no flags */ + if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; req = &reqs[out++]; req->op = GRPC_IOREQ_SEND_INITIAL_METADATA; req->data.send_metadata.count = op->data.send_initial_metadata.count; req->data.send_metadata.metadata = op->data.send_initial_metadata.metadata; + req->flags = op->flags; break; case GRPC_OP_SEND_MESSAGE: + if (!are_write_flags_valid(op->flags)){ + return GRPC_CALL_ERROR_INVALID_FLAGS; + } req = &reqs[out++]; req->op = GRPC_IOREQ_SEND_MESSAGE; req->data.send_message = op->data.send_message; + req->flags = ops->flags; break; case GRPC_OP_SEND_CLOSE_FROM_CLIENT: + /* Flag validation: currently allow no flags */ + if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; if (!call->is_client) { return GRPC_CALL_ERROR_NOT_ON_SERVER; } req = &reqs[out++]; req->op = GRPC_IOREQ_SEND_CLOSE; + req->flags = op->flags; break; case GRPC_OP_SEND_STATUS_FROM_SERVER: + /* Flag validation: currently allow no flags */ + if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; if (call->is_client) { return GRPC_CALL_ERROR_NOT_ON_CLIENT; } req = &reqs[out++]; req->op = GRPC_IOREQ_SEND_TRAILING_METADATA; + req->flags = op->flags; req->data.send_metadata.count = op->data.send_status_from_server.trailing_metadata_count; req->data.send_metadata.metadata = @@ -1248,24 +1275,33 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, req->op = GRPC_IOREQ_SEND_CLOSE; break; case GRPC_OP_RECV_INITIAL_METADATA: + /* Flag validation: currently allow no flags */ + if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; if (!call->is_client) { return GRPC_CALL_ERROR_NOT_ON_SERVER; } req = &reqs[out++]; req->op = GRPC_IOREQ_RECV_INITIAL_METADATA; req->data.recv_metadata = op->data.recv_initial_metadata; + req->flags = op->flags; break; case GRPC_OP_RECV_MESSAGE: + /* Flag validation: currently allow no flags */ + if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; req = &reqs[out++]; req->op = GRPC_IOREQ_RECV_MESSAGE; req->data.recv_message = op->data.recv_message; + req->flags = op->flags; break; case GRPC_OP_RECV_STATUS_ON_CLIENT: + /* Flag validation: currently allow no flags */ + if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; if (!call->is_client) { return GRPC_CALL_ERROR_NOT_ON_SERVER; } req = &reqs[out++]; req->op = GRPC_IOREQ_RECV_STATUS; + req->flags = op->flags; req->data.recv_status.set_value = set_status_value_directly; req->data.recv_status.user_data = op->data.recv_status_on_client.status; req = &reqs[out++]; @@ -1283,8 +1319,11 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, finish_func = finish_batch_with_close; break; case GRPC_OP_RECV_CLOSE_ON_SERVER: + /* Flag validation: currently allow no flags */ + if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; req = &reqs[out++]; req->op = GRPC_IOREQ_RECV_STATUS; + req->flags = op->flags; req->data.recv_status.set_value = set_cancelled_value; req->data.recv_status.user_data = op->data.recv_close_on_server.cancelled; diff --git a/src/core/surface/call.h b/src/core/surface/call.h index 9116538948..7a14161253 100644 --- a/src/core/surface/call.h +++ b/src/core/surface/call.h @@ -79,6 +79,7 @@ typedef union { typedef struct { grpc_ioreq_op op; grpc_ioreq_data data; + gpr_uint32 flags; /**< A copy of the write flags from grpc_op */ } grpc_ioreq; typedef void (*grpc_ioreq_completion_func)(grpc_call *call, int success, -- cgit v1.2.3 From b56975ceb93ac80f4789ebcc156db3a17c806bb1 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 15 Jun 2015 10:11:16 -0700 Subject: Verify that completion queues are server registered --- include/grpc/grpc.h | 5 ++++- src/core/surface/completion_queue.c | 5 +++++ src/core/surface/completion_queue.h | 3 +++ src/core/surface/server.c | 7 +++++++ 4 files changed, 19 insertions(+), 1 deletion(-) (limited to 'src/core/surface') diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index 2caa9680ba..352c0ac653 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -144,7 +144,10 @@ typedef enum grpc_call_error { /* the flags value was illegal for this call */ GRPC_CALL_ERROR_INVALID_FLAGS, /* invalid metadata was passed to this call */ - GRPC_CALL_ERROR_INVALID_METADATA + GRPC_CALL_ERROR_INVALID_METADATA, + /* completion queue for notification has not been registered with the server + */ + GRPC_CALL_ERROR_NOT_SERVER_COMPLETION_QUEUE } grpc_call_error; /* Write Flags: */ diff --git a/src/core/surface/completion_queue.c b/src/core/surface/completion_queue.c index 57ecf365cc..bd0fabf9da 100644 --- a/src/core/surface/completion_queue.c +++ b/src/core/surface/completion_queue.c @@ -73,6 +73,7 @@ struct grpc_completion_queue { event *queue; /* Fixed size chained hash table of events for pluck() */ event *buckets[NUM_TAG_BUCKETS]; + int is_server_cq; }; grpc_completion_queue *grpc_completion_queue_create(void) { @@ -323,3 +324,7 @@ void grpc_cq_hack_spin_pollset(grpc_completion_queue *cc) { gpr_time_add(gpr_now(), gpr_time_from_millis(100))); gpr_mu_unlock(GRPC_POLLSET_MU(&cc->pollset)); } + +void grpc_cq_mark_server_cq(grpc_completion_queue *cc) { cc->is_server_cq = 1; } + +int grpc_cq_is_server_cq(grpc_completion_queue *cc) { return cc->is_server_cq; } diff --git a/src/core/surface/completion_queue.h b/src/core/surface/completion_queue.h index 2249d0e789..e76910c00b 100644 --- a/src/core/surface/completion_queue.h +++ b/src/core/surface/completion_queue.h @@ -63,4 +63,7 @@ grpc_pollset *grpc_cq_pollset(grpc_completion_queue *cc); void grpc_cq_hack_spin_pollset(grpc_completion_queue *cc); +void grpc_cq_mark_server_cq(grpc_completion_queue *cc); +int grpc_cq_is_server_cq(grpc_completion_queue *cc); + #endif /* GRPC_INTERNAL_CORE_SURFACE_COMPLETION_QUEUE_H */ diff --git a/src/core/surface/server.c b/src/core/surface/server.c index 3671efe0d0..10cb8538ac 100644 --- a/src/core/surface/server.c +++ b/src/core/surface/server.c @@ -709,6 +709,7 @@ void grpc_server_register_completion_queue(grpc_server *server, if (server->cqs[i] == cq) return; } GRPC_CQ_INTERNAL_REF(cq, "server"); + grpc_cq_mark_server_cq(cq); n = server->cq_count++; server->cqs = gpr_realloc(server->cqs, server->cq_count * sizeof(grpc_completion_queue *)); @@ -1081,6 +1082,9 @@ grpc_call_error grpc_server_request_call( GRPC_SERVER_LOG_REQUEST_CALL(GPR_INFO, server, call, details, initial_metadata, cq_bound_to_call, cq_for_notification, tag); + if (!grpc_cq_is_server_cq(cq_for_notification)) { + return GRPC_CALL_ERROR_NOT_SERVER_COMPLETION_QUEUE; + } grpc_cq_begin_op(cq_for_notification, NULL); rc.type = BATCH_CALL; rc.tag = tag; @@ -1099,6 +1103,9 @@ grpc_call_error grpc_server_request_registered_call( grpc_completion_queue *cq_for_notification, void *tag) { requested_call rc; registered_method *registered_method = rm; + if (!grpc_cq_is_server_cq(cq_for_notification)) { + return GRPC_CALL_ERROR_NOT_SERVER_COMPLETION_QUEUE; + } grpc_cq_begin_op(cq_for_notification, NULL); rc.type = REGISTERED_CALL; rc.tag = tag; -- cgit v1.2.3 From de52625ae55403dff7eed9cb293722e660f22781 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Mon, 15 Jun 2015 13:15:26 -0700 Subject: Added changes to c++ and ruby wrappers --- src/core/surface/call.c | 14 +++++++------- src/cpp/common/call.cc | 8 ++++++++ src/ruby/ext/grpc/rb_call.c | 1 + 3 files changed, 16 insertions(+), 7 deletions(-) (limited to 'src/core/surface') diff --git a/src/core/surface/call.c b/src/core/surface/call.c index 4b0a4c82f0..5cdd7cd0f6 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -1226,7 +1226,7 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, switch (op->op) { case GRPC_OP_SEND_INITIAL_METADATA: /* Flag validation: currently allow no flags */ - if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; + if (op->flags != 0) return GRPC_CALL_ERROR_INVALID_FLAGS; req = &reqs[out++]; req->op = GRPC_IOREQ_SEND_INITIAL_METADATA; req->data.send_metadata.count = op->data.send_initial_metadata.count; @@ -1245,7 +1245,7 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, break; case GRPC_OP_SEND_CLOSE_FROM_CLIENT: /* Flag validation: currently allow no flags */ - if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; + if (op->flags != 0) return GRPC_CALL_ERROR_INVALID_FLAGS; if (!call->is_client) { return GRPC_CALL_ERROR_NOT_ON_SERVER; } @@ -1255,7 +1255,7 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, break; case GRPC_OP_SEND_STATUS_FROM_SERVER: /* Flag validation: currently allow no flags */ - if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; + if (op->flags != 0) return GRPC_CALL_ERROR_INVALID_FLAGS; if (call->is_client) { return GRPC_CALL_ERROR_NOT_ON_CLIENT; } @@ -1276,7 +1276,7 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, break; case GRPC_OP_RECV_INITIAL_METADATA: /* Flag validation: currently allow no flags */ - if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; + if (op->flags != 0) return GRPC_CALL_ERROR_INVALID_FLAGS; if (!call->is_client) { return GRPC_CALL_ERROR_NOT_ON_SERVER; } @@ -1287,7 +1287,7 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, break; case GRPC_OP_RECV_MESSAGE: /* Flag validation: currently allow no flags */ - if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; + if (op->flags != 0) return GRPC_CALL_ERROR_INVALID_FLAGS; req = &reqs[out++]; req->op = GRPC_IOREQ_RECV_MESSAGE; req->data.recv_message = op->data.recv_message; @@ -1295,7 +1295,7 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, break; case GRPC_OP_RECV_STATUS_ON_CLIENT: /* Flag validation: currently allow no flags */ - if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; + if (op->flags != 0) return GRPC_CALL_ERROR_INVALID_FLAGS; if (!call->is_client) { return GRPC_CALL_ERROR_NOT_ON_SERVER; } @@ -1320,7 +1320,7 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, break; case GRPC_OP_RECV_CLOSE_ON_SERVER: /* Flag validation: currently allow no flags */ - if (op->flags) return GRPC_CALL_ERROR_INVALID_FLAGS; + if (op->flags != 0) return GRPC_CALL_ERROR_INVALID_FLAGS; req = &reqs[out++]; req->op = GRPC_IOREQ_RECV_STATUS; req->flags = op->flags; diff --git a/src/cpp/common/call.cc b/src/cpp/common/call.cc index 1068111e3f..3212f770f4 100644 --- a/src/cpp/common/call.cc +++ b/src/cpp/common/call.cc @@ -224,11 +224,13 @@ void CallOpBuffer::FillOps(grpc_op* ops, size_t* nops) { ops[*nops].op = GRPC_OP_SEND_INITIAL_METADATA; ops[*nops].data.send_initial_metadata.count = initial_metadata_count_; ops[*nops].data.send_initial_metadata.metadata = initial_metadata_; + ops[*nops].flags = 0; (*nops)++; } if (recv_initial_metadata_) { ops[*nops].op = GRPC_OP_RECV_INITIAL_METADATA; ops[*nops].data.recv_initial_metadata = &recv_initial_metadata_arr_; + ops[*nops].flags = 0; (*nops)++; } if (send_message_ || send_message_buffer_) { @@ -245,15 +247,18 @@ void CallOpBuffer::FillOps(grpc_op* ops, size_t* nops) { } ops[*nops].op = GRPC_OP_SEND_MESSAGE; ops[*nops].data.send_message = send_buf_; + ops[*nops].flags = 0; (*nops)++; } if (recv_message_ || recv_message_buffer_) { ops[*nops].op = GRPC_OP_RECV_MESSAGE; ops[*nops].data.recv_message = &recv_buf_; + ops[*nops].flags = 0; (*nops)++; } if (client_send_close_) { ops[*nops].op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; + ops[*nops].flags = 0; (*nops)++; } if (recv_status_) { @@ -264,6 +269,7 @@ void CallOpBuffer::FillOps(grpc_op* ops, size_t* nops) { ops[*nops].data.recv_status_on_client.status_details = &status_details_; ops[*nops].data.recv_status_on_client.status_details_capacity = &status_details_capacity_; + ops[*nops].flags = 0; (*nops)++; } if (send_status_available_) { @@ -275,11 +281,13 @@ void CallOpBuffer::FillOps(grpc_op* ops, size_t* nops) { ops[*nops].data.send_status_from_server.status = send_status_code_; ops[*nops].data.send_status_from_server.status_details = send_status_details_.empty() ? nullptr : send_status_details_.c_str(); + ops[*nops].flags = 0; (*nops)++; } if (recv_closed_) { ops[*nops].op = GRPC_OP_RECV_CLOSE_ON_SERVER; ops[*nops].data.recv_close_on_server.cancelled = &cancelled_buf_; + ops[*nops].flags = 0; (*nops)++; } } diff --git a/src/ruby/ext/grpc/rb_call.c b/src/ruby/ext/grpc/rb_call.c index 29f870f929..33bfd006da 100644 --- a/src/ruby/ext/grpc/rb_call.c +++ b/src/ruby/ext/grpc/rb_call.c @@ -507,6 +507,7 @@ static void grpc_run_batch_stack_fill_ops(run_batch_stack *st, VALUE ops_hash) { NUM2INT(this_op)); }; st->ops[st->op_num].op = (grpc_op_type)NUM2INT(this_op); + st->ops[st->op_num].flags = 0; st->op_num++; } } -- cgit v1.2.3 From b8f54505141724ad15dfe9c9c4edd8f1faf08fdf Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Mon, 15 Jun 2015 16:19:10 -0700 Subject: More missing flags = 0 lines... --- src/core/surface/server.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/core/surface') diff --git a/src/core/surface/server.c b/src/core/surface/server.c index 733f0e8a11..73995202a4 100644 --- a/src/core/surface/server.c +++ b/src/core/surface/server.c @@ -1111,6 +1111,7 @@ static void begin_call(grpc_server *server, call_data *calld, rc->data.batch.details->deadline = calld->deadline; r->op = GRPC_IOREQ_RECV_INITIAL_METADATA; r->data.recv_metadata = rc->data.batch.initial_metadata; + r->flags = 0; r++; publish = publish_registered_or_batch; break; @@ -1118,10 +1119,12 @@ static void begin_call(grpc_server *server, call_data *calld, *rc->data.registered.deadline = calld->deadline; r->op = GRPC_IOREQ_RECV_INITIAL_METADATA; r->data.recv_metadata = rc->data.registered.initial_metadata; + r->flags = 0; r++; if (rc->data.registered.optional_payload) { r->op = GRPC_IOREQ_RECV_MESSAGE; r->data.recv_message = rc->data.registered.optional_payload; + r->flags = 0; r++; } publish = publish_registered_or_batch; -- cgit v1.2.3