From b17bf8b8b3564d5f9835f7a0a4c6cc6e58203bca Mon Sep 17 00:00:00 2001 From: Michael Rebello Date: Mon, 25 Feb 2019 16:15:06 -0800 Subject: [PATCH] Add ability to manually shut down channels Addresses https://github.com/grpc/grpc-swift/issues/198. It's currently not possible to manually shut down a gRPC channel, which means that the only way to shut down a channel is to deallocate it. This requirement is a bit fragile for cases where a consumer wants to manually shut down a channel, since it requires confidence that nothing else is holding a strong reference to the channel. This PR: - Adds a `shutdown` function to `Channel`, allowing consumers to arbitrarily shut down connections - Cancels all existing calls that are using the channel upon shutdown - Validates that existing calls will throw errors when attempting to read/write from a previously shut down channel - Ensures creating new calls using previously shut down channels will result in the initializer throwing (already handled by code generators) This change is increasingly relevant because consumers will need to shut down channels and restart them to help mitigate issues when switching between wifi/cellular as mentioned here: https://github.com/grpc/grpc-swift/issues/337. --- Examples/SimpleXcode/Simple/Document.swift | 17 ++-- Sources/Examples/Simple/main.swift | 2 +- Sources/SwiftGRPC/Core/Channel.swift | 46 +++++++-- Sources/SwiftGRPC/Runtime/ClientCall.swift | 19 ++-- Tests/LinuxMain.swift | 1 + .../SwiftGRPCTests/ChannelShutdownTests.swift | 97 +++++++++++++++++++ Tests/SwiftGRPCTests/GRPCTests.swift | 6 +- 7 files changed, 159 insertions(+), 29 deletions(-) create mode 100644 Tests/SwiftGRPCTests/ChannelShutdownTests.swift diff --git a/Examples/SimpleXcode/Simple/Document.swift b/Examples/SimpleXcode/Simple/Document.swift index 7bf8de8bf..aeded6892 100644 --- a/Examples/SimpleXcode/Simple/Document.swift +++ b/Examples/SimpleXcode/Simple/Document.swift @@ -144,16 +144,17 @@ class Document: NSDocument { if !self.isRunning() { break } - let method = (i < steps) ? "/hello" : "/quit" - let call = self.channel.makeCall(method) - - let metadata = try! Metadata([ - "x": "xylophone", - "y": "yu", - "z": "zither" - ]) do { + let method = (i < steps) ? "/hello" : "/quit" + let call = try self.channel.makeCall(method) + + let metadata = try Metadata([ + "x": "xylophone", + "y": "yu", + "z": "zither" + ]) + try call.start(.unary, metadata: metadata, message: messageData) { callResult in diff --git a/Sources/Examples/Simple/main.swift b/Sources/Examples/Simple/main.swift index 2840f1729..2d349a6d7 100644 --- a/Sources/Examples/Simple/main.swift +++ b/Sources/Examples/Simple/main.swift @@ -30,7 +30,7 @@ func client() throws { let method = (i < steps - 1) ? "/hello" : "/quit" print("calling " + method) - let call = c.makeCall(method) + let call = try! c.makeCall(method) let metadata = try Metadata([ "x": "xylophone", diff --git a/Sources/SwiftGRPC/Core/Channel.swift b/Sources/SwiftGRPC/Core/Channel.swift index 4fffdf704..ce4923f75 100644 --- a/Sources/SwiftGRPC/Core/Channel.swift +++ b/Sources/SwiftGRPC/Core/Channel.swift @@ -27,6 +27,10 @@ public class Channel { private let completionQueue: CompletionQueue /// Observer for connectivity state changes. Created lazily if needed private var connectivityObserver: ConnectivityObserver? + /// Weak references to API calls using this channel that are in-flight + private var activeCalls = NSHashTable.weakObjects() + /// Whether the gRPC channel has been shut down + private var hasBeenShutdown = false /// Timeout for new calls public var timeout: TimeInterval = 600.0 @@ -34,6 +38,14 @@ public class Channel { /// Default host to use for new calls public var host: String + /// Errors that may be thrown by the channel. + enum Error: Swift.Error { + /// Action cannot be performed because the channel has already been shut down. + case alreadyShutdown + /// Failed to create a new call within the gRPC stack. + case callCreationFailed + } + /// Initializes a gRPC channel /// /// - Parameter address: the address of the server to be called @@ -94,12 +106,21 @@ public class Channel { completionQueue.run() // start a loop that watches the channel's completion queue } - deinit { + /// Shut down the channel. No new calls may be made using this channel after it is shut down + public func shutdown() { self.mutex.synchronize { + guard !self.hasBeenShutdown else { return } + + self.hasBeenShutdown = true self.connectivityObserver?.shutdown() + cgrpc_channel_destroy(self.underlyingChannel) + self.completionQueue.shutdown() + self.activeCalls.allObjects.forEach { $0.cancel() } } - cgrpc_channel_destroy(self.underlyingChannel) - self.completionQueue.shutdown() + } + + deinit { + self.shutdown() } /// Constructs a Call object to make a gRPC API call @@ -108,11 +129,20 @@ public class Channel { /// - Parameter host: the gRPC host name for the call. If unspecified, defaults to the Client host /// - Parameter timeout: a timeout value in seconds /// - Returns: a Call object that can be used to perform the request - public func makeCall(_ method: String, host: String = "", timeout: TimeInterval? = nil) -> Call { - let host = host.isEmpty ? self.host : host - let timeout = timeout ?? self.timeout - let underlyingCall = cgrpc_channel_create_call(underlyingChannel, method, host, timeout)! - return Call(underlyingCall: underlyingCall, owned: true, completionQueue: completionQueue) + public func makeCall(_ method: String, host: String? = nil, timeout: TimeInterval? = nil) throws -> Call { + guard (self.mutex.synchronize { !self.hasBeenShutdown }) else { + throw Error.alreadyShutdown + } + + guard let underlyingCall = cgrpc_channel_create_call( + self.underlyingChannel, method, host ?? self.host, timeout ?? self.timeout) else + { + throw Error.callCreationFailed + } + + let call = Call(underlyingCall: underlyingCall, owned: true, completionQueue: self.completionQueue) + self.mutex.synchronize { self.activeCalls.add(call) } + return call } /// Check the current connectivity state diff --git a/Sources/SwiftGRPC/Runtime/ClientCall.swift b/Sources/SwiftGRPC/Runtime/ClientCall.swift index 8473c92a1..48f4725d2 100644 --- a/Sources/SwiftGRPC/Runtime/ClientCall.swift +++ b/Sources/SwiftGRPC/Runtime/ClientCall.swift @@ -13,27 +13,28 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -import Dispatch -import Foundation import SwiftProtobuf public protocol ClientCall: class { static var method: String { get } - + /// Cancel the call. func cancel() } -open class ClientCallBase: ClientCall { +open class ClientCallBase { open class var method: String { fatalError("needs to be overridden") } public let call: Call /// Create a call. - public init(_ channel: Channel) { - call = channel.makeCall(type(of: self).method) + public init(_ channel: Channel) throws { + self.call = try channel.makeCall(type(of: self).method) + } +} + +extension ClientCallBase: ClientCall { + public func cancel() { + self.call.cancel() } - - public func cancel() { call.cancel() } } diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 91d0a9807..0cc7c5c78 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -22,6 +22,7 @@ XCTMain([ testCase(gRPCTests.allTests), testCase(ChannelArgumentTests.allTests), testCase(ChannelConnectivityTests.allTests), + testCase(ChannelShutdownTests.allTests), testCase(ClientCancellingTests.allTests), testCase(ClientTestExample.allTests), testCase(ClientTimeoutTests.allTests), diff --git a/Tests/SwiftGRPCTests/ChannelShutdownTests.swift b/Tests/SwiftGRPCTests/ChannelShutdownTests.swift new file mode 100644 index 000000000..92dee642a --- /dev/null +++ b/Tests/SwiftGRPCTests/ChannelShutdownTests.swift @@ -0,0 +1,97 @@ +/* + * Copyright 2018, gRPC Authors All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +@testable import SwiftGRPC +import XCTest + +final class ChannelShutdownTests: BasicEchoTestCase { + static var allTests: [(String, (ChannelShutdownTests) -> () throws -> Void)] { + return [ + ("testThrowsWhenCreatingCallWithAlreadyShutDownChannel", testThrowsWhenCreatingCallWithAlreadyShutDownChannel), + ("testCallReceiveThrowsWhenChannelIsShutDown", testCallReceiveThrowsWhenChannelIsShutDown), + ("testCallCloseThrowsWhenChannelIsShutDown", testCallCloseThrowsWhenChannelIsShutDown), + ("testCallCloseAndReceiveThrowsWhenChannelIsShutDown", testCallCloseAndReceiveThrowsWhenChannelIsShutDown), + ("testCallSendThrowsWhenChannelIsShutDown", testCallSendThrowsWhenChannelIsShutDown), + ("testCancelsActiveCallWhenShutdownIsCalled", testCancelsActiveCallWhenShutdownIsCalled), + ] + } +} + +extension ChannelShutdownTests { + func testThrowsWhenCreatingCallWithAlreadyShutDownChannel() { + self.client.channel.shutdown() + + XCTAssertThrowsError(try self.client.channel.makeCall("foobar")) { error in + XCTAssertEqual(.alreadyShutdown, error as? Channel.Error) + } + } + + func testCallReceiveThrowsWhenChannelIsShutDown() { + let call = try! self.client.channel.makeCall("foo") + self.client.channel.shutdown() + + XCTAssertThrowsError(try call.receiveMessage(completion: { _ in })) { error in + XCTAssertEqual(.completionQueueShutdown, error as? CallError) + XCTAssertNotNil(self.client.channel) + } + } + + func testCallCloseThrowsWhenChannelIsShutDown() { + let call = try! self.client.channel.makeCall("foo") + self.client.channel.shutdown() + + XCTAssertThrowsError(try call.close()) { error in + XCTAssertEqual(.completionQueueShutdown, error as? CallError) + XCTAssertNotNil(self.client.channel) + } + } + + func testCallCloseAndReceiveThrowsWhenChannelIsShutDown() { + let call = try! self.client.channel.makeCall("foo") + self.client.channel.shutdown() + + XCTAssertThrowsError(try call.closeAndReceiveMessage(completion: { _ in })) { error in + XCTAssertEqual(.completionQueueShutdown, error as? CallError) + XCTAssertNotNil(self.client.channel) + } + } + + func testCallSendThrowsWhenChannelIsShutDown() { + let call = try! self.client.channel.makeCall("foo") + self.client.channel.shutdown() + + XCTAssertThrowsError(try call.sendMessage(data: Data())) { error in + XCTAssertEqual(.completionQueueShutdown, error as? CallError) + XCTAssertNotNil(self.client.channel) + } + } + + func testCancelsActiveCallWhenShutdownIsCalled() { + let errorExpectation = self.expectation(description: "error is returned to call when channel is shut down") + let call = try! self.client.channel.makeCall("foo") + + try! call.receiveMessage { result in + XCTAssertFalse(result.success) + errorExpectation.fulfill() + } + + self.client.channel.shutdown() + self.waitForExpectations(timeout: 0.1) + + XCTAssertThrowsError(try call.close()) { error in + XCTAssertEqual(.completionQueueShutdown, error as? CallError) + } + } +} diff --git a/Tests/SwiftGRPCTests/GRPCTests.swift b/Tests/SwiftGRPCTests/GRPCTests.swift index aadd4b8df..546fb37f8 100644 --- a/Tests/SwiftGRPCTests/GRPCTests.swift +++ b/Tests/SwiftGRPCTests/GRPCTests.swift @@ -177,7 +177,7 @@ func callUnary(channel: Channel) throws { func callUnaryIndividual(channel: Channel, message: Data, shouldSucceed: Bool) throws { let sem = DispatchSemaphore(value: 0) let method = hello - let call = channel.makeCall(method) + let call = try channel.makeCall(method) let metadata = try Metadata(initialClientMetadata) try call.start(.unary, metadata: metadata, message: message) { response in @@ -228,7 +228,7 @@ func callServerStream(channel: Channel) throws { let sem = DispatchSemaphore(value: 0) let method = helloServerStream - let call = channel.makeCall(method) + let call = try channel.makeCall(method) try call.start(.serverStreaming, metadata: metadata, message: message) { response in @@ -270,7 +270,7 @@ func callBiDiStream(channel: Channel) throws { let sem = DispatchSemaphore(value: 0) let method = helloBiDiStream - let call = channel.makeCall(method) + let call = try channel.makeCall(method) try call.start(.bidiStreaming, metadata: metadata, message: nil) { response in