Skip to content

Commit ac67b22

Browse files
committed
Fix thread leak in FileDownloadDelegate
1 parent 5e3e58d commit ac67b22

File tree

1 file changed

+61
-2
lines changed

1 file changed

+61
-2
lines changed

Sources/AsyncHTTPClient/FileDownloadDelegate.swift

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate {
3333
private let io: NonBlockingFileIO
3434
private let reportHead: ((HTTPResponseHead) -> Void)?
3535
private let reportProgress: ((Progress) -> Void)?
36+
37+
private enum ThreadPool {
38+
case unowned
39+
// if we own the thread pool we also need to shut it down
40+
case owned(NIOThreadPool)
41+
}
42+
private let threadPool: ThreadPool
3643

3744
private var fileHandleFuture: EventLoopFuture<NIOFileHandle>?
3845
private var writeFuture: EventLoopFuture<Void>?
@@ -47,19 +54,55 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate {
4754
/// the total byte count and download byte count passed to it as arguments. The callbacks
4855
/// will be invoked in the same threading context that the delegate itself is invoked,
4956
/// as controlled by `EventLoopPreference`.
50-
public init(
57+
public convenience init(
58+
path: String,
59+
pool: NIOThreadPool,
60+
reportHead: ((HTTPResponseHead) -> Void)? = nil,
61+
reportProgress: ((Progress) -> Void)? = nil
62+
) throws {
63+
try self.init(path: path, sharedThreadPool: pool, reportHead: reportHead, reportProgress: reportProgress)
64+
}
65+
66+
/// Initializes a new file download delegate and spawns a new thread for file I/O.
67+
///
68+
/// - parameters:
69+
/// - path: Path to a file you'd like to write the download to.
70+
/// - reportHead: A closure called when the response head is available.
71+
/// - reportProgress: A closure called when a body chunk has been downloaded, with
72+
/// the total byte count and download byte count passed to it as arguments. The callbacks
73+
/// will be invoked in the same threading context that the delegate itself is invoked,
74+
/// as controlled by `EventLoopPreference`.
75+
public convenience init(
76+
path: String,
77+
reportHead: ((HTTPResponseHead) -> Void)? = nil,
78+
reportProgress: ((Progress) -> Void)? = nil
79+
) throws {
80+
try self.init(path: path, sharedThreadPool: nil, reportHead: reportHead, reportProgress: reportProgress)
81+
}
82+
83+
private init(
5184
path: String,
52-
pool: NIOThreadPool = NIOThreadPool(numberOfThreads: 1),
85+
sharedThreadPool: NIOThreadPool?,
5386
reportHead: ((HTTPResponseHead) -> Void)? = nil,
5487
reportProgress: ((Progress) -> Void)? = nil
5588
) throws {
89+
let pool: NIOThreadPool
90+
if let sharedThreadPool = sharedThreadPool {
91+
pool = sharedThreadPool
92+
self.threadPool = .unowned
93+
} else {
94+
pool = NIOThreadPool(numberOfThreads: 1)
95+
self.threadPool = .owned(pool)
96+
}
97+
5698
pool.start()
5799
self.io = NonBlockingFileIO(threadPool: pool)
58100
self.filePath = path
59101

60102
self.reportHead = reportHead
61103
self.reportProgress = reportProgress
62104
}
105+
63106

64107
public func didReceiveHead(
65108
task: HTTPClient.Task<Response>,
@@ -107,6 +150,12 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate {
107150
private func close(fileHandle: NIOFileHandle) {
108151
try! fileHandle.close()
109152
self.fileHandleFuture = nil
153+
switch threadPool {
154+
case .unowned:
155+
break
156+
case .owned(let pool):
157+
try! pool.syncShutdownGracefully()
158+
}
110159
}
111160

112161
private func finalize() {
@@ -128,4 +177,14 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate {
128177
self.finalize()
129178
return self.progress
130179
}
180+
181+
deinit {
182+
switch threadPool {
183+
case .unowned:
184+
break
185+
case .owned(let pool):
186+
// if the delegate is unused we still need to shutdown the thread pool
187+
try! pool.syncShutdownGracefully()
188+
}
189+
}
131190
}

0 commit comments

Comments
 (0)