Skip to content

Fix CloudXR runtime crash from OpenSSL symbol conflict#254

Merged
rwiltz merged 8 commits into
mainfrom
rwiltz/strip-ssl
Mar 12, 2026
Merged

Fix CloudXR runtime crash from OpenSSL symbol conflict#254
rwiltz merged 8 commits into
mainfrom
rwiltz/strip-ssl

Conversation

@rwiltz
Copy link
Copy Markdown
Contributor

@rwiltz rwiltz commented Mar 12, 2026

Load libcloudxr.so with RTLD_DEEPBIND so NVST resolves OpenSSL symbols from its own scope instead of picking up Python's incompatible libssl.so.3 from the global scope.
Add a patchelf build step to strip the spurious libssl.so.3 NEEDED entry from libcloudxr.so after extracting the SDK tarball, since the library never calls any OpenSSL functions directly.

Summary by CodeRabbit

  • Bug Fixes

    • Improved library loading behavior and runtime initialization stability.
  • Chores

    • Optimized build process to remove unnecessary system dependencies from the CloudXR library, reducing installation footprint and improving initialization performance.

@rwiltz rwiltz requested a review from jiwenc-nv March 12, 2026 18:27
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 12, 2026

📝 Walkthrough

Walkthrough

These changes implement conditional library dependency stripping and symbol binding behavior for CloudXR. A new build-time patchelf integration removes the libssl.so.3 dependency from libcloudxr.so when available, while runtime library loading uses RTLD_DEEPBIND flag mode when supported by the platform.

Changes

Cohort / File(s) Summary
Build-time Dependency Patching
src/core/cloudxr/python/CMakeLists.txt
Introduces cloudxr_patchelf custom target that conditionally strips libssl.so.3 dependency using patchelf. Adjusts cloudxr_python dependency chain to use patchelf target when available, otherwise falls back to cloudxr_native_bundle. Emits warning if patchelf is unavailable.
Runtime Symbol Loading
src/core/cloudxr/python/runtime.py
Changes ctypes.CDLL library loading mode from default to RTLD_DEEPBIND flag when available on the platform, affecting symbol resolution behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A patch here, a bind there,
Dependencies disappear in the air,
With patchelf's magic touch so fine,
And symbols deep-bound in a line,
The library builds, clean and fair! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix CloudXR runtime crash from OpenSSL symbol conflict' directly describes the main change: resolving a crash caused by OpenSSL symbol conflicts through patchelf and library loading modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch rwiltz/strip-ssl
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@rwiltz rwiltz changed the title Rwiltz/strip ssl Fix CloudXR runtime crash from OpenSSL symbol conflict Mar 12, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/core/cloudxr/python/CMakeLists.txt`:
- Around line 87-94: The patchelf target (cloudxr_patchelf using
PATCHELF_EXECUTABLE on "${CLOUDXR_NATIVE_DIR}/libcloudxr.so") must be guarded by
a symbol check: after extracting the SDK tarball (respecting
CXR_RUNTIME_SDK_VERSION) run readelf -d on libcloudxr.so to confirm libssl.so.3
is in DT_NEEDED, then run readelf -Ws (or equivalent) to search undefined symbol
names for OpenSSL prefixes (e.g. SSL_, EVP_, TLS_, BIO_, X509_); only execute
patchelf --remove-needed libssl.so.3 if libssl.so.3 is needed and no OpenSSL
symbols are referenced, otherwise fail the build with a clear error message
instructing to remove the workaround or update the SDK; implement this logic in
the cloudxr_patchelf custom target (or a helper custom command) so
add_dependencies(cloudxr_patchelf cloudxr_native_bundle) remains correct.

In `@src/core/cloudxr/python/runtime.py`:
- Around line 193-194: Consolidate libcloudxr.so loading by extracting the
ctypes.CDLL call into a single helper (e.g., _load_libcloudxr or
load_libcloudxr) and replace direct ctypes.CDLL usages in both run() and
runtime_version() with that helper; ensure the helper accepts/uses RTLD_DEEPBIND
(using getattr(os, "RTLD_DEEPBIND", 0)) as the mode and returns the loaded
library so both run() and runtime_version() get the same symbol-resolution
behavior consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 53f5927c-8627-4941-ac90-d9a31cfa7a2c

📥 Commits

Reviewing files that changed from the base of the PR and between 6dc03be and 5fc3b7b.

📒 Files selected for processing (2)
  • src/core/cloudxr/python/CMakeLists.txt
  • src/core/cloudxr/python/runtime.py

Comment thread src/core/cloudxr/python/CMakeLists.txt
Comment thread src/core/cloudxr/python/runtime.py
@rwiltz rwiltz enabled auto-merge (squash) March 12, 2026 21:15
@rwiltz rwiltz merged commit 6de8b35 into main Mar 12, 2026
61 checks passed
jiwenc-nv added a commit that referenced this pull request Mar 13, 2026
* Revert "Promote run_cloudxr_via_wheel.sh to run_cloudxr.sh"

This reverts commit 882ae3b.

* Add Kitmaker integration (#172)

* Revert "Rename run_cloudxr.sh to run_cloudxr_via_docker.sh"

This reverts commit ae0d9b6.

* Revert "Revert "Rename run_cloudxr.sh to run_cloudxr_via_docker.sh""

This reverts commit 706d746.

* Revert "Revert "Promote run_cloudxr_via_wheel.sh to run_cloudxr.sh""

This reverts commit c32b91d.

* WIP test

---------

Co-authored-by: Jiwen Cai <jiwenc@nvidia.com>
Co-authored-by: Andrei Aristarkhov <aaristarkhov@nvidia.com>
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