fix: correctly close HTTP-related CRT resources in native sourceset (#1345)

Author: ianbotsf

.github/workflows/artifact-size-metrics.yml

@@ -45,8 +45,10 @@ jobs:
     steps:
       - name: Checkout Sources
         uses: actions/checkout@v4
+        with:
+          path: smithy-kotlin
       - name: Setup build
-        uses: .github/actions/setup-build
+        uses: ./smithy-kotlin/.github/actions/setup-build
       - name: Configure JDK
         uses: actions/setup-java@v3
         with:
@@ -60,16 +62,21 @@ jobs:
           aws-region: us-west-2
       - name: Configure Gradle
         uses: awslabs/aws-kotlin-repo-tools/.github/actions/configure-gradle@main
+        with:
+          working-directory: smithy-kotlin
       - name: Generate Artifact Size Metrics
         run: ./gradlew -Paws.kotlin.native=false artifactSizeMetrics
+        working-directory: smithy-kotlin
       - name: Analyze Artifact Size Metrics
         run: ./gradlew analyzeArtifactSizeMetrics
-
+        working-directory: smithy-kotlin
       - name: Show Results
         uses: awslabs/aws-kotlin-repo-tools/.github/actions/artifact-size-metrics/show-results@main
-
+        with:
+          working-directory: smithy-kotlin
       - name: Evaluate
         if: ${{ !contains(github.event.pull_request.labels.*.name, 'acknowledge-artifact-size-increase') }}
+        working-directory: smithy-kotlin
         run: |
           cd build/reports/metrics
           cat has-significant-change.txt | grep false || {

runtime/crt-util/jvmAndNative/src/aws/smithy/kotlin/runtime/crt/SdkDefaultIO.kt

@@ -14,33 +14,39 @@ private const val DEFAULT_EVENT_LOOP_THREAD_COUNT: Int = 1
 /**
  * Default (CRT) IO used by the SDK when not configured manually/directly
  */
+@Deprecated("This API is no longer used by the SDK and will be removed in version 1.6.x")
 @InternalApi
 public object SdkDefaultIO {
     /**
      * The default event loop group to run IO on
      */
+    @Deprecated("This API is no longer used by the SDK and will be removed in version 1.6.x")
     public val EventLoop: EventLoopGroup by lazy {
-        // TODO - can we register shutdown in appropriate runtimes (e.g. jvm: addShutdown, native: atexit(), etc) when/if these lazy block(s) run?
         EventLoopGroup(DEFAULT_EVENT_LOOP_THREAD_COUNT)
     }
 
     /**
      * The default host resolver to resolve DNS queries with
      */
+    @Deprecated("This API is no longer used by the SDK and will be removed in version 1.6.x")
     public val HostResolver: HostResolver by lazy {
+        @Suppress("DEPRECATION")
         HostResolver(EventLoop)
     }
 
     /**
      * The default client bootstrap
      */
+    @Deprecated("This API is no longer used by the SDK and will be removed in version 1.6.x")
     public val ClientBootstrap: ClientBootstrap by lazy {
+        @Suppress("DEPRECATION")
         ClientBootstrap(EventLoop, HostResolver)
     }
 
     /**
      * The default TLS context
      */
+    @Deprecated("This API is no longer used by the SDK and will be removed in version 1.6.x")
     public val TlsContext: TlsContext by lazy {
         TlsContext(TlsContextOptions.defaultClient())
     }

runtime/protocol/http-client-engines/http-client-engine-crt/jvmAndNative/src/aws/smithy/kotlin/runtime/http/engine/crt/ConnectionManager.kt

@@ -5,11 +5,11 @@
 package aws.smithy.kotlin.runtime.http.engine.crt
 
 import aws.sdk.kotlin.crt.http.*
+import aws.sdk.kotlin.crt.io.ClientBootstrap
 import aws.sdk.kotlin.crt.io.SocketOptions
 import aws.sdk.kotlin.crt.io.TlsContext
 import aws.sdk.kotlin.crt.io.TlsContextOptionsBuilder
 import aws.sdk.kotlin.crt.io.Uri
-import aws.smithy.kotlin.runtime.crt.SdkDefaultIO
 import aws.smithy.kotlin.runtime.http.HttpErrorCode
 import aws.smithy.kotlin.runtime.http.HttpException
 import aws.smithy.kotlin.runtime.http.engine.ProxyConfig
@@ -39,8 +39,11 @@ internal class ConnectionManager(
         .build()
         .let(::CrtTlsContext)
 
+    private val manageClientBootstrap = config.clientBootstrap == null
+    private val clientBootstrap = config.clientBootstrap ?: ClientBootstrap()
+
     private val options = HttpClientConnectionManagerOptionsBuilder().apply {
-        clientBootstrap = config.clientBootstrap ?: SdkDefaultIO.ClientBootstrap
+        clientBootstrap = this@ConnectionManager.clientBootstrap
         tlsContext = crtTlsContext
         manualWindowManagement = true
         socketOptions = SocketOptions(
@@ -55,7 +58,7 @@ internal class ConnectionManager(
     private val connManagers = mutableMapOf<String, HttpClientConnectionManager>()
     private val mutex = Mutex()
 
-    public suspend fun acquire(request: HttpRequest): HttpClientConnection {
+    suspend fun acquire(request: HttpRequest): HttpClientConnection {
         val proxyConfig = config.proxySelector.select(request.url)
 
         val manager = getManagerForUri(request.uri, proxyConfig)
@@ -88,6 +91,7 @@ internal class ConnectionManager(
             throw httpEx
         }
     }
+
     private suspend fun getManagerForUri(uri: Uri, proxyConfig: ProxyConfig): HttpClientConnectionManager = mutex.withLock {
         connManagers.getOrPut(uri.authority) {
             val connOpts = options.apply {
@@ -109,9 +113,12 @@ internal class ConnectionManager(
             HttpClientConnectionManager(connOpts)
         }
     }
+
     override fun close() {
         connManagers.forEach { entry -> entry.value.close() }
         crtTlsContext.close()
+
+        if (manageClientBootstrap) clientBootstrap.close()
     }
 
     private inner class LeasedConnection(private val delegate: HttpClientConnection) : HttpClientConnection by delegate {
@@ -127,7 +134,7 @@ internal class ConnectionManager(
 }
 
 private fun toCrtTlsVersion(sdkTlsVersion: SdkTlsVersion?): CrtTlsVersion = when (sdkTlsVersion) {
-    null -> aws.sdk.kotlin.crt.io.TlsVersion.SYS_DEFAULT
+    null -> CrtTlsVersion.SYS_DEFAULT
     TlsVersion.TLS_1_0 -> CrtTlsVersion.TLSv1
     TlsVersion.TLS_1_1 -> CrtTlsVersion.TLS_V1_1
     TlsVersion.TLS_1_2 -> CrtTlsVersion.TLS_V1_2

runtime/protocol/http-client-engines/http-client-engine-crt/jvmAndNative/src/aws/smithy/kotlin/runtime/http/engine/crt/CrtHttpEngine.kt

@@ -15,9 +15,10 @@ import aws.smithy.kotlin.runtime.io.internal.SdkDispatchers
 import aws.smithy.kotlin.runtime.operation.ExecutionContext
 import aws.smithy.kotlin.runtime.telemetry.logging.logger
 import aws.smithy.kotlin.runtime.time.Instant
-import kotlinx.coroutines.*
+import kotlinx.coroutines.job
 import kotlinx.coroutines.sync.Semaphore
 import kotlinx.coroutines.sync.withPermit
+import kotlinx.coroutines.withContext
 
 internal const val DEFAULT_WINDOW_SIZE_BYTES: Int = 16 * 1024
 internal const val CHUNK_BUFFER_SIZE: Long = 64 * 1024

runtime/protocol/http-client-engines/http-client-engine-default/native/src/aws/smithy/kotlin/runtime/http/engine/DefaultHttpEngineNative.kt

@@ -7,4 +7,9 @@ package aws.smithy.kotlin.runtime.http.engine
 
 import aws.smithy.kotlin.runtime.http.engine.crt.CrtHttpEngine
 
-internal actual fun newDefaultHttpEngine(block: (HttpClientEngineConfig.Builder.() -> Unit)?): CloseableHttpClientEngine = CrtHttpEngine()
+internal actual fun newDefaultHttpEngine(
+    block: (HttpClientEngineConfig.Builder.() -> Unit)?,
+): CloseableHttpClientEngine = when (block) {
+    null -> CrtHttpEngine()
+    else -> CrtHttpEngine(block)
+}