openai / openai/openai-java

Async internal plumbing bypasses both `dispatcherExecutorService` and `streamHandlerExecutor`, forcing a hop through `ForkJoinPool.commonPool()`

Open
#952 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Kotlin
Stars
1.5k
Forks
264
Avg merge
9h 46m
Merged PRs (30d)
96

Description

Sorry this issue is AI generated, I ran into this problem while trying to fix my issues with the security context. I think this would allow me to solve it a bit cleaner than i currently can. If you dont allow or want AI generated issues feel free to close this.

Summary

Every generated async service implementation (48 files, e.g. ChatCompletionServiceAsyncImpl.kt,
EmbeddingServiceAsyncImpl.kt, ResponseServiceAsyncImpl.kt, ...) chains its internal
CompletableFuture composition with the no-executor overloads, e.g.:

// ChatCompletionServiceAsyncImpl.kt:185
.thenComposeAsync { clientOptions.httpClient.executeAsync(it, requestOptions) }

Per plain CompletableFuture semantics, thenComposeAsync(fn) with no executor argument runs fn on
ForkJoinPool.commonPool() - a JVM-wide, shared pool that has no relationship to either
ClientOptions.Builder.dispatcherExecutorService(...) or .streamHandlerExecutor(...). Both of those
builder methods are documented as ways for a caller to control the SDK's threading, but this internal
hop is invisible to both of them and cannot be configured through any public API.

Why this matters

In a Jakarta EE / Java EE environment, thread identity/security-context propagation for a
ManagedExecutorService is captured from whichever thread calls .execute()/.submit(). If an
application supplies a container-managed executor via dispatcherExecutorService/
streamHandlerExecutor specifically to get correct context propagation into async callbacks, this
ForkJoinPool.commonPool() hop is a gap neither setting can close: the pool's own worker threads are
plain JDK threads, created once and reused for the lifetime of the JVM, with no Jakarta EE context
association at all. This can result in security-context confusion (the wrong "current user" being
resolved) for any application relying on ambient/thread-local identity propagation while integrating
this SDK's async APIs - the exact failure mode we hit and traced in detail (see reproduction below).

Reproduction / trace (traced against 4.31.0)

Full call chain for client.async().chat().completions().createStreaming(params).subscribe(handler, executor):

  1. ChatCompletionServiceAsyncImpl.kt:161-186 builds the request via .prepareAsync(...), which
    returns an already-completed future (PrepareRequest.kt:27-33).
  2. ChatCompletionServiceAsyncImpl.kt:185:
    .thenComposeAsync { clientOptions.httpClient.executeAsync(it, requestOptions) }
    
    No executor argument on an already-complete future -> fn runs on ForkJoinPool.commonPool(),
    not on the caller's thread and not on dispatcherExecutorService.
  3. From that commonPool thread, OkHttpClient.kt:52-79's executeAsync calls call.enqueue(callback).
    OkHttp's Dispatcher (configured via dispatcherExecutorService - OkHttpClient.kt:267,
    dispatcherExecutorService?.let { dispatcher(Dispatcher(it)) }) picks up the actual HTTP work from
    here, so this hop is the one dispatcherExecutorService genuinely controls.
  4. future.complete(response.toResponse()) (OkHttpClient.kt:62) runs inside Callback.onResponse,
    on the dispatcher thread.
  5. That triggers, synchronously on the same dispatcher thread: retry bookkeeping
    (RetryingHttpClient.kt:101-129, deliberately same-thread - "Run in the same thread."), then
    ChatCompletionServiceAsyncImpl.kt:186/76's .thenApply { ... } chain.
  6. Finally, AsyncStreamResponse.kt:94-133's whenCompleteAsync({ ... }, executor) - executor here
    is clientOptions.streamHandlerExecutor (ChatCompletionServiceAsyncImpl.kt:77) or the
    caller-supplied subscribe(handler, executor) argument. This is the only hop of the whole chain
    that either public builder setting actually reaches - and it's reached only because the dispatcher
    thread (step 4) happens to call executor.execute(...) at this point.

So: dispatcherExecutorService controls step 3 only; streamHandlerExecutor/subscribe's executor
argument controls step 6 only. Step 2 - the very first async hop, before either configured executor is
ever touched - always runs on ForkJoinPool.commonPool(), unconditionally, for every async service
call in the SDK.

(Confirmed directly by reading openai-java-core-4.31.0-sources.jar and
openai-java-client-okhttp-4.31.0-sources.jar; standard CompletableFuture.thenComposeAsync(fn)
executor-selection semantics are per the JDK spec, not inferred.)

Scope

The same .thenComposeAsync { ... } / .thenApplyAsync { ... } no-executor pattern appears in 48
files under com.openai.services.async.** (grep across the 4.31.0 core sources jar), e.g.
BatchServiceAsyncImpl.kt, EmbeddingServiceAsyncImpl.kt, ResponseServiceAsyncImpl.kt,
FileServiceAsyncImpl.kt, VectorStoreServiceAsyncImpl.kt, and 43 others - this is a
code-generation-template-level issue, not isolated to chat completions.

Reconfirmed on 4.54.0 (the current release at time of filing): BatchServiceAsyncImpl.kt's
create/retrieve/list/cancel all still contain the identical
request.thenComposeAsync { clientOptions.httpClient.executeAsync(it, requestOptions) } with no
executor argument, so this is not something already fixed between 4.31.0 and the latest release.

Suggested direction

Thread an explicit executor through the first .thenComposeAsync/.thenApplyAsync call in each
generated service implementation, rather than relying on the no-arg overload's default
(ForkJoinPool.commonPool()). Possible approaches, in rough order of how much they change the public
API:

  1. Reuse dispatcherExecutorService for this hop too, since it's already positioned as "the executor
    for running HTTP requests" and this hop exists purely to kick off that HTTP request.
  2. Introduce a new, distinct ClientOptions executor (e.g. internalAsyncExecutor) if the maintainers
    consider "the executor that runs HTTP requests" and "the executor that does internal
    CompletableFuture bookkeeping before the HTTP call" to be conceptually different things.

Either way, since this is generated code (each file's header states "File generated from our OpenAPI
spec by Castiron. See CONTRIBUTING.md for details."; CONTRIBUTING.md itself says "Most of the SDK is
generated code. Modifications to code will be persisted between generations, but may result in merge
conflicts..."), a complete fix needs to happen in the generator template, not just in the 48 checked-in
files, for it to apply consistently and survive regeneration across the whole SDK.

Environment
  • com.openai:openai-java-core, com.openai:openai-java-client-okhttp - reproduced on both 4.31.0 and
    the current 4.54.0.
  • Found while integrating streaming chat completions into a Jakarta EE 8 (Payara 5) application using
    javax.enterprise.concurrent.ManagedExecutorService for context/identity propagation.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Begin with ChatCompletionServiceAsyncImpl.kt and BatchServiceAsyncImpl.kt, then inspect the generator path described by the generated-file header and CONTRIBUTING.md. Trace the no-executor thenComposeAsync/thenApplyAsync calls and identify the template change needed across the async services. Done means regenerated implementations consistently use an explicit configured executor and the security-context reproduction no longer reaches ForkJoinPool.commonPool().

Written by the indexing model from the issue text.

Assessment

Tech stack
java, kotlin
Domain
api, backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.