Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Jul 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions .github/workflows/artifact-size-metrics.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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 || {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Loading