Skip to content

Add cancellation and deadline support to Session.process_request - #994

Open
bmehta001 wants to merge 21 commits into
mainfrom
bhamehta001/support-cancellation
Open

Add cancellation and deadline support to Session.process_request#994
bmehta001 wants to merge 21 commits into
mainfrom
bhamehta001/support-cancellation

Conversation

@bmehta001

@bmehta001 bmehta001 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds invocation-scoped cancellation and timeouts for synchronous inference without introducing a separate operation API.

  • Request.Cancel() targets only the active invocation, interrupts its ORT GenAI generator, and is a no-op while idle.
  • Each ProcessRequest call gets a fresh wall-clock deadline covering native admission and inference.
  • Session.Cancel() permanently cancels active and queued calls and rejects later processing on that Session.
  • Chat and audio remain serialized with cancellable admission; distinct embeddings Requests remain concurrent and cancel independently.
  • C#, JavaScript, and Python expose the same processing behavior and keep native Request/Session handles alive until inference exits.

Results

Cause Result
Request.Cancel() Active call returns FOUNDRY_LOCAL_ERROR_OPERATION_CANCELLED; later sequential reuse is supported
Timeout Active call returns FOUNDRY_LOCAL_ERROR_TIMEOUT; the next call starts a new deadline
Session.Cancel() Active/queued calls are cancelled; later calls return FOUNDRY_LOCAL_ERROR_INVALID_USAGE
Streaming callback returns nonzero Cooperative successful stop with FOUNDRY_LOCAL_FINISH_NONE

Cancellation, timeout, callback stop, completion, and failure use one first-winner invocation outcome. Cancelled or timed-out chat turns are not committed to history. Cooperative callback stop rewinds the uncommitted turn; hard engine interruption discards the generator so the next call rebuilds from committed history. Embeddings responses remain all-or-nothing.

Assumptions and deferrals

  • A Request is used by one invocation at a time. Sequential reuse is supported; overlapping use is not runtime-enforced.
  • Admission order is unspecified.
  • Timeout does not include time spent in a managed SDK worker queue before entering native ProcessRequest.
  • The C ABI is unpublished, so this initial release keeps API version 1 while tail-appending the new functions.
  • No public operation handle or shared deadline scheduler is added.

Validation

  • Focused native cancellation and generator-lifetime tests pass on Windows and Linux, including repeated shuffled runs.
  • Real streaming-audio cancellation passes on Windows, Linux, and macOS packaging CI.
  • C#, JavaScript, and Python cancellation/lifetime suites pass on their completed platform jobs.
  • CodeQL and all completed packaging checks pass; remaining packaging jobs are still running.

Copilot AI balanced review requested due to automatic review settings August 12, 2026 17:32
@vercel

vercel Bot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
foundry-local Ready Ready Preview Aug 16, 2026 7:45pm

Request Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds cross-language cancellation and deadline support for synchronous inference, including engine-level interruption and teardown handling.

Changes:

  • Adds native session cancellation, request deadlines, and timeout errors.
  • Exposes cancellation/timeouts through C++, Python, C#, and JavaScript.
  • Adds chat cancellation and deadline regression tests.

