Skip to content

Make CXX molecule ownership exclusive and validate packaged RDKit workflows - #57

Draft
xrl wants to merge 8 commits into
mainfrom
modernization/safe-cxx-packaged-rdkit
Draft

xrl wants to merge 8 commits into
mainfrom
modernization/safe-cxx-packaged-rdkit

Conversation

@xrl

@xrl xrl commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Existing shared native handles could expose aliasing mutable atoms, parser nulls could become invalid molecules, and most throwing C++ calls did not translate failures. This draft replaces those handles with exclusive ownership, ties borrowed views to their molecules, and tests a useful chemistry workflow against ordinary Ubuntu RDKit packages.

Behavior and design

  • Use UniquePtr, &T and Pin<&mut T> throughout the exposed native ownership boundary. Native deep copies preserve independence; consuming RWMol conversion uses C++ base-class conversion instead of Rust transmute.
  • Translate throwing calls, check parser failures, indices, integer/enum inputs and exception downcasts, and acquire container ownership before potentially throwing allocations. Preserve valid empty SMILES/SMARTS.
  • Add immutable atom/bond views, checked graph editing, 2D conformer coordinates, configurable/MACCS fingerprints and functional scaffold networks. Fix exact fingerprint lengths, zero-vector similarity, tautomer remaining length and SDF record/error handling.
  • Keep owned molecules Send and !Sync; borrowed views do not gain unsafe thread traits. Periodic valence lists are owned copies; table handles own a small stateless shim, supporting RDKit 2026's deleted native copy constructor.
  • Map bond and hybridization enums explicitly, preserving Rust values across native enum-width changes. Assert the specific descriptor results of the supported releases without changing RDKit's chemistry.
  • Retain existing high-level convenience methods where practical and add fallible alternatives. Preserve the error text used by an actual downstream caller.

The audit covers all 145 original native declarations, their C++ shims and high-level wrappers. The branch has 189 native callable declarations across 11 modules, listed in the inventory. This is a defined binding subset, with no claim of complete RDKit API coverage or a memory-safety proof.

Native/platform matrix

Verified 2026-10-09. Current upstream is RDKit 2026.09.1, published 2026-10-03; the practical default is a released LTS package.

Target Exact RDKit development package Role
Ubuntu 26.04 LTS amd64 202503.6-4 / RDKit 2025.03.6 Primary install and CI target
Ubuntu 24.04 LTS amd64 202309.3-3build2 / RDKit 2023.09.3 Compatibility CI target
Saved Debian 13 amd64 202503.1-4 / RDKit 2025.03.1 Local validation
Ubuntu 26.04 amd64 and arm64, upstream 2026.09.1 2026.09.1+ds1-1~ubuntu26.04 Additional reference-package CI targets

Ubuntu 26.10 is still prerelease on this verification date. The build instructions link official dated sources and pin Ubuntu container digests, RDKit package versions, Rust 1.99.0, CXX 1.0.203 through the committed lockfile, and C++20. Prefix discovery is explicit and Cargo tracks its inputs. The activated-Conda feature is forwarded, with distro-prefix feature checks distinguished from a real Conda solve.

Validation

Final-head CI and recorded local validation:

Check Result
Full workspace CI 110 tests passed on each of four platforms: Ubuntu 24.04/RDKit 2023.09 amd64; Ubuntu 26.04/RDKit 2025.03 amd64; reference RDKit 2026.09.1 on native amd64 and arm64
Local reference-package run 110 tests passed with all targets; example and stable benchmark targets passed
Prior saved-environment run, all targets 107 passed; examples and stable benchmark smoke also ran
Borrow/lifetime compile failures 6/6 expected diagnostics matched on all four CI platforms, inside one test harness per platform
Focused safety/correctness regressions 20 passed against RDKit 2023.09, 2025.03 and 2026.09
Ubuntu 26.04 C++ ASan/UBSan 81 tests passed against each of RDKit 2025.03.6 and reference 2026.09.1; leak detection enabled, no findings
Isolated real downstream application 50 existing tests passed in the saved environment and again with reference RDKit 2026.09.1 on Ubuntu 26.04; no application source or test changes
Formatting and linting Rustfmt, clang-format 18, strict all-targets Clippy passed
Documentation Rustdoc with warnings denied passed on both Ubuntu versions

The validation record explains the commands, development failures fixed, and limits. ASan/UBSan instrument the bridge and inlined header code; packaged RDKit and Rust are not rebuilt with sanitizers. Native version differences are asserted explicitly, including canonicalization, periodic-table data and the corrected 2026 chi2n/chi2v registry results.

