Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import io.autorender.core.ClientOptions
import io.autorender.core.LogLevel
import io.autorender.core.Sleeper
import io.autorender.core.Timeout
import io.autorender.core.http.AsyncStreamResponse
import io.autorender.core.http.Headers
import io.autorender.core.http.HttpClient
import io.autorender.core.http.ProxyAuthenticator
Expand All @@ -18,6 +19,7 @@ import java.net.Proxy
import java.time.Clock
import java.time.Duration
import java.util.Optional
import java.util.concurrent.Executor
import java.util.concurrent.ExecutorService
import javax.net.ssl.HostnameVerifier
import javax.net.ssl.SSLSocketFactory
Expand Down Expand Up @@ -201,6 +203,17 @@ class AutorenderOkHttpClient private constructor() {
*/
fun jsonMapper(jsonMapper: JsonMapper) = apply { clientOptions.jsonMapper(jsonMapper) }

/**
* The executor to use for running [AsyncStreamResponse.Handler] callbacks.
*
* Defaults to a dedicated cached thread pool.
*
* This class takes ownership of the executor and shuts it down, if possible, when closed.
*/
fun streamHandlerExecutor(streamHandlerExecutor: Executor) = apply {
clientOptions.streamHandlerExecutor(streamHandlerExecutor)
}

/**
* The interface to use for delaying execution, like during retries.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import io.autorender.core.ClientOptions
import io.autorender.core.LogLevel
import io.autorender.core.Sleeper
import io.autorender.core.Timeout
import io.autorender.core.http.AsyncStreamResponse
import io.autorender.core.http.Headers
import io.autorender.core.http.HttpClient
import io.autorender.core.http.ProxyAuthenticator
Expand All @@ -18,6 +19,7 @@ import java.net.Proxy
import java.time.Clock
import java.time.Duration
import java.util.Optional
import java.util.concurrent.Executor
import java.util.concurrent.ExecutorService
import javax.net.ssl.HostnameVerifier
import javax.net.ssl.SSLSocketFactory
Expand Down Expand Up @@ -201,6 +203,17 @@ class AutorenderOkHttpClientAsync private constructor() {
*/
fun jsonMapper(jsonMapper: JsonMapper) = apply { clientOptions.jsonMapper(jsonMapper) }

/**
* The executor to use for running [AsyncStreamResponse.Handler] callbacks.
*
* Defaults to a dedicated cached thread pool.
*
* This class takes ownership of the executor and shuts it down, if possible, when closed.
*/
fun streamHandlerExecutor(streamHandlerExecutor: Executor) = apply {
clientOptions.streamHandlerExecutor(streamHandlerExecutor)
}

/**
* The interface to use for delaying execution, like during retries.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// File generated from our OpenAPI spec by Stainless.

package io.autorender.core

import java.util.stream.Stream
import java.util.stream.StreamSupport

class AutoPager<T> private constructor(private val firstPage: Page<T>) : Iterable<T> {

companion object {

fun <T> from(firstPage: Page<T>): AutoPager<T> = AutoPager(firstPage)
}

override fun iterator(): Iterator<T> =
generateSequence(firstPage) { if (it.hasNextPage()) it.nextPage() else null }
.flatMap { it.items() }
.iterator()

fun stream(): Stream<T> = StreamSupport.stream(spliterator(), false)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// File generated from our OpenAPI spec by Stainless.

package io.autorender.core

import io.autorender.core.http.AsyncStreamResponse
import java.util.Optional
import java.util.concurrent.CompletableFuture
import java.util.concurrent.CompletionException
import java.util.concurrent.Executor
import java.util.concurrent.atomic.AtomicReference

class AutoPagerAsync<T>
private constructor(private val firstPage: PageAsync<T>, private val defaultExecutor: Executor) :
AsyncStreamResponse<T> {

companion object {

fun <T> from(firstPage: PageAsync<T>, defaultExecutor: Executor): AutoPagerAsync<T> =
AutoPagerAsync(firstPage, defaultExecutor)
}

private val onCompleteFuture = CompletableFuture<Void?>()
private val state = AtomicReference(State.NEW)

override fun subscribe(handler: AsyncStreamResponse.Handler<T>): AsyncStreamResponse<T> =
subscribe(handler, defaultExecutor)

override fun subscribe(
handler: AsyncStreamResponse.Handler<T>,
executor: Executor,
): AsyncStreamResponse<T> = apply {
// TODO(JDK): Use `compareAndExchange` once targeting JDK 9.
check(state.compareAndSet(State.NEW, State.SUBSCRIBED)) {
if (state.get() == State.SUBSCRIBED) "Cannot subscribe more than once"
else "Cannot subscribe after the response is closed"
}

fun PageAsync<T>.handle(): CompletableFuture<Void?> {
if (state.get() == State.CLOSED) {
return CompletableFuture.completedFuture(null)
}

items().forEach { handler.onNext(it) }
return if (hasNextPage()) nextPage().thenCompose { it.handle() }
else CompletableFuture.completedFuture(null)
}

executor.execute {
firstPage.handle().whenComplete { _, error ->
val actualError =
if (error is CompletionException && error.cause != null) error.cause else error
try {
handler.onComplete(Optional.ofNullable(actualError))
} finally {
try {
if (actualError == null) {
onCompleteFuture.complete(null)
} else {
onCompleteFuture.completeExceptionally(actualError)
}
} finally {
close()
}
}
}
}
}

override fun onCompleteFuture(): CompletableFuture<Void?> = onCompleteFuture

override fun close() {
val previousState = state.getAndSet(State.CLOSED)
if (previousState == State.CLOSED) {
return
}

// When the stream is closed, we should always consider it closed. If it closed due
// to an error, then we will have already completed the future earlier, and this
// will be a no-op.
onCompleteFuture.complete(null)
}
}

private enum class State {
NEW,
SUBSCRIBED,
CLOSED,
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package io.autorender.core

import com.fasterxml.jackson.databind.json.JsonMapper
import io.autorender.core.http.AsyncStreamResponse
import io.autorender.core.http.Headers
import io.autorender.core.http.HttpClient
import io.autorender.core.http.LoggingHttpClient
Expand All @@ -12,6 +13,11 @@ import io.autorender.core.http.RetryingHttpClient
import java.time.Clock
import java.time.Duration
import java.util.Optional
import java.util.concurrent.Executor
import java.util.concurrent.ExecutorService
import java.util.concurrent.Executors
import java.util.concurrent.ThreadFactory
import java.util.concurrent.atomic.AtomicLong
import kotlin.jvm.optionals.getOrNull

/** A class representing the SDK client configuration. */
Expand Down Expand Up @@ -41,6 +47,14 @@ private constructor(
* needs to be overridden.
*/
@get:JvmName("jsonMapper") val jsonMapper: JsonMapper,
/**
* The executor to use for running [AsyncStreamResponse.Handler] callbacks.
*
* Defaults to a dedicated cached thread pool.
*
* This class takes ownership of the executor and shuts it down, if possible, when closed.
*/
@get:JvmName("streamHandlerExecutor") val streamHandlerExecutor: Executor,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
/**
* The interface to use for delaying execution, like during retries.
*
Expand Down Expand Up @@ -153,6 +167,7 @@ private constructor(
private var httpClient: HttpClient? = null
private var checkJacksonVersionCompatibility: Boolean = true
private var jsonMapper: JsonMapper = jsonMapper()
private var streamHandlerExecutor: Executor? = null
private var sleeper: Sleeper? = null
private var clock: Clock = Clock.systemUTC()
private var baseUrl: String? = null
Expand All @@ -169,6 +184,7 @@ private constructor(
httpClient = clientOptions.originalHttpClient
checkJacksonVersionCompatibility = clientOptions.checkJacksonVersionCompatibility
jsonMapper = clientOptions.jsonMapper
streamHandlerExecutor = clientOptions.streamHandlerExecutor

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline autorender-java-core/src/main/kotlin/io/autorender/core/ClientOptions.kt --items all
rg -n -C 5 'from\(clientOptions: ClientOptions\)|streamHandlerExecutor|fun close\(|toBuilder' \
  autorender-java-core/src/main/kotlin/io/autorender/core/ClientOptions.kt
rg -n -C 5 'ClientOptions.*builder|toBuilder\(|streamHandlerExecutor|close\(' \
  autorender-java-core/src/test/kotlin

Repository: autorender/autorender-java

Length of output: 45084


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the relevant ClientOptions implementation sections and track copied executor references.
sed -n '24,145p' autorender-java-core/src/main/kotlin/io/autorender/core/ClientOptions.kt
sed -n '182,190p' autorender-java-core/src/main/kotlin/io/autorender/core/ClientOptions.kt
sed -n '458,543p' autorender-java-core/src/main/kotlin/io/autorender/core/ClientOptions.kt

# Locate AsyncStreamResponse/Client subclasses that call close() and subscribe() on streamHandlerExecutor.
rg -n -C 4 'streamHandlerExecutor|executor|close\(\)|Cannot subscribe after|execute\(' autorender-java-core/src/main/kotlin -g '*.kt'

# Behavioral probe: simulate Builder.from copying an ExecutorService and ClientOptions.close calling shutdown,
# then a later asynchronous subscribe/task attempting execution the way AsyncStreamResponse does.
python3 - <<'PY'
import threading, time
from concurrent.futures import Future
print("No runtime source is executed; this probe only documents ExecutorService shutdown semantics from Java ExecutorService.shutdown().")
print("Relevant observed code shape:")
print("- build default: streamHandlerExecutor wraps Executors.newCachedThreadPool and ClientOptions close calls ExecutorService.shutdown().")
print("- Builder.from copies clientOptions.streamHandlerExecutor value without creating a new executor.")
print("- AsyncStreamResponse close may call shutdown() on the same streamHandlerExecutor in other code paths if that executor is shared.")
print("Observed code also uses a PhantomReachableExecutorService wrapper whose shutdown likely delegates to the wrapped executable.")
PY

Repository: autorender/autorender-java

Length of output: 50385


Do not share streamHandlerExecutor ownership between copied ClientOptions.

Builder.from copies the same default ExecutorService, and ClientOptions.close() shuts it down. When toBuilder().build() replaces a reachable original client, copying also closes the original executor. Use an independent executor for copied options, or use shared reference-counted ownership that only shuts down after the last owner closes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@autorender-java-core/src/main/kotlin/io/autorender/core/ClientOptions.kt` at
line 187, Update Builder.from and the streamHandlerExecutor handling in
ClientOptions so copied options do not share shutdown ownership with the source
options. Create an independent executor for copied options, or implement
reference-counted ownership that keeps the executor alive until the final
ClientOptions.close() call.

sleeper = clientOptions.sleeper
clock = clientOptions.clock
baseUrl = clientOptions.baseUrl
Expand Down Expand Up @@ -211,6 +227,20 @@ private constructor(
*/
fun jsonMapper(jsonMapper: JsonMapper) = apply { this.jsonMapper = jsonMapper }

/**
* The executor to use for running [AsyncStreamResponse.Handler] callbacks.
*
* Defaults to a dedicated cached thread pool.
*
* This class takes ownership of the executor and shuts it down, if possible, when closed.
*/
fun streamHandlerExecutor(streamHandlerExecutor: Executor) = apply {
this.streamHandlerExecutor =
if (streamHandlerExecutor is ExecutorService)
PhantomReachableExecutorService(streamHandlerExecutor)
else streamHandlerExecutor
}

/**
* The interface to use for delaying execution, like during retries.
*
Expand Down Expand Up @@ -427,6 +457,24 @@ private constructor(
*/
fun build(): ClientOptions {
val httpClient = checkRequired("httpClient", httpClient)
val streamHandlerExecutor =
streamHandlerExecutor
?: PhantomReachableExecutorService(
Executors.newCachedThreadPool(
object : ThreadFactory {

private val threadFactory: ThreadFactory =
Executors.defaultThreadFactory()
private val count = AtomicLong(0)

override fun newThread(runnable: Runnable): Thread =
threadFactory.newThread(runnable).also {
it.name =
"autorender-stream-handler-thread-${count.getAndIncrement()}"
}
}
)
)
val sleeper = sleeper ?: PhantomReachableSleeper(DefaultSleeper())

val headers = Headers.builder()
Expand Down Expand Up @@ -464,6 +512,7 @@ private constructor(
.build(),
checkJacksonVersionCompatibility,
jsonMapper,
streamHandlerExecutor,
sleeper,
clock,
baseUrl,
Expand All @@ -490,6 +539,7 @@ private constructor(
*/
fun close() {
httpClient.close()
(streamHandlerExecutor as? ExecutorService)?.shutdown()
sleeper.close()
}
}
33 changes: 33 additions & 0 deletions autorender-java-core/src/main/kotlin/io/autorender/core/Page.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// File generated from our OpenAPI spec by Stainless.

package io.autorender.core

/**
* An interface representing a single page, with items of type [T], from a paginated endpoint
* response.
*
* Implementations of this interface are expected to request additional pages synchronously. For
* asynchronous pagination, see the [PageAsync] interface.
*/
interface Page<T> {

/**
* Returns whether there's another page after this one.
*
* The method generally doesn't make requests so the result depends entirely on the data in this
* page. If a significant amount of time has passed between requesting this page and calling
* this method, then the result could be stale.
*/
fun hasNextPage(): Boolean

/**
* Returns the page after this one by making another request.
*
* @throws IllegalStateException if it's impossible to get the next page. This exception is
* avoidable by calling [hasNextPage] first.
*/
fun nextPage(): Page<T>

/** Returns the items in this page. */
fun items(): List<T>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// File generated from our OpenAPI spec by Stainless.

package io.autorender.core

import java.util.concurrent.CompletableFuture

/**
* An interface representing a single page, with items of type [T], from a paginated endpoint
* response.
*
* Implementations of this interface are expected to request additional pages asynchronously. For
* synchronous pagination, see the [Page] interface.
*/
interface PageAsync<T> {

/**
* Returns whether there's another page after this one.
*
* The method generally doesn't make requests so the result depends entirely on the data in this
* page. If a significant amount of time has passed between requesting this page and calling
* this method, then the result could be stale.
*/
fun hasNextPage(): Boolean

/**
* Returns the page after this one by making another request.
*
* @throws IllegalStateException if it's impossible to get the next page. This exception is
* avoidable by calling [hasNextPage] first.
*/
fun nextPage(): CompletableFuture<out PageAsync<T>>

/** Returns the items in this page. */
fun items(): List<T>
}
Loading
Loading