Async internal plumbing bypasses both `dispatcherExecutorService` and `streamHandlerExecutor`, forcing a hop through `ForkJoinPool.commonPool()`
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 5/5
- Thời gian dự kiến
- Hơn một tuần
- Mức phù hợp với người mới
- 35/100
Hướng nghiên cứu
Bắt đầu với ChatCompletionServiceAsyncImpl.kt và BatchServiceAsyncImpl.kt, sau đó kiểm tra đường dẫn generator được mô tả trong header của file được sinh ra và CONTRIBUTING.md. Truy vết các lệnh gọi thenComposeAsync/thenApplyAsync không có executor và xác định thay đổi cần thiết trong template trên toàn bộ các service bất đồng bộ. Được xem là hoàn tất khi các implementation được sinh lại nhất quán sử dụng một executor được cấu hình rõ ràng và việc tái tạo security context không còn đi tới ForkJoinPool.commonPool().
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
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):
ChatCompletionServiceAsyncImpl.kt:161-186builds the request via.prepareAsync(...), which
returns an already-completed future (PrepareRequest.kt:27-33).ChatCompletionServiceAsyncImpl.kt:185:
No executor argument on an already-complete future ->.thenComposeAsync { clientOptions.httpClient.executeAsync(it, requestOptions) }fnruns onForkJoinPool.commonPool(),
not on the caller's thread and not ondispatcherExecutorService.- From that commonPool thread,
OkHttpClient.kt:52-79'sexecuteAsynccallscall.enqueue(callback).
OkHttp'sDispatcher(configured viadispatcherExecutorService-OkHttpClient.kt:267,
dispatcherExecutorService?.let { dispatcher(Dispatcher(it)) }) picks up the actual HTTP work from
here, so this hop is the onedispatcherExecutorServicegenuinely controls. future.complete(response.toResponse())(OkHttpClient.kt:62) runs insideCallback.onResponse,
on the dispatcher thread.- 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. - Finally,
AsyncStreamResponse.kt:94-133'swhenCompleteAsync({ ... }, executor)-executorhere
isclientOptions.streamHandlerExecutor(ChatCompletionServiceAsyncImpl.kt:77) or the
caller-suppliedsubscribe(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 callexecutor.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:
- Reuse
dispatcherExecutorServicefor 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. - Introduce a new, distinct
ClientOptionsexecutor (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.ManagedExecutorServicefor context/identity propagation.
- Ngôn ngữ chính
- Kotlin
- Star
- 1.5k
- Fork
- 264
- Merge trung bình
- 13 giờ 51 phút
- Pull request đã merge (30 ngày)
- 113
Hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của openai/openai-java
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
openai/openai-java#886 · 2 bình luận ·
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 68/100
openai/openai-java#802 · 2 bình luận · 3 reaction ·
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 72/100
openai/openai-java#755 · 1 bình luận ·
-
bug sdk
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 72/100
openai/openai-java#652 · 2 bình luận ·
-
Độ khó 3/5 1-2 ngày Mức phù hợp với người mới 74/100
openai/openai-java#973 ·
Tất cả issue của openai/openai-java
Issue tương tự
-
Flaky: a relaunched catch-up replay can still report catching up right after its marker is written Đang mởbug
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
johanhaleby/occurrent#1134 ·
-
autoteam community team/use
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
-
android: AlbumArtistDetailViewModel keeps groupKeys of albums that disappeared in expandedAlbums Đang mởtask
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 70/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 90/100
aaif-goose/goose#12496 ·
-
Schema rejections are neither logged nor attributed, so the offending object cannot be identified Đang mở
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 75/100
zotero/zotero-android#350 ·