Skip to content

Commit 864c4bc

Browse files
committed
Refactor RedisClient state to use enum instead of booleans
Motivation: When later adding support for pub/sub, and potentially batching commands, a state is going to need to be defined and multiple booleans will create a large matrix that will be error prone. Results: Rather than using `Atomic<Bool>`, a `Lock` and `ConnectionState` enum are used to determine if a connection is open.
1 parent ed106a9 commit 864c4bc

File tree

1 file changed

+20
-8
lines changed

1 file changed

+20
-8
lines changed

Sources/NIORedis/RedisClient.swift

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,28 @@ private let loggingKeyID = "RedisConnection"
4040
///
4141
/// See `RedisClient`
4242
public final class RedisConnection: RedisClient {
43+
private enum ConnectionState {
44+
case open
45+
case closed
46+
}
47+
4348
/// See `RedisClient.eventLoop`
4449
public var eventLoop: EventLoop { return channel.eventLoop }
4550
/// Is the client still connected to Redis?
46-
public var isConnected: Bool { return !sentQuitCommand.load() }
51+
public var isConnected: Bool { return state != .closed }
4752

4853
private let channel: Channel
4954
private var logger: Logger
50-
private var sentQuitCommand = Atomic<Bool>(value: false)
55+
56+
private let _stateLock = Lock()
57+
private var _state: ConnectionState
58+
private var state: ConnectionState {
59+
get { return _stateLock.withLock { self._state } }
60+
set(newValue) { _stateLock.withLockVoid { self._state = newValue } }
61+
}
5162

5263
deinit {
53-
if !sentQuitCommand.load() {
64+
if isConnected {
5465
assertionFailure("close() was not called before deinit!")
5566
logger.warning("RedisConnection did not properly shutdown before deinit!")
5667
}
@@ -68,6 +79,7 @@ public final class RedisConnection: RedisClient {
6879

6980
self.logger[metadataKey: loggingKeyID] = "\(UUID())"
7081
self.logger.debug("Connection created.")
82+
self._state = .open
7183
}
7284

7385
/// Sends a `QUIT` command, then closes the `Channel` this instance was initialized with.
@@ -76,10 +88,6 @@ public final class RedisConnection: RedisClient {
7688
/// - Returns: An `EventLoopFuture` that resolves when the connection has been closed.
7789
@discardableResult
7890
public func close() -> EventLoopFuture<Void> {
79-
// this needs to be true in order to prevent multiple close() chains, and to stop
80-
// allowing commands to be sent - but we don't want to set it before we send the QUIT command
81-
defer { sentQuitCommand.store(true) }
82-
8391
guard isConnected else {
8492
logger.notice("Connection received more than one close() request.")
8593
return channel.eventLoop.makeSucceededFuture(())
@@ -94,9 +102,13 @@ public final class RedisConnection: RedisClient {
94102
.map { self.logger.debug("Connection closed.") }
95103
.recover {
96104
self.logger.error("Encountered error during close(): \($0)")
97-
self.sentQuitCommand.store(false)
105+
self.state = .open
98106
}
99107

108+
// setting it to closed now prevents multiple close() chains, but doesn't stop the QUIT command
109+
// if the connection wasn't closed, it's reset in the callback chain
110+
state = .closed
111+
100112
return result
101113
}
102114

0 commit comments

Comments
 (0)