Multiple fixes

1 files changed, 73 insertions, 72 deletions

diff --git a/src/objective-c/GRPCClient/private/GRPCChannelPool.m b/src/objective-c/GRPCClient/private/GRPCChannelPool.m
index 7c139b3717..f6615b5840 100644
--- a/src/objective-c/GRPCClient/private/GRPCChannelPool.m
+++ b/src/objective-c/GRPCClient/private/GRPCChannelPool.m
@@ -52,10 +52,9 @@ static const NSTimeInterval kDefaultChannelDestroyDelay = 30;
   __weak GRPCChannelPool *_channelPool;
   GRPCChannelConfiguration *_channelConfiguration;
   NSMutableSet *_unmanagedCalls;
+  GRPCChannel *_wrappedChannel;
 }
 
-@synthesize wrappedChannel = _wrappedChannel;
-
 - (instancetype)initWithChannelConfiguration:(GRPCChannelConfiguration *)channelConfiguration
                                  channelPool:(GRPCChannelPool *)channelPool {
   NSAssert(channelConfiguration != nil, @"channelConfiguration cannot be empty.");
@@ -68,11 +67,17 @@ static const NSTimeInterval kDefaultChannelDestroyDelay = 30;
     _channelPool = channelPool;
     _channelConfiguration = channelConfiguration;
     _unmanagedCalls = [NSMutableSet set];
+    _wrappedChannel = nil;
   }
 
   return self;
 }
 
+- (void)dealloc {
+  NSAssert([_unmanagedCalls count] == 0 && _wrappedChannel == nil, @"Pooled channel should only be"
+           "destroyed after the wrapped channel is destroyed");
+}
+
 - (grpc_call *)unmanagedCallWithPath:(NSString *)path
                      completionQueue:(GRPCCompletionQueue *)queue
                          callOptions:(GRPCCallOptions *)callOptions {
@@ -99,31 +104,38 @@ static const NSTimeInterval kDefaultChannelDestroyDelay = 30;
   return call;
 }
 
