From 0447b0359ef5f35ccd277892d9fe374e8c16b132 Mon Sep 17 00:00:00 2001
From: George Barnett <gbarnett@apple.com>
Date: Tue, 1 Apr 2025 15:29:54 +0100
Subject: [PATCH] Strict concurrency for NIOHTTPCompression (#257)

---
 Package.swift                                 |  6 +-
 .../NIOHTTPCompression/HTTPCompression.swift  |  2 +-
 .../HTTPDecompression.swift                   |  2 +-
 .../HTTPResponseCompressorTest.swift          | 68 +++++++++++--------
 4 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/Package.swift b/Package.swift
index 65729c4..0a17d0b 100644
--- a/Package.swift
+++ b/Package.swift
@@ -50,7 +50,8 @@ var targets: [PackageDescription.Target] = [
             .product(name: "NIO", package: "swift-nio"),
             .product(name: "NIOCore", package: "swift-nio"),
             .product(name: "NIOHTTP1", package: "swift-nio"),
-        ]
+        ],
+        swiftSettings: strictConcurrencySettings
     ),
     .executableTarget(
         name: "HTTPServerWithQuiescingDemo",
@@ -132,7 +133,8 @@ var targets: [PackageDescription.Target] = [
             .product(name: "NIOEmbedded", package: "swift-nio"),
             .product(name: "NIOHTTP1", package: "swift-nio"),
             .product(name: "NIOConcurrencyHelpers", package: "swift-nio"),
-        ]
+        ],
+        swiftSettings: strictConcurrencySettings
     ),
     .testTarget(
         name: "NIOSOCKSTests",
diff --git a/Sources/NIOHTTPCompression/HTTPCompression.swift b/Sources/NIOHTTPCompression/HTTPCompression.swift
index 69ad36b..f696871 100644
--- a/Sources/NIOHTTPCompression/HTTPCompression.swift
+++ b/Sources/NIOHTTPCompression/HTTPCompression.swift
@@ -16,7 +16,7 @@ import CNIOExtrasZlib
 import NIOCore
 
 /// Namespace for compression code.
-public enum NIOCompression {
+public enum NIOCompression: Sendable {
 
     /// Which algorithm should be used for compression.
     public struct Algorithm: CustomStringConvertible, Equatable, Sendable {
diff --git a/Sources/NIOHTTPCompression/HTTPDecompression.swift b/Sources/NIOHTTPCompression/HTTPDecompression.swift
index 4ff0d44..3acc1c5 100644
--- a/Sources/NIOHTTPCompression/HTTPDecompression.swift
+++ b/Sources/NIOHTTPCompression/HTTPDecompression.swift
@@ -16,7 +16,7 @@ import CNIOExtrasZlib
 import NIOCore
 
 /// Namespace for decompression code.
-public enum NIOHTTPDecompression {
+public enum NIOHTTPDecompression: Sendable {
     /// Specifies how to limit decompression inflation.
     public struct DecompressionLimit: Sendable {
         private enum Limit {
diff --git a/Tests/NIOHTTPCompressionTests/HTTPResponseCompressorTest.swift b/Tests/NIOHTTPCompressionTests/HTTPResponseCompressorTest.swift
index 75f6713..0b6d0b0 100644
--- a/Tests/NIOHTTPCompressionTests/HTTPResponseCompressorTest.swift
+++ b/Tests/NIOHTTPCompressionTests/HTTPResponseCompressorTest.swift
@@ -40,7 +40,7 @@ private class PromiseOrderer {
         let thisPromiseIndex = promiseArray.count
         promiseArray.append(promise)
 
-        promise.futureResult.whenComplete { (_: Result<Void, Error>) in
+        promise.futureResult.hop(to: self.eventLoop).assumeIsolated().whenComplete { (_: Result<Void, Error>) in
             let priorFutures = self.promiseArray[0..<thisPromiseIndex]
             let subsequentFutures = self.promiseArray[(thisPromiseIndex + 1)...]
             let allPriorFuturesFired = priorFutures.map { $0.futureResult.isFulfilled }.allSatisfy { $0 }
@@ -772,9 +772,11 @@ class HTTPResponseCompressorTest: XCTestCase {
     }
 
     func testConditionalCompressionEnabled() throws {
-        let predicateWasCalled = expectation(description: "Predicate was called")
+        let loop = EmbeddedEventLoop()
+        defer { try! loop.syncShutdownGracefully() }
+        let predicateWasCalled = loop.makePromise(of: Void.self)
         let compressor = HTTPResponseCompressor { responseHeaders, isCompressionSupported in
-            defer { predicateWasCalled.fulfill() }
+            defer { predicateWasCalled.succeed() }
             XCTAssertEqual(responseHeaders.headers, ["Content-Type": "json"])
             XCTAssertEqual(isCompressionSupported, true)
             return .compressIfPossible
@@ -796,13 +798,15 @@ class HTTPResponseCompressorTest: XCTestCase {
             ]
         )
 
-        waitForExpectations(timeout: 0)
+        try predicateWasCalled.futureResult.wait()
     }
 
     func testUnsupportedRequestConditionalCompressionEnabled() throws {
-        let predicateWasCalled = expectation(description: "Predicate was called")
+        let loop = EmbeddedEventLoop()
+        defer { try! loop.syncShutdownGracefully() }
+        let predicateWasCalled = loop.makePromise(of: Void.self)
         let compressor = HTTPResponseCompressor { responseHeaders, isCompressionSupported in
-            defer { predicateWasCalled.fulfill() }
+            defer { predicateWasCalled.succeed() }
             XCTAssertEqual(responseHeaders.headers, ["Content-Type": "json"])
             XCTAssertEqual(isCompressionSupported, false)
             return .compressIfPossible
@@ -823,13 +827,15 @@ class HTTPResponseCompressorTest: XCTestCase {
             ]
         )
 
-        waitForExpectations(timeout: 0)
+        try predicateWasCalled.futureResult.wait()
     }
 
     func testUnsupportedStatusConditionalCompressionEnabled() throws {
-        let predicateWasCalled = expectation(description: "Predicate was called")
+        let loop = EmbeddedEventLoop()
+        defer { try! loop.syncShutdownGracefully() }
+        let predicateWasCalled = loop.makePromise(of: Void.self)
         let compressor = HTTPResponseCompressor { responseHeaders, isCompressionSupported in
-            defer { predicateWasCalled.fulfill() }
+            defer { predicateWasCalled.succeed() }
             XCTAssertEqual(responseHeaders.status, .notModified)
             XCTAssertEqual(responseHeaders.headers, ["Content-Type": "json"])
             XCTAssertEqual(isCompressionSupported, false)
@@ -862,13 +868,15 @@ class HTTPResponseCompressorTest: XCTestCase {
             }
         }
 
-        waitForExpectations(timeout: 0)
+        try predicateWasCalled.futureResult.wait()
     }
 
     func testConditionalCompressionDisabled() throws {
-        let predicateWasCalled = expectation(description: "Predicate was called")
+        let loop = EmbeddedEventLoop()
+        defer { try! loop.syncShutdownGracefully() }
+        let predicateWasCalled = loop.makePromise(of: Void.self)
         let compressor = HTTPResponseCompressor { responseHeaders, isCompressionSupported in
-            defer { predicateWasCalled.fulfill() }
+            defer { predicateWasCalled.succeed() }
             XCTAssertEqual(responseHeaders.headers, ["Content-Type": "json"])
             XCTAssertEqual(isCompressionSupported, true)
             return .doNotCompress
@@ -889,13 +897,15 @@ class HTTPResponseCompressorTest: XCTestCase {
             ]
         )
 
-        waitForExpectations(timeout: 0)
+        try predicateWasCalled.futureResult.wait()
     }
 
     func testUnsupportedRequestConditionalCompressionDisabled() throws {
-        let predicateWasCalled = expectation(description: "Predicate was called")
+        let loop = EmbeddedEventLoop()
+        defer { try! loop.syncShutdownGracefully() }
+        let predicateWasCalled = loop.makePromise(of: Void.self)
         let compressor = HTTPResponseCompressor { responseHeaders, isCompressionSupported in
-            defer { predicateWasCalled.fulfill() }
+            defer { predicateWasCalled.succeed() }
             XCTAssertEqual(responseHeaders.headers, ["Content-Type": "json"])
             XCTAssertEqual(isCompressionSupported, false)
             return .doNotCompress
@@ -916,13 +926,15 @@ class HTTPResponseCompressorTest: XCTestCase {
             ]
         )
 
-        waitForExpectations(timeout: 0)
+        try predicateWasCalled.futureResult.wait()
     }
 
     func testUnsupportedStatusConditionalCompressionDisabled() throws {
-        let predicateWasCalled = expectation(description: "Predicate was called")
+        let loop = EmbeddedEventLoop()
+        defer { try! loop.syncShutdownGracefully() }
+        let predicateWasCalled = loop.makePromise(of: Void.self)
         let compressor = HTTPResponseCompressor { responseHeaders, isCompressionSupported in
-            defer { predicateWasCalled.fulfill() }
+            defer { predicateWasCalled.succeed() }
             XCTAssertEqual(responseHeaders.status, .notModified)
             XCTAssertEqual(responseHeaders.headers, ["Content-Type": "json"])
             XCTAssertEqual(isCompressionSupported, false)
@@ -955,14 +967,13 @@ class HTTPResponseCompressorTest: XCTestCase {
             }
         }
 
-        waitForExpectations(timeout: 0)
+        try predicateWasCalled.futureResult.wait()
     }
 
     func testConditionalCompressionModifiedHeaders() throws {
-        let predicateWasCalled = expectation(description: "Predicate was called")
-        predicateWasCalled.expectedFulfillmentCount = 2
+        let counter = NIOLockedValueBox(0)
         let compressor = HTTPResponseCompressor { responseHeaders, isCompressionSupported in
-            defer { predicateWasCalled.fulfill() }
+            defer { counter.withLockedValue { $0 += 1 } }
             let isEnabled = responseHeaders.headers[canonicalForm: "x-compression"].first == "enable"
             XCTAssertEqual(
                 responseHeaders.headers,
@@ -999,7 +1010,7 @@ class HTTPResponseCompressorTest: XCTestCase {
             ]
         )
 
-        waitForExpectations(timeout: 0)
+        XCTAssertEqual(counter.withLockedValue { $0 }, 2)
     }
 }
 
@@ -1009,25 +1020,22 @@ extension EventLoopFuture {
             // Easy, we're on the EventLoop. Let's just use our knowledge that we run completed future callbacks
             // immediately.
             var fulfilled = false
-            self.whenComplete { _ in
+            self.assumeIsolated().whenComplete { _ in
                 fulfilled = true
             }
             return fulfilled
         } else {
-            let lock = NIOLock()
             let group = DispatchGroup()
-            var fulfilled = false  // protected by lock
+            let fulfilled = NIOLockedValueBox(false)
 
             group.enter()
             self.eventLoop.execute {
                 let isFulfilled = self.isFulfilled  // This will now enter the above branch.
-                lock.withLock {
-                    fulfilled = isFulfilled
-                }
+                fulfilled.withLockedValue { $0 = isFulfilled }
                 group.leave()
             }
             group.wait()  // this is very nasty but this is for tests only, so...
-            return lock.withLock { fulfilled }
+            return fulfilled.withLockedValue { $0 }
         }
     }
 }