Skip to content

Make manager reinitialization safe - #990

Open
bmehta001 wants to merge 4 commits into
mainfrom
bhamehta/fix-version-review
Open

Make manager reinitialization safe#990
bmehta001 wants to merge 4 commits into
mainfrom
bhamehta/fix-version-review

Conversation

@bmehta001

Copy link
Copy Markdown
Contributor

Summary

  • keep ORT GenAI globals alive across manager recreation
  • defer OgaShutdown() until process exit
  • preserve per-manager cleanup while making initialize() -> close() -> initialize() safe

This also includes the branch's previously completed Python package-version resolution changes.

Validation

  • CApiTest.ManagerCanBeRecreatedAfterRelease

Copilot AI balanced review requested due to automatic review settings August 12, 2026 08:30
@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 13, 2026 8:46am

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

Makes native manager recreation safe while improving Python package-version resolution.

Changes:

  • Defers GenAI global shutdown until process exit.
  • Updates manager lifetime documentation.
  • Resolves Python versions from source or distribution metadata with tests.

Reviewed changes

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

File Description
sdk_v2/cpp/src/manager.cc Moves GenAI shutdown to an exit hook.
sdk_v2/cpp/src/manager.h Documents process-scoped GenAI lifetime.
sdk_v2/python/src/foundry_local_sdk/version.py Adds package-version resolution logic.
sdk_v2/python/test/unit/test_version.py Tests version precedence and fallbacks.

💡 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/src/manager.cc Outdated
Comment thread sdk_v2/python/src/foundry_local_sdk/version.py Outdated
bmehta001 and others added 2 commits August 12, 2026 15:57
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c57f1ea3-b538-4db9-9223-8ca4f174f277
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c57f1ea3-b538-4db9-9223-8ca4f174f277
@baijumeswani

Copy link
Copy Markdown
Collaborator

ogashutdown and re-creating the ogamodel should work? Is that not the case?

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c90dfab7-1285-4d6c-a735-451395a09887
@bmehta001

bmehta001 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Yes - our pinned ORT GenAI 0.15.2 supports calling OgaShutdown() and recreating an OgaModel. This change avoids coupling each Foundry Manager lifetime to that process-global teardown/reinitialization cycle: manager-owned models and sessions are still released on Destroy(), while the final GenAI shutdown happens at process exit. I also fixed the exit ordering so any still-live manager is destroyed before that final shutdown.

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 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

sdk_v2/python/src/foundry_local_sdk/version.py:42

  • This source-first lookup can return an unrelated application's version. For example, a wheel installed with pip --target <project>/src has distribution metadata available, but this path resolves to <project>/pyproject.toml, so the host project's [project].version wins. Only accept the source-tree file when its project name identifies this distribution; otherwise fall back to _dist_version().
    source_version = _version_from_source_tree()
    if source_version is not None:
        return source_version

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c90dfab7-1285-4d6c-a735-451395a09887

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 3 out of 3 changed files in this pull request and generated no new comments.

@bmehta001 bmehta001 self-assigned this Aug 13, 2026
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.

3 participants