-- (void)unrefUnmanagedCall:(grpc_call *)unmanagedCall {
-  if (unmanagedCall == nil) return;
+- (void)destroyUnmanagedCall:(grpc_call *)unmanagedCall {
+  if (unmanagedCall == NULL) {
+    return;
+  }
 
   grpc_call_unref(unmanagedCall);
-  BOOL timedDestroy = NO;
   @synchronized(self) {
-    if ([_unmanagedCalls containsObject:[NSValue valueWithPointer:unmanagedCall]]) {
-      [_unmanagedCalls removeObject:[NSValue valueWithPointer:unmanagedCall]];
-      if ([_unmanagedCalls count] == 0) {
-        timedDestroy = YES;
-      }
+    NSValue *removedCall = [NSValue valueWithPointer:unmanagedCall];
+    [_unmanagedCalls removeObject:removedCall];
+    if ([_unmanagedCalls count] == 0) {
+      _wrappedChannel = nil;
+      GRPCChannelPool *strongPool = _channelPool;
+      [strongPool unrefChannelWithConfiguration:_channelConfiguration];
     }
   }
-  if (timedDestroy) {
-    [self timedDestroy];
-  }
 }
 
 - (void)disconnect {
   @synchronized(self) {
-    _wrappedChannel = nil;
-    [_unmanagedCalls removeAllObjects];
+    if (_wrappedChannel != nil) {
+      _wrappedChannel = nil;
+      [_unmanagedCalls removeAllObjects];
+      GRPCChannelPool *strongPool = _channelPool;
+      [strongPool unrefChannelWithConfiguration:_channelConfiguration];
+    }
   }
 }
 
+@end
+
+@implementation GRPCPooledChannel (Test)
+
 - (GRPCChannel *)wrappedChannel {
   GRPCChannel *channel = nil;
   @synchronized(self) {
@@ -132,67 +144,52 @@ static const NSTimeInterval kDefaultChannelDestroyDelay = 30;
   return channel;
 }
 
-- (void)timedDestroy {
-  __strong GRPCChannelPool *pool = nil;
-  @synchronized(self) {
-    // Check if we still want to destroy the channel.
-    if ([_unmanagedCalls count] == 0) {
-      pool = _channelPool;
-      _wrappedChannel = nil;
-    }
-  }
-  [pool unrefChannelWithConfiguration:_channelConfiguration];
-}
-
 @end
 
 /**
  * A convenience value type for cached channel.
  */
-@interface GRPCChannelRecord : NSObject<NSCopying>
+@interface GRPCChannelRecord : NSObject
 
 /** Pointer to the raw channel. May be nil when the channel has been destroyed. */
 @property GRPCChannel *channel;
 
 /** Channel proxy corresponding to this channel configuration. */
-@property GRPCPooledChannel *proxy;
+@property GRPCPooledChannel *pooledChannel;
 
 /** Last time when a timed destroy is initiated on the channel. */
 @property NSDate *timedDestroyDate;
 
 /** Reference count of the proxy to the channel. */
-@property NSUInteger refcount;
+@property NSUInteger refCount;
 
 @end
 
 @implementation GRPCChannelRecord
 
-- (id)copyWithZone:(NSZone *)zone {
-  GRPCChannelRecord *newRecord = [[GRPCChannelRecord allocWithZone:zone] init];
-  newRecord.channel = _channel;
-  newRecord.proxy = _proxy;
-  newRecord.timedDestroyDate = _timedDestroyDate;
-  newRecord.refcount = _refcount;
+@end
 
-  return newRecord;
-}
+@interface GRPCChannelPool ()
+
+- (instancetype)initInstanceWithDestroyDelay:(NSTimeInterval)destroyDelay NS_DESIGNATED_INITIALIZER;
 
 @end
 
 @implementation GRPCChannelPool {
   NSMutableDictionary<GRPCChannelConfiguration *, GRPCChannelRecord *> *_channelPool;
   dispatch_queue_t _dispatchQueue;
+  NSTimeInterval _destroyDelay;
 }
 
 + (instancetype)sharedInstance {
   dispatch_once(&gInitChannelPool, ^{
-    gChannelPool = [[GRPCChannelPool alloc] init];
+    gChannelPool = [[GRPCChannelPool alloc] initInstanceWithDestroyDelay:kDefaultChannelDestroyDelay];
     NSAssert(gChannelPool != nil, @"Cannot initialize global channel pool.");
   });
   return gChannelPool;
 }
 
-- (instancetype)init {
+- (instancetype)initInstanceWithDestroyDelay:(NSTimeInterval)destroyDelay {
   if ((self = [super init])) {
     _channelPool = [NSMutableDictionary dictionary];
 #if __IPHONE_OS_VERSION_MAX_ALLOWED >= 110000 || __MAC_OS_X_VERSION_MAX_ALLOWED >= 101300
@@ -206,7 +203,7 @@ static const NSTimeInterval kDefaultChannelDestroyDelay = 30;
 #endif
       _dispatchQueue = dispatch_queue_create(NULL, DISPATCH_QUEUE_SERIAL);
     }
-    _destroyDelay = kDefaultChannelDestroyDelay;
+    _destroyDelay = destroyDelay;
 
     // Connectivity monitor is not required for CFStream
     char *enableCFStream = getenv(kCFStreamVarName);
@@ -217,56 +214,56 @@ static const NSTimeInterval kDefaultChannelDestroyDelay = 30;
   return self;
 }
 
+- (void)dealloc {
+  [GRPCConnectivityMonitor unregisterObserver:self];
+}
+
 - (GRPCPooledChannel *)channelWithHost:(NSString *)host callOptions:(GRPCCallOptions *)callOptions {
   NSAssert(host.length > 0, @"Host must not be empty.");
   NSAssert(callOptions != nil, @"callOptions must not be empty.");
-  if (host.length == 0) return nil;
-  if (callOptions == nil) return nil;
+  if (host.length == 0 || callOptions == nil) {
+    return nil;
+  }
 
-  GRPCPooledChannel *channelProxy = nil;
+  GRPCPooledChannel *pooledChannel = nil;
   GRPCChannelConfiguration *configuration =
       [[GRPCChannelConfiguration alloc] initWithHost:host callOptions:callOptions];
   @synchronized(self) {
     GRPCChannelRecord *record = _channelPool[configuration];
     if (record == nil) {
       record = [[GRPCChannelRecord alloc] init];
-      record.proxy =
+      record.pooledChannel =
           [[GRPCPooledChannel alloc] initWithChannelConfiguration:configuration channelPool:self];
-      record.timedDestroyDate = nil;
       _channelPool[configuration] = record;
-      channelProxy = record.proxy;
+      pooledChannel = record.pooledChannel;
     } else {
-      channelProxy = record.proxy;
+      pooledChannel = record.pooledChannel;
     }
   }
-  return channelProxy;
-}
-
-- (void)closeOpenConnections {
-  [self disconnectAllChannels];
+  return pooledChannel;
 }
 
 - (GRPCChannel *)refChannelWithConfiguration:(GRPCChannelConfiguration *)configuration {
   GRPCChannel *ret = nil;
   @synchronized(self) {
     NSAssert(configuration != nil, @"configuration cannot be empty.");
-    if (configuration == nil) return nil;
+    if (configuration == nil) {
+      return nil;
+    }
 
     GRPCChannelRecord *record = _channelPool[configuration];
     NSAssert(record != nil, @"No record corresponding to a proxy.");
-    if (record == nil) return nil;
+    if (record == nil) {
+      return nil;
+    }
 
+    record.refCount++;
+    record.timedDestroyDate = nil;
     if (record.channel == nil) {
       // Channel is already destroyed;
       record.channel = [[GRPCChannel alloc] initWithChannelConfiguration:configuration];
-      record.timedDestroyDate = nil;
-      record.refcount = 1;
-      ret = record.channel;
-    } else {
-      ret = record.channel;
-      record.timedDestroyDate = nil;
-      record.refcount++;
     }
+    ret = record.channel;
   }
   return ret;
 }
@@ -275,11 +272,13 @@ static const NSTimeInterval kDefaultChannelDestroyDelay = 30;
   @synchronized(self) {
     GRPCChannelRecord *record = _channelPool[configuration];
     NSAssert(record != nil, @"No record corresponding to a proxy.");
-    if (record == nil) return;
-    NSAssert(record.refcount > 0, @"Inconsistent channel refcount.");
-    if (record.refcount > 0) {
-      record.refcount--;
-      if (record.refcount == 0) {
+    if (record == nil) {
+      return;
+    }
+    NSAssert(record.refCount > 0, @"Inconsistent channel refcount.");
+    if (record.refCount > 0) {
+      record.refCount--;
+      if (record.refCount == 0) {
         NSDate *now = [NSDate date];
         record.timedDestroyDate = now;
         dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(_destroyDelay * NSEC_PER_SEC)),
@@ -288,7 +287,6 @@ static const NSTimeInterval kDefaultChannelDestroyDelay = 30;
                            if (now == record.timedDestroyDate) {
                              // Destroy the raw channel and reset related records.
                              record.timedDestroyDate = nil;
-                             record.refcount = 0;
                              record.channel = nil;
                            }
                          }
@@ -306,8 +304,7 @@ static const NSTimeInterval kDefaultChannelDestroyDelay = 30;
                                             GRPCChannelRecord *_Nonnull obj, BOOL *_Nonnull stop) {
           obj.channel = nil;
           obj.timedDestroyDate = nil;
-          obj.refcount = 0;
-          [proxySet addObject:obj.proxy];
+          [proxySet addObject:obj.pooledChannel];
         }];
   }
   // Disconnect proxies
@@ -320,8 +317,12 @@ static const NSTimeInterval kDefaultChannelDestroyDelay = 30;
   [self disconnectAllChannels];
 }
 
-- (void)dealloc {
-  [GRPCConnectivityMonitor unregisterObserver:self];
+@end
+
+@implementation GRPCChannelPool (Test)
+
+- (instancetype)initTestPoolWithDestroyDelay:(NSTimeInterval)destroyDelay {
+  return [self initInstanceWithDestroyDelay:destroyDelay];
 }
 
 @end