Make SPICE kernel management reference counted

[juan-g-bonilla]

• Tickets addressed: Closes The same SPICE kernels can be loaded many times #1177, closes Flakey SPICE behavior #1023, closes Reference counting issue with SpiceInterface REQUIRED_KERNELS #1010, closes Spice makes BSK simulations not thread safe #220
• Review: By commit
• Merge strategy: Merge (no squash)

Description

This change refactors SPICE kernel lifetime management so that kernels are reference counted per canonical absolute file path instead of being repeatedly loaded and unloaded per simulation.

Key pieces:

• Introduce SpiceKernel, an RAII helper that furnishes a kernel on construction and unloads it on destruction. Instances are cached in a static std::unordered_map<std::string, std::weak_ptr<SpiceKernel>> guarded by a std::mutex, so a given path is only furnished once per process and is unloaded when the last shared pointer goes out of scope.
• Rework SpiceInterface::loadSpiceKernel and SpiceInterface::unloadSpiceKernel to use SpiceKernel::request and a per interface loadedKernels map keyed by canonical absolute path, instead of calling furnsh_c or unload_c directly.
• Deprecate SpiceInterface::clearKeeper in the header and via SWIG (%deprecated_function), documenting that it clears all kernels for the process and should be avoided in favor of automatic per interface cleanup.

Verification

• Added test_multipleInterfaces to construct many small simulations, each with its own SpiceInterface, then verify that:
  • The target kernel is not loaded before use.
  • Loading one simulation loads the kernel.
  • Loading many more simulations does not increase the SPICE kernel count beyond the one sim baseline.
  • After all simulations are out of scope and gc.collect() runs, the kernel is no longer reported as loaded.
• Added test_spiceThreadSafety which:
  • Spawns a configurable number of worker processes (default 50) and iterations per worker.
  • In each worker, repeatedly creates, resets, and destroys SpiceInterface instances against a fixed data path, with a short sleep to increase contention.
  • Wraps the stress test in a controlling process with a hard timeout, collects per worker success and failure counts, and fails the test if any iteration throws or if the run times out.
• Sanity checked that existing SPICE based examples still initialize and run without changes.

Documentation

• Updated bskReleaseNotes.rst .
• Added class and method level documentation comments in spiceInterface.h and spiceInterface.cpp for SpiceKernel, SpiceInterface::clearKeeper, and the new helper functions, to clarify lifetime semantics and deprecation status.
• Marked SpiceInterface::clearKeeper as deprecated in the SWIG interface so Python users will receive a warning and updated guidance.

Future work

These kinds of errors are very hard to catch consistently, so only experience will tell us if this actually addresses all user errors.

[juan-g-bonilla]

Assigned to @Mark2000 because I'll be out for a couple weeks.

[Mark2000]

Looks like the spice thread safety test is failing across various runners.

[juan-g-bonilla]

Looks like the spice thread safety test is failing across various runners.

@Mark2000

I grabbed the test from an old PR from Nico, but instead of only doing two parallel workers I did 50. That's like 150 sims interacting with Spice at the same time. Clearly that's too much, and the mutex protection for loading (or unloading) doesn't really address it.

If this PR performs well in bsk-rl (it doesnt introduce any of the errors you guys were observing with Niko's original PR), then I'd say this is not a complete solution but at least a positive step.

Feel free to make changes to the branch and take over.

[Mark2000]

Sure, so the failing test is to see if the PR is successfully addressing #220, and not reflecting a regression? If so, I think it makes sense to leave the test and mark it as skipped (or flakey) so that there's a reference for the desired but currently unsuccessful behavior.

This PR does improve the performance of BSK-RL considerably, so as long as there are no regressions for other use cases I think we should go with this implementation.