The reference installer pins the release bundle and published manifest hashes, checks every internal checksum, and installs its exact runtime/development/data packages. This validates GitHub release-artifact installation, using apt-get for dependency resolution. The packaging project's separate APT verification passed on both architectures; those native smoke tests are separate from this full binding suite.

Final-head CI passed for 0ae060c9cfc14626172070989fee890b16454373: Packaged RDKit passed all four platform jobs, both C++ ASan/UBSan jobs and the required test_suite check. C++ formatting also passed. Every platform job passed tests, strict all-targets Clippy, rustdoc and all-features prefix checks. The initial expanded run failed during Docker Hub image pulls; the successful run uses Docker's public ECR mirror with identical pinned digests.

Migration and limits

See the migration guide. Low-level callers must adopt unique owners, borrowed inputs and Result. RWMol::from_smarts has a typed error; Uncharger requires mutable access; valence lists are owned vectors; unknown sanitize exception kinds have an Other variant. Molecule borrow rules intentionally prevent formerly aliasing access. The historical tanimoto_distance name remains a similarity alias.

Outstanding scope includes dedicated ring APIs/additional named descriptors, 3D embedding/ETKDG/ensembles, MMFF/UFF, standalone Murcko helpers, reactions and other broad upstream areas. Windows, macOS and real Conda installation remain unvalidated. Arm64 scope is the new reference-package CI job; local and sanitizer checks run on amd64. Native chemistry data/algorithms can change between RDKit versions; binary ABI interchangeability is not promised.

Contributor work and triage

This adapts and credits useful work from #19, #45, #50, #51, #52, #54 and #56. The scaffold implementation/tests in #56 and atom/bond/fingerprint direction in #50–#54 were reworked around the ownership contract. Full attribution is in the audit and relevant commit trailers.

All 10 open issues and 10 open PRs were enumerated with pagination, including a second empty page for each collection, and their discussions/diffs/status reviewed. Pending fixes stay open until they land; partial work such as #53, #54 and #55 is not treated as resolved.

No new code comments, crate version bumps, releases, tags, merges or deployment are included. This remains a draft for review.

Complete triage ledger

All actions below were posted after reviewing the paginated open collections, discussions, diffs and current status. Closures do not imply the draft's pending code has merged.

Item Action Rationale / recorded update
PR #14 Closed Update: Superseded scaffold prototype; functional replacement pending in #57/#56, #21 stays open.
issue #16 Kept open Update: Bond views/basic edits implemented in draft; broader controls remain.
issue #18 Kept open Update: Invalid SMARTS regression fixed in draft; keep open until landed.
PR #19 Kept open Update: Contribution adapted and credited; replacement still pending.
issue #21 Kept open Update: Functional scaffold nodes/edges/counts in draft; keep open until landed.
issue #23 Kept open Update: Prefix/header guidance improved; macOS reproduction still unvalidated.
PR #28 Closed Update: Superseded CI approach; package matrix replaces it, real Conda solve unvalidated.
issue #37 Kept open Update: Windows support remains unimplemented/unvalidated.
issue #39 Kept open Update: Direct methods retained where suitable; checked shims still required.
PR #45 Kept open Update: Explicit prefix support adapted and credited; pending replacement review.
issue #46 Closed Update: Scope question answered; native bindings remain the chosen approach.
issue #48 Kept open Update: Reproducible package installation documented/tested in draft; pending merge.
issue #49 Kept open Update: Crates.io/workspace links corrected in draft; pending merge.
PR #50 Kept open Update: Ownership/AtomRef/tooling adapted; remaining differences preserved for review.
PR #51 Kept open Update: Bond/basic-edit work adapted; additional mutation/MolOps scope remains.
PR #52 Kept open Update: Configurable/MACCS fingerprints adapted and tested; pending review.
PR #53 Kept open Update: Dedicated ring APIs and added named descriptors remain outstanding.
PR #54 Kept open Update: 2D/coordinate views only; 3D embedding and wider MolOps remain.
issue #55 Kept open Update: Murcko/ETKDG/ensembles/MMFF/UFF request remains outstanding.
PR #56 Kept open Update: Functional scaffold implementation/tests adapted and credited; pending merge.

xrl and others added 3 commits October 9, 2026 20:30
Audit all existing native declarations and shims, translate exceptions, and
tie atom, bond and conformer views to their molecule owners. Add graph edits,
configurable fingerprints and scaffold networks with regression coverage.
Preserve compatible downstream error text and native version semantics.

Adapt contributor work from PRs #19, #50, #51, #52, #54 and #56.

Co-authored-by: Alexandra Ebberg <aebberg@uni-muenster.de>
Co-authored-by: Sharif Haason <sharif.haason@gmail.com>
Co-authored-by: Chris Petersen <chris@petersen.io>
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.

1 participant