From 42dab364a337666003a17d72b4dcad0c4568587a Mon Sep 17 00:00:00 2001 From: Greg Haines Date: Wed, 2 Mar 2016 15:04:41 -0800 Subject: Pass a non-infinite deadline to grpc_completion_queue_next() to prevent queues from blocking indefinitely in poll(). --- .../GRPCClient/private/GRPCCompletionQueue.h | 7 ++++++ .../GRPCClient/private/GRPCCompletionQueue.m | 25 ++++++++++++++++------ 2 files changed, 25 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.h b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.h index fe3b8f39d1..03fd2e0d95 100644 --- a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.h +++ b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.h @@ -36,6 +36,8 @@ typedef void(^GRPCQueueCompletionHandler)(bool success); +extern const int64_t kGRPCCompletionQueueDefaultTimeoutSecs; + /** * This class lets one more easily use |grpc_completion_queue|. To use it, pass the value of the * |unmanagedQueue| property of an instance of this class to |grpc_channel_create_call|. Then for @@ -49,6 +51,11 @@ typedef void(^GRPCQueueCompletionHandler)(bool success); */ @interface GRPCCompletionQueue : NSObject @property(nonatomic, readonly) grpc_completion_queue *unmanagedQueue; +@property(nonatomic, readonly) int64_t timeoutSecs; + (instancetype)completionQueue; + +- (instancetype)init; +- (instancetype)initWithTimeout:(int64_t)timeoutSecs NS_DESIGNATED_INITIALIZER; + @end diff --git a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m index ea2b01ee1d..8163236cc4 100644 --- a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m +++ b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m @@ -35,6 +35,9 @@ #import + +const int64_t kGRPCCompletionQueueDefaultTimeoutSecs = 60; + @implementation GRPCCompletionQueue + (instancetype)completionQueue { @@ -42,8 +45,13 @@ } - (instancetype)init { + return [self initWithTimeout:kGRPCCompletionQueueDefaultTimeoutSecs]; +} + +- (instancetype)initWithTimeout:(int64_t)timeoutSecs { if ((self = [super init])) { _unmanagedQueue = grpc_completion_queue_create(NULL); + _timeoutSecs = timeoutSecs; // This is for the following block to capture the pointer by value (instead // of retaining self and doing self->_unmanagedQueue). This is essential @@ -52,6 +60,7 @@ // anymore (i.e. on self dealloc). So the block would never end if it // retained self. grpc_completion_queue *unmanagedQueue = _unmanagedQueue; + int64_t lTimeoutSecs = _timeoutSecs; // Start a loop on a concurrent queue to read events from the completion // queue and dispatch each. @@ -61,22 +70,24 @@ gDefaultConcurrentQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); }); dispatch_async(gDefaultConcurrentQueue, ^{ + gpr_timespec deadline = gpr_time_from_seconds(lTimeoutSecs, GPR_CLOCK_REALTIME); while (YES) { - // The following call blocks until an event is available. - grpc_event event = grpc_completion_queue_next(unmanagedQueue, - gpr_inf_future(GPR_CLOCK_REALTIME), - NULL); + // The following call blocks until an event is available or the deadline elapses. + grpc_event event = grpc_completion_queue_next(unmanagedQueue, deadline, NULL); GRPCQueueCompletionHandler handler; switch (event.type) { - case GRPC_OP_COMPLETE: + case GRPC_OP_COMPLETE: // Falling through deliberately + case GRPC_QUEUE_TIMEOUT: handler = (__bridge_transfer GRPCQueueCompletionHandler)event.tag; - handler(event.success); + if (handler) { + handler(event.success); + } break; case GRPC_QUEUE_SHUTDOWN: grpc_completion_queue_destroy(unmanagedQueue); return; default: - [NSException raise:@"Unrecognized completion type" format:@""]; + [NSException raise:@"Unrecognized completion type" format:@"type=%d", event.type]; } }; }); -- cgit v1.2.3 From 91bd627de4fb1ab1b8c31a915d4a91ccdcf72bbe Mon Sep 17 00:00:00 2001 From: Greg Haines Date: Wed, 2 Mar 2016 23:05:46 -0800 Subject: Feedback from @jcanizales and @vjpai --- src/objective-c/GRPCClient/private/GRPCCompletionQueue.m | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m index 8163236cc4..ffbb14374d 100644 --- a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m +++ b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m @@ -60,7 +60,6 @@ const int64_t kGRPCCompletionQueueDefaultTimeoutSecs = 60; // anymore (i.e. on self dealloc). So the block would never end if it // retained self. grpc_completion_queue *unmanagedQueue = _unmanagedQueue; - int64_t lTimeoutSecs = _timeoutSecs; // Start a loop on a concurrent queue to read events from the completion // queue and dispatch each. @@ -70,18 +69,18 @@ const int64_t kGRPCCompletionQueueDefaultTimeoutSecs = 60; gDefaultConcurrentQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); }); dispatch_async(gDefaultConcurrentQueue, ^{ - gpr_timespec deadline = gpr_time_from_seconds(lTimeoutSecs, GPR_CLOCK_REALTIME); + gpr_timespec deadline = gpr_time_from_seconds(timeoutSecs, GPR_CLOCK_REALTIME); while (YES) { // The following call blocks until an event is available or the deadline elapses. grpc_event event = grpc_completion_queue_next(unmanagedQueue, deadline, NULL); GRPCQueueCompletionHandler handler; switch (event.type) { - case GRPC_OP_COMPLETE: // Falling through deliberately - case GRPC_QUEUE_TIMEOUT: + case GRPC_OP_COMPLETE: handler = (__bridge_transfer GRPCQueueCompletionHandler)event.tag; - if (handler) { - handler(event.success); - } + handler(event.success); + break; + case GRPC_QUEUE_TIMEOUT: + // Nothing to do here break; case GRPC_QUEUE_SHUTDOWN: grpc_completion_queue_destroy(unmanagedQueue); -- cgit v1.2.3 From aaf66a9981a658b62be005ac5e04d704545ab2da Mon Sep 17 00:00:00 2001 From: Greg Haines Date: Fri, 4 Mar 2016 13:10:26 -0800 Subject: Add comments and feature flag. --- src/objective-c/GRPCClient/private/GRPCCompletionQueue.m | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m index ffbb14374d..b7d5af67d9 100644 --- a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m +++ b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m @@ -69,7 +69,11 @@ const int64_t kGRPCCompletionQueueDefaultTimeoutSecs = 60; gDefaultConcurrentQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); }); dispatch_async(gDefaultConcurrentQueue, ^{ - gpr_timespec deadline = gpr_time_from_seconds(timeoutSecs, GPR_CLOCK_REALTIME); + // Using a non-infinite deadline to re-enter grpc_completion_queue_next() + // alleviates https://github.com/grpc/grpc/issues/5593 + gpr_timespec deadline = (timeoutSecs < 0) + ? gpr_inf_future(GPR_CLOCK_REALTIME) + : gpr_time_from_seconds(timeoutSecs, GPR_CLOCK_REALTIME); while (YES) { // The following call blocks until an event is available or the deadline elapses. grpc_event event = grpc_completion_queue_next(unmanagedQueue, deadline, NULL); -- cgit v1.2.3