feat: add config to disable or restrict native UDF library loading, and load libraries without a global lock

[andygrove]

Which issue does this PR close?

Closes #6175.

Closes #5297.

Rationale for this change

CometNativeUDF.register loads whatever shared library the caller names, on the driver and on every executor, with the full privileges of the JVM. An operator had no way to turn the feature off or to limit where libraries can come from (#6175).

Separately, the native UDF library cache held its single write lock across dlopen, so one slow library load blocked lookups of every other library, and a panic under the lock poisoned it and disabled native UDFs for the life of the process (#5297).

What changes are included in this PR?

Load policy (#6175)

• spark.comet.nativeUdf.enabled (default true): when false, no native UDF library is loaded.
• spark.comet.nativeUdf.allowedPaths (default empty, meaning unrestricted): comma-separated directories a library must be under. The library path and each directory are canonicalized before comparing, so .. and symlinks cannot escape, and matching is by path component (/opt/udfs does not match /opt/udfs-other). With a list set, a relative path or one that does not resolve is refused, since the dynamic loader's search path cannot be checked.
• The policy lives in one Rust type (c_udf/policy.rs) used on both sides. The driver passes the two configs to validateLibrary, so register fails with the new CometNativeUdfNotAllowedException. The executor receives them in the serialized Spark configs, keeps them as a DataFusion session config extension, and the planner checks the path in the NativeScalarUdf before loading it, so a plan built elsewhere cannot bypass the driver check.
• Defaults to enabled so existing opt-in-by-API-call behavior is unchanged.
• Docs: new "Restricting which libraries can be loaded" section in rust_udfs.md, including that these are ordinary session configs and the allow-list is not protected against a replaced file.

Library cache (#5297)

• The cache is now a map of per-library slots. The map lock is only held for a lookup or insert, never across a load; each slot's mutex is held while that library loads, so concurrent requests for the same library share one load and requests for other libraries are not blocked.
• Poisoned locks are recovered rather than unwrapped. A slot is only filled after a successful load, so a library whose load panics fails its own queries and the next request retries.
• Libraries are still never unloaded, and a symlink and its target still share one LoadedLibrary.

How are these changes tested?

• Rust unit tests in policy.rs (disabled, allow-list match, sibling-prefix directory, .. and symlink escapes, symlinked allowed directory, relative and missing paths, comma parsing, config parsing), in cache.rs (a blocked load does not block another library, concurrent requests load once, a panicking load does not disable the cache, a failed load is retried), and a planner test that the policy extension is applied to a NativeScalarUdf.
• Four new cases in CometNativeUdfSuite: register refused when disabled, register refused outside allowedPaths, register and run under allowedPaths, and the executor refusing a function registered under the default policy once the policy is tightened. The suite passes (72 tests) on the default Spark profile.
• cargo clippy --all-targets --workspace -- -D warnings, make format, and make are clean.

[sunchao]

Reviewed 99beb0f1fff3df55ec47efa459864b4dd76ae8f9 against supplied base 5db0efe69966b11d8af8e1f7c9c8c2ca9ce03a95. The PR remains open and non-draft. No introduced P1/P2 issues found within this review. Existing reviews, conversation comments, inline comments and threads were empty.

Routed skills: review-comet-pr, review-comet-expression-pr, and review-comet-ffi-pr.

Reviewed the full base-relative comparison. Its four Arrow-writer/test differences originate from base-only commit 5db0efe6 (#6730), which is absent from this branch’s ancestry. They are not regressions introduced by this PR. All 14 files in the actual PR diff were reviewed.

Summary

• Prior state and problem: Native UDF registration allowed unrestricted library loading, and the process-wide cache held its write lock while loading a library. Operators lacked loading controls, and unrelated cache requests could wait behind a slow load.
• Design approach: Introduces shared NativeUdfPolicy checks on the driver and executor, plus per-library cache slots that separate map access from loading.
• Correctness: Both production loading call sites check policy before consulting the cache. Disabled loading also rejects already-cached libraries. Canonical path comparisons, failed-load retries, concurrent requests and poisoned-slot recovery are covered by tests. No introduced correctness issue meeting the P1/P2 bar was identified.
• Compatibility analysis: Checked SQLExecution.withSQLConfPropagated, SQLConf.get and configuration conversion against Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0 sources. Resolved configuration values cross JNI consistently. The latest release branch, branch-1.1 at e9efd9f764ee0a59b7898ff028d6985d4a7a28e1, has no native UDF API, so these controls affect an unreleased feature. Existing registration defaults remain enabled and unrestricted.
• Key design decisions: Policy stays session-scoped, libraries remain loaded for the process lifetime, and aliases share canonical cache entries. Documentation explicitly describes configuration mutability and file-replacement limitations.
• Implementation sketch: CometConf supplies the settings, driver validation passes them through JNI, and executor configuration stores the policy as a DataFusion session extension. NativeScalarUdf planning checks it before loading. The cache fills each slot only after successful loading.
• Performance: The concurrency tests demonstrate that the cache’s global lock no longer spans loading and that concurrent same-library requests share one result. Policy work occurs during registration and planning, with no added per-row checks. No throughput benchmark was run.
• Design: Centralizing policy and limiting the global lock to map operations makes the behavior straightforward to follow. No additional design issue met the reporting bar.
• Abstraction & complexity: NativeUdfPolicy prevents divergent driver/executor rules. Generic Cache<T> enables deterministic concurrency tests without loading native libraries. These abstractions are appropriately scoped.
• Behavioral changes worth calling out: Disabling native UDFs or restricting paths can reject registration and subsequent execution of previously registered functions. Registration reports CometNativeUdfNotAllowedException. Default loading behavior and library lifetime are preserved.
• Suggested improvements: No additional improvement meeting the P1/P2 reporting bar was identified.

Exact-head CI: run 37788596138 succeeded. Reported checks comprise 26 successes and 15 skips. Logs confirm 2,216 Rust tests passed, including policy/cache and planner tests, and all 72 CometNativeUdfSuite cases passed on Spark 4.1. Linux execution, scans, shuffle, TPC-H and TPC-DS checks also passed.

Validation limits: An isolated local harness passed all 14 policy tests and four cache concurrency/recovery tests using the pinned source. It does not exercise JNI or actual dynamic-library loading; exact-head CI provides that integration evidence. No full local JVM/native build or Spark SQL suite was run. Spark SQL suites, macOS, Iceberg and PyArrow jobs were skipped in CI, and runtime coverage of the other supported Spark profiles was unavailable.

[paleolimbot]

A few questions here but this is definitely a problem worth solving and I trust the tests here that suggest it's covered. Thanks!

[paleolimbot]

Just curious: if this is true, is there a point to sandboxing at all? It may be worth documenting why one might set this feature...is it common to have users that can register comet native UDFs that can't set Spark configs?

[paleolimbot]

I trust the test here; however, this reads a whole lot like holding a lock while loading a dynamic library. It may be worth a comment in this section explaining why this is not the case.