Reviewed changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
sdk_v2/python/src/foundry_local_sdk/session.py Adds timeout, cancellation, and teardown signaling.
sdk_v2/python/src/foundry_local_sdk/request.py Adds request timeout configuration.
sdk_v2/python/src/foundry_local_sdk/_native/build_cffi.py Extends Python C ABI definitions.
sdk_v2/js/src/session.ts Adds AbortSignal cancellation and session cancellation.
sdk_v2/js/src/request.ts Adds request deadlines.
sdk_v2/js/src/detail/native.ts Extends native TypeScript interfaces.
sdk_v2/js/src/detail/errors.ts Adds the timeout error code.
sdk_v2/js/native/src/session.h Declares native cancellation methods.
sdk_v2/js/native/src/session.cc Implements native session cancellation bindings.
sdk_v2/js/native/src/request.h Declares native timeout support.
sdk_v2/js/native/src/request.cc Implements native timeout binding.
sdk_v2/cs/src/Session.cs Adds token/session cancellation and teardown signaling.
sdk_v2/cs/src/Request.cs Adds TimeSpan deadlines.
sdk_v2/cs/src/Detail/NativeMethods.cs Extends C# ABI declarations.
sdk_v2/cs/src/Detail/FoundryLocalApi.cs Maps timeout errors and session cancellation.
sdk_v2/cpp/test/internal_api/chat/chat_session_test.cc Adds cancellation and deadline tests.
sdk_v2/cpp/src/inferencing/session/session.h Defines session cancellation state and watchdog support.
sdk_v2/cpp/src/inferencing/session/session.cc Implements cancellation, tracking, and deadlines.
sdk_v2/cpp/src/inferencing/session/session_manager.h Documents process-wide cancellation.
sdk_v2/cpp/src/inferencing/session/session_manager.cc Cancels sessions during shutdown.
sdk_v2/cpp/src/inferencing/session/request.h Implements reusable request deadlines.
sdk_v2/cpp/src/inferencing/session/oga_generator_cancellable.h Declares the OGA cancellation adapter.
sdk_v2/cpp/src/inferencing/session/oga_generator_cancellable.cc Implements engine termination.
sdk_v2/cpp/src/inferencing/session/live_session_registry.h Declares live-session tracking.
sdk_v2/cpp/src/inferencing/session/live_session_registry.cc Implements the live-session registry.
sdk_v2/cpp/src/inferencing/session/cancellable.h Defines the cancellation interface.
sdk_v2/cpp/src/inferencing/generative/embeddings/embeddings_session.h Adds request-aware embedding generation.
sdk_v2/cpp/src/inferencing/generative/embeddings/embeddings_session.cc Adds embedding cancellation checks.
sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc Publishes chat generators for cancellation.
sdk_v2/cpp/src/inferencing/generative/chat/chat_generator.h Makes chat generators cancellable.
sdk_v2/cpp/src/inferencing/generative/audio/audio_session.cc Adds cancellation throughout audio generation.
sdk_v2/cpp/src/inferencing/generative/audio/audio_generator.h Makes audio generators cancellable.
sdk_v2/cpp/src/c_api.cc Implements new C ABI operations.
sdk_v2/cpp/include/foundry_local/foundry_local_cpp.inline.h Implements C++ wrapper methods.
sdk_v2/cpp/include/foundry_local/foundry_local_cpp.h Exposes C++ cancellation and deadlines.
sdk_v2/cpp/include/foundry_local/foundry_local_c.h Extends the public C ABI.
sdk_v2/cpp/CMakeLists.txt Builds the new cancellation sources.
Suppressed comments (2)

sdk_v2/cpp/src/inferencing/generative/audio/audio_session.cc:604

  • This second raw OGA decode loop has the same termination-exception gap: Session::Cancel() now terminates the generator during GenerateNextToken(), but this helper does not catch the expected runtime error. A Nemotron cancellation or deadline can therefore bypass normal cancellation/timeout handling. Handle termination when original_request.ShouldStop() is true and rethrow unrelated engine failures.
  while (!generator.IsDone() && !generator.IsSessionTerminated() && !original_request.ShouldStop()) {

sdk_v2/js/src/session.ts:275

  • Request_Cancel only sets the request flag; it never invokes the active generator's Cancel(). Consequently this AbortSignal still waits for an in-progress prefill/decode call and cannot interrupt a non-terminating compute as documented. Moreover, the native cancellation path returns a FINISH_NONE response rather than FOUNDRY_LOCAL_ERROR_OPERATION_CANCELLED, so this promise resolves instead of rejecting. Associate request cancellation with its active generator and define the abort rejection before exposing this option.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread sdk_v2/cpp/include/foundry_local/foundry_local_c.h
Comment thread sdk_v2/cpp/src/inferencing/session/session.cc Outdated
Comment thread sdk_v2/cpp/src/inferencing/session/session.cc Outdated
Comment thread sdk_v2/cpp/src/inferencing/session/session.cc Outdated
Comment thread sdk_v2/cpp/src/inferencing/session/session_manager.cc Outdated
Comment thread sdk_v2/cpp/test/internal_api/chat/chat_session_test.cc Outdated
Comment thread sdk_v2/cpp/src/inferencing/session/session.cc Outdated
Comment thread sdk_v2/js/native/src/request.cc Outdated
Comment thread sdk_v2/cpp/src/c_api.cc Outdated
Comment thread sdk_v2/js/src/session.ts Outdated
bmehta001 and others added 2 commits August 13, 2026 12:46
Session.process_request() previously had no way to time out or be cancelled
outside the streaming path, so a non-terminating non-streaming generation
permanently pinned the session refcount. This caused Model.Unload() to fail
with '1 session(s) still using it', FoundryLocalManager.close() to exceed its
drain deadline, and process teardown to intermittently crash.

C++ core:
- Add ICancellable interface and OgaGeneratorCancellable adapter so Session
  can interrupt ORT GenAI mid-compute (SetRuntimeOption(terminate_session)),
  not just between token boundaries.
- Add Request::SetTimeout/ArmDeadline/ShouldStop for a re-armable wall-clock
  deadline that applies to streaming and non-streaming requests alike.
- Add Session::Cancel() with active-generator tracking and a deadline
  watchdog thread; wire it into all chat/audio/embeddings generation loops.
- Cancel() now latches request.canceled on every in-flight request (not just
  published generators) so finish_reason and history rollback are correct
  even when cancellation lands during prefill.
- Add LiveSessionRegistry so SessionManager::CancelAll() can reach every
  live session, including ones created via the direct API that never took
  a SessionRegistration.
- Add FOUNDRY_LOCAL_ERROR_TIMEOUT and two C ABI vtable entries:
  Request_SetTimeoutMs, Session_Cancel (appended to preserve ABI ordering).

Bindings (C++ wrapper, Python, C#, JS):
- Request.SetTimeout()/set_timeout()/setTimeout(), Session.Cancel()/cancel().
- Non-streaming ProcessRequestAsync (C#) and processRequest (JS) now accept
  a CancellationToken / AbortSignal that genuinely interrupts an in-flight
  generation, not just prevents scheduling.
- Session teardown (Python _close, C# Dispose, JS dispose) now cancels the
  session itself, covering the non-streaming case that was previously
  invisible to shutdown.

Tests: added ChatSessionTest coverage for timeout enforcement, timeout error
code, deadline re-arming across reused requests, mid-flight cross-thread
cancellation, cancel-before-request rejection, and cancel-when-idle safety.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 86c9e761-c751-48bc-8e73-2db5257c9e88
Prevent deadline enforcement from poisoning reusable chat state, preserve timeout semantics when ORT terminates raw generators, cancel inference before joining web workers, and release JS streaming callbacks on every exit.

Files changed:
- sdk_v2/cpp/src/inferencing/generative/audio/audio_session.cc
- sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc
- sdk_v2/cpp/src/inferencing/generative/chat/chat_session.h
- sdk_v2/cpp/src/inferencing/generative/embeddings/embeddings_session.cc
- sdk_v2/cpp/src/inferencing/session/oga_generator_cancellable.h
- sdk_v2/cpp/src/inferencing/session/session.cc
- sdk_v2/cpp/src/inferencing/session/session.h
- sdk_v2/cpp/src/inferencing/session/session_manager.cc
- sdk_v2/cpp/src/manager.cc
- sdk_v2/cpp/src/service/audio_transcriptions_handler.cc
- sdk_v2/cpp/src/service/chat_completions_handler.cc
- sdk_v2/cpp/src/service/responses_handler.cc
- sdk_v2/cpp/test/CMakeLists.txt
- sdk_v2/cpp/test/internal_api/chat/chat_session_test.cc
- sdk_v2/cpp/test/internal_api/oga_generator_cancellable_test.cc
- sdk_v2/cpp/test/internal_api/session_manager_test.cc
- sdk_v2/js/native/src/session.cc
- sdk_v2/js/test/streaming.test.ts

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349
Replace session-wide cancellation bookkeeping with a small per-call state so request cancellation interrupts the correct generator, queued timeout includes admission wait, and concurrent embedding requests remain isolated. Keep terminal Session.Cancel semantics without adding new API surface.

- Cancel active generators safely without retaining stale pointers
- Wake queued chat/audio requests on cancel or timeout
- Isolate distinct embedding request cancellation
- Preserve sequential request reuse and reject invalid timeout ranges
- Add focused model-free regression coverage

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349
# Conflicts:
#	sdk_v2/cpp/src/inferencing/session/session.cc
#	sdk_v2/cpp/src/inferencing/session/session.h
Make live-session shutdown cancellation lifetime-safe, keep JS and Python session handles alive through native work, and align cancellation outcomes across bindings. Replace timing-sensitive timeout tests and the old streaming-audio FINISH_NONE expectation with the new error contract.

- Store weak SessionControl references instead of raw Session pointers
- Defer JS and Python native release until active calls finish
- Reject invalid binding timeout values and pre-aborted JS requests
- Make timeout tests deterministic across CI machines
- Simplify cancellation naming, comments, and raw OGA adapter wiring

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349
Ensure CancellationToken cancellation completes with the caller token, release raced native responses, and keep Request and Session handles alive until all native processing exits.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 54 out of 54 changed files in this pull request and generated no new comments.

Suppressed comments (6)

sdk_v2/python/src/foundry_local_sdk/session.py:485

  • _check_open() releases the lifecycle lock before this native call, so _close() can transition to closing, release the native session, and leave this thread calling Session_Cancel with None or a freed pointer. cancel() is explicitly thread-safe and can race teardown; hold an active-call lease around the native call just like process_request() does.
    sdk_v2/python/src/foundry_local_sdk/request.py:138
  • A positive timeout below one millisecond is truncated to 0, which disables the deadline even though the API documents that only None and non-positive values disable it. Round positive durations up to the next millisecond so a requested deadline is never silently removed.
    sdk_v2/cs/src/Request.cs:148
  • A positive TimeSpan shorter than one millisecond casts to 0, silently disabling the timeout despite the contract saying only zero or negative values disable it. Round positive durations up to one millisecond rather than truncating them.
    sdk_v2/python/src/foundry_local_sdk/session.py:480
  • This documentation omits the method's terminal behavior: native Session_Cancel permanently marks the session cancelled, so even an idle call causes every later processing call to fail with invalid usage. State that explicitly; describing only interruption and saying it is safe while idle can lead callers to expect the session remains reusable.
    sdk_v2/cs/src/Session.cs:226
  • This public contract does not mention that Cancel() is terminal. The native implementation permanently cancels the session, and subsequent processing calls fail with invalid usage even when Cancel() was invoked while idle. Document that behavior so callers do not expect this to be a reusable, current-operation-only cancellation.
    sdk_v2/js/src/session.ts:308
  • The documentation presents this as interruption of current work, but native Session::Cancel() is terminal: calling it while idle still makes all later processing calls fail with FlErrorCode.InvalidUsage. Document the permanent state transition so consumers do not attempt to reuse the session.

Document terminal session cancellation, preserve positive sub-millisecond timeouts, and hold Python/C# native-call leases during cancellation and timeout updates. Make streaming-audio cancellation wait for active processing and consolidate redundant model timeout tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 54 out of 54 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

sdk_v2/js/src/session.ts:285

  • This listener has the same queueing race as streaming: an abort can occur before the async worker attaches the request's invocation state, and native Request.Cancel() then intentionally does nothing. Since the listener runs only once, the aborted operation can consume the entire generation before line 289 merely converts its successful result to AbortError. Retry until settlement or have the addon latch cancellation for the queued worker.
    sdk_v2/js/src/session.ts:166
  • Request.Cancel() is now an idle no-op, so this one-shot abort can race the queued native worker: if the signal fires after processStreamingRequest returns but before BeginInvocation attaches cancellation state, cancellation is lost and the full inference still runs before JavaScript rejects it. Keep retrying cancellation until the native promise settles, or add a pending-cancellation handshake in the addon.

This issue also appears on line 285 of the same file.
sdk_v2/cs/src/Session.cs:356

  • This catch wraps native TimeoutException and direct Request.Cancel()'s OperationCanceledException in FoundryLocalException whenever the linked token itself was not canceled. That makes streaming expose different error types from non-streaming and hides the newly added timeout result. Preserve these two mapped exception types before applying the generic streaming wrapper.

Comment thread sdk_v2/cpp/src/inferencing/session/live_session_registry.cc
Destroy chat generator state before releasing its model, make audio generator cancellation single-shot, preserve typed C# streaming errors, and make Python streaming call leases safe across thread startup. Drain streaming callbacks before finalizing audio and chat outcomes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349
@bmehta001
bmehta001 requested a balanced review from Copilot August 14, 2026 23:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated 2 comments.

Suppressed comments (4)

sdk_v2/cs/src/Session.cs:456

  • The disposal drain still does not cover every native session call. SetOptions, SetStreaming, and the ChatSession tool/turn methods only call ThrowIfDisposed() before invoking native code; none increments _activeCalls. Dispose can therefore set _disposing, observe zero here, and release _session after one of those methods passed its check but while its P/Invoke is still running. Wrap every session-handle operation in the same EnterActiveCall/ExitActiveCall lease (including the modality-specific methods) before relying on this count for release safety.
    sdk_v2/python/src/foundry_local_sdk/session.py:308
  • This is only a point-in-time check, not a lifetime lease. After the condition is released, _close() can switch to CLOSING, see _active_native_calls == 0, and release _ptr while set_options, set_streaming, or one of the newly guarded chat tool/turn methods is entering native code. Acquire a call lease and use its captured pointer for every native session-handle operation, releasing it in finally, just as the processing paths do.
    sdk_v2/cpp/src/inferencing/session/session.cc:272
  • Each request with a timeout creates a dedicated OS thread, including every queued chat/audio call and every concurrent embeddings call. The thread count therefore grows without bound with timed request concurrency; under load, std::thread construction can exhaust process resources and turn otherwise valid inference into std::system_error. Use a shared deadline scheduler/timer queue that retains each CancellationState instead of one thread per invocation.
  if (timeout.count() > 0) {
    deadline_thread = std::thread(&Session::MonitorDeadline, state, std::ref(logger_), timeout);

sdk_v2/cpp/src/inferencing/session/request.cc:44

  • This validates only milliseconds::rep, which is much wider than the representable steady_clock deadline on platforms whose clock duration is nanoseconds. Such values are accepted here (the JS layer explicitly accepts Number.MAX_SAFE_INTEGER) but DeadlineFor later rejects them during ProcessRequest, so an apparently valid timeout fails only when executed. Validate against the clock deadline range here, or define saturating behavior, so unsupported values are rejected by the setter as documented.
void Request::SetTimeoutMs(uint64_t timeout_ms) {
  using Rep = std::chrono::milliseconds::rep;
  const auto max_timeout = static_cast<uint64_t>((std::numeric_limits<Rep>::max)());
  if (timeout_ms > max_timeout) {
    FL_THROW(FOUNDRY_LOCAL_ERROR_INVALID_ARGUMENT, "request timeout is outside the supported range");

Comment on lines 98 to 101
out = ffi.new("flResponse**")
api.check_status(
api.inference.Session_ProcessRequest(session._ptr, request._ptr, out)
api.inference.Session_ProcessRequest(session_ptr, request._ptr, out)
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 1bfd759. StreamingResponse now acquires a Request processing lease before starting its worker, passes the captured pointer into Session_ProcessRequest, and releases the lease in the worker finally block and every setup/thread-start failure path. Explicit Request close cancels and waits before releasing.

Comment on lines +492 to +493
out = ffi.new("flResponse**")
api.check_status(api.inference.Session_ProcessRequest(session_ptr, request._ptr, out))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 1bfd759. Synchronous process_request now acquires both Session and Request leases, uses the captured Request pointer, and releases in nested finally blocks. Request close rejects new processing, cancels while active, drains, and releases exactly once.

Keep Python Request handles alive through synchronous and streaming processing, reject overlapping JavaScript streams before they can overwrite the session callback, and validate request timeouts against the steady-clock deadline range at setter time. Add deterministic lifetime, guard-release, and boundary coverage.

Files changed:
- sdk_v2/cpp/src/inferencing/session/request.cc
- sdk_v2/cpp/src/inferencing/session/session.cc
- sdk_v2/cpp/src/inferencing/session/timeout_limits.h
- sdk_v2/cpp/test/internal_api/session_cancellation_test.cc
- sdk_v2/js/src/session.ts
- sdk_v2/js/test/items.test.ts
- sdk_v2/js/test/session-stream-guard.test.ts
- sdk_v2/js/test/streaming.test.ts
- sdk_v2/python/src/foundry_local_sdk/request.py
- sdk_v2/python/src/foundry_local_sdk/session.py
- sdk_v2/python/test/unit/test_request_lifecycle.py
- sdk_v2/python/test/unit/test_session_lifecycle.py

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 59 out of 59 changed files in this pull request and generated 1 comment.

Comment thread sdk_v2/js/src/session.ts
Comment on lines 375 to +380
processStreamingRequest(request: Request, options?: StreamOptions): StreamingResponse {
return streamItems(this.native, request, options?.signal);
if (this.#streamingInFlight) {
throw new Error(CONCURRENT_STREAM_ERROR);
}

this.#streamingInFlight = true;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in fa66c65. JS Session now tracks active non-streaming calls and the active stream separately: a stream is rejected while any invocation is unsettled, and processRequest rejects while a stream is unsettled. Multiple non-streaming calls remain supported. Guard release is covered on success, error, abort, pre-abort, and synchronous startup failure.

bmehta001 added a commit that referenced this pull request Aug 15, 2026
Bring the latest PR #994 cancellation fixes into the dependent managed-session lifetime branch while preserving both Request processing leases and all-operation Session leases.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349
Remove century-scale timeout behavior and duplicate model-backed tests. Keep processing-time overflow safety, and use one minimal per-session guard so streaming cannot overlap any other invocation while concurrent non-streaming calls remain supported.

Files changed:
- sdk_v2/cpp/src/inferencing/session/request.cc
- sdk_v2/cpp/src/inferencing/session/session.cc
- sdk_v2/cpp/src/inferencing/session/timeout_limits.h
- sdk_v2/cpp/test/internal_api/session_cancellation_test.cc
- sdk_v2/js/src/session.ts
- sdk_v2/js/test/chat-session.test.ts
- sdk_v2/js/test/items.test.ts
- sdk_v2/js/test/session-stream-guard.test.ts
- sdk_v2/js/test/streaming.test.ts

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 58 out of 58 changed files in this pull request and generated no new comments.

Suppressed comments (1)

sdk_v2/cpp/src/inferencing/session/session.cc:272

  • Each timed invocation creates a dedicated OS thread and keeps it until the invocation settles. Because embeddings explicitly allow concurrent requests, a burst of timed embedding calls can create one watchdog thread (and stack reservation) per request, exhausting process/thread limits before inference admission provides any backpressure. Please use a shared deadline scheduler/timer queue (or another bounded watchdog mechanism) rather than spawning an unbounded thread per timeout.
  if (timeout.count() > 0) {
    deadline_thread = std::thread(&Session::MonitorDeadline, state, std::ref(logger_), timeout);

Centralize repeated stop-outcome handling in Session::ProcessRequest and remove an Unregister wakeup that cannot change admission predicates. Preserve first-winner outcomes, telemetry, diagnostic flags, and cancellation-drain behavior.

Files changed:
- sdk_v2/cpp/src/inferencing/session/session.cc
- sdk_v2/cpp/src/inferencing/session/session_control.cc

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 58 out of 58 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

sdk_v2/cpp/src/inferencing/session/session.cc:98

  • The move constructor is declared noexcept, but this allocation and the following registry insertion can both throw (for example std::bad_alloc). Any such failure therefore terminates the process instead of propagating. Either avoid allocating/registering a replacement control for the moved-from object, or remove noexcept consistently from the base and derived move constructors.
  other.control_ = std::make_shared<SessionControl>();

  LiveSessionRegistry::Instance().Add(other.control_);

Comment on lines +467 to +471
if not self._streaming_in_flight.acquire(blocking=False):
raise FoundryLocalException(
"Concurrent streaming requests on the same session are not supported. "
"Drain or cancel the in-flight stream before starting another."
)
Comment thread sdk_v2/cs/src/Session.cs
Comment on lines +296 to +300
if (Interlocked.CompareExchange(ref _activeChannel, channel, null) != null)
{
throw new InvalidOperationException(
"Concurrent streaming requests on the same session are not supported. "
+ "Drain or cancel the in-flight stream before starting another.");
Keep timeout outcome, diagnostics, engine interruption, and generator cancellation in one lock-held helper, and let completion arbitration perform its own guarded state check.

Files changed:
- sdk_v2/cpp/src/inferencing/session/cancellation_state.h
- sdk_v2/cpp/src/inferencing/session/cancellation_state.cc
- sdk_v2/cpp/src/inferencing/session/session.cc

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349
Restore cache and cancellation comments whose invariants remain valid, updated for first-winner cancellation behavior. Remove a model-backed test of framework pre-cancellation and a duplicate addon-internal timeout validation test.

Files changed:
- sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc
- sdk_v2/cs/test/FoundryLocal.Tests/ChatSessionTests.cs
- sdk_v2/js/test/items.test.ts

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 58 out of 58 changed files in this pull request and generated no new comments.

Suppressed comments (3)

sdk_v2/js/src/session.ts:388

  • This concurrency guard can mask disposal: after dispose() with an unsettled non-streaming call, a new streaming call throws the generic overlap Error instead of the documented FoundryLocalError with InvalidUsage; a pre-aborted call similarly returns AbortError. Perform the disposed check before these local guards and abort handling.
    sdk_v2/cpp/include/foundry_local/foundry_local_c.h:188
  • The new timeout code is not handled by ActionStatusFromException (sdk_v2/cpp/src/telemetry/telemetry.cc:91-100). Any outer ActionTracker that records a timeout therefore reports kFailure instead of kTimeout, even though the session-level tracker distinguishes it. Add the timeout case to the central exception-to-status mapping and cover it with its mapping tests.
  /// The configured timeout elapsed. FOUNDRY_LOCAL_ERROR_OPERATION_CANCELLED indicates explicit cancellation.
  FOUNDRY_LOCAL_ERROR_TIMEOUT = 7,

sdk_v2/js/src/session.ts:328

  • Local checks run before the native disposed check. If dispose() is called while a stream is settling, this method rejects with the generic overlap error; with a pre-aborted signal it rejects with AbortError. Both contradict the class contract that every post-disposal call fails with FoundryLocalError/InvalidUsage. Check native.isDisposed() first and surface the documented disposed error before concurrency or signal handling.

This issue also appears on line 385 of the same file.

Bring the full modified chat session file under the 120-character limit without changing behavior.

Files changed:
- sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16cc8b6f-2bc4-4469-bf59-f21439569349
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants