feat(build): support nanobind 3, widening the bound to <4

CHANGELOG.md

@@ -13,6 +13,39 @@ against, and semantic-release manages only the **patch** component.
 
 ## [Unreleased]
 
+### Added
+
+- **nanobind 3 is supported**, and the build requirement widens from
+  `nanobind>=2.0,<3` to `>=2.0,<4`
+  ([#85](https://github.com/stillwater-sc/mtl5-python/issues/85)). The cap added
+  when nanobind 3.0.0 broke every wheel build was a hold, not a fix; this is the
+  port. **Both majors are built and tested** — 1374 passed, 3 skipped under each
+  of nanobind 2.15.0 and 3.0.0.
+
+  nanobind 3 removed the free `nb::detail::keep_alive(nurse, patient)`, routing
+  the same operation through a backend slot that must be handed the extension's
+  context pointer. `mtl5_ndarray.cpp` now spells it once, in `keep_view_alive()`,
+  which selects the form on `NB_VERSION_MAJOR` — so the bound moved rather than
+  jumped, and a source build against either major produces the same behaviour.
+
+  The obvious simplification is wrong, and there is now a test saying so.
+  Replacing these calls with the declarative `nb::keep_alive<0, 1>` annotation
+  would compile and pass the old suite, but the annotation applies to **every**
+  return of a `.def`, and `__getitem__`, `reshape` and `ravel` each return a
+  view on one path and a copy (or, for a fully-integer index, a scalar) on
+  another. Pinning a large source array to an independent copy of itself is a
+  memory-retention bug, so the pin has to stay per-path.
+
+  Two gaps in the lifetime tests are closed as part of this, both of which a
+  broken keep-alive would previously have survived:
+  - `test_a_view_of_an_OWNING_array_keeps_it_alive` — the existing test derives
+    its views from `asarray()`, where the view *also* carries the NumPy owner
+    object, so the buffer survives on that reference alone. A `copy()` owns its
+    memory with no second owner, which is the configuration where dropping the
+    parent is a genuine use-after-free.
+  - `test_a_copy_does_not_pin_its_source` — asserts the aliasing/copying split
+    directly, via refcount: a view increfs its parent, a copy does not.
+
 ### Fixed
 
 - **nanobind capped below 3.0** (`nanobind>=2.0,<3`), which unbreaks every wheel

pyproject.toml

@@ -25,16 +25,15 @@
 # one, is what a wheel build actually resolves, and unbounded it resolved
 # nanobind 3.0.0 the day that released.
 #
-# nanobind 3 is not a drop-in: it moved the internals to a backend-slot dispatch
-# model, so the free function nb::detail::keep_alive(nurse, patient) that
-# mtl5_ndarray.cpp uses to tie a view's lifetime to its owner is gone, replaced
-# by NB_CALL(keep_alive_py)(NB_CTX, ...) which needs a context not available at
-# those call sites. Lifting this cap is a real port, not a version edit; until
-# someone does it, pin the major so a nanobind release cannot retroactively fail
-# every build the way one already has.
+# The bound is now <4, i.e. nanobind 2 AND 3 are supported and tested. It was
+# <3 while mtl5_ndarray.cpp still called the free nb::detail::keep_alive that
+# nanobind 3 removed; keep_view_alive() there now spells that operation for
+# both majors, so the cap moved rather than disappeared. Keep an upper bound:
+# nanobind 3.0.0 broke every build the day it released precisely because there
+# was none.
 requires = [
     "scikit-build-core>=0.10",
-    "nanobind>=2.0,<3",
+    "nanobind>=2.0,<4",
     "numpy>=1.24",
     "packaging>=22",
     "typing_extensions>=4.0; python_version < '3.11'",

python/CMakeLists.txt

@@ -6,7 +6,7 @@ find_package(Python REQUIRED COMPONENTS Interpreter Development.Module)
 # find_package almost always wins under `pip install .`, because pyproject.toml's
 # build requirement puts a nanobind in the build overlay. The FetchContent tag
 # below is the fallback for a bare CMake build with no nanobind installed -- and
-# it is kept in step with the `nanobind>=2.0,<3` bound in pyproject.toml on
+# it is kept in step with the `nanobind>=2.0,<4` bound in pyproject.toml on
 # purpose. When the two drifted (fallback v2.4.0, requirement unbounded), a local
 # CMake build and a CI wheel build compiled against different major versions of
 # nanobind, so a source change could pass locally and fail every CI job. Move
@@ -17,7 +17,7 @@ if(NOT nanobind_FOUND)
     FetchContent_Declare(
         nanobind
         GIT_REPOSITORY https://github.com/wjakob/nanobind.git
-        GIT_TAG        v2.15.0
+        GIT_TAG        v3.0.0
         GIT_SHALLOW    TRUE
     )
     FetchContent_MakeAvailable(nanobind)

python/src/mtl5_ndarray.cpp

@@ -30,9 +30,16 @@
 // builds the view through ndarray's (pointer, shape, strides) constructor,
 // which is the same primitive `slice` is built on.
 //
-// Lifetime: every method returning a view uses nb::keep_alive<0, 1>, so the
-// array it borrows from cannot be collected first. `asarray` additionally holds
-// the NumPy object, which is what keeps the underlying buffer alive.
+// Lifetime: a method that returns a view keeps the array it borrows from alive,
+// so the parent cannot be collected first. `asarray` and the `as_ndarray`
+// converters do that with nb::keep_alive<0, 1>, the declarative form.
+//
+// `__getitem__`, `reshape` and `ravel` cannot: each returns a view on one path
+// and a copy (or, for a fully-integer index, a scalar) on another, while the
+// annotation applies to every return of a `.def`. Pinning a large parent array
+// to an independent copy of itself is a memory-retention bug, so those three
+// call `keep_view_alive` on exactly the aliasing paths. See its comment below --
+// that split is the reason the port to nanobind 3 was not a one-line rename.
 
 #include "mtl5_types.hpp"
 
@@ -58,6 +65,32 @@ namespace {
 
 namespace ma = mtl::array;
 
+/// Tie `patient`'s lifetime to `nurse`: the patient cannot be collected while
+/// the nurse is alive. Used where we hand back a VIEW that aliases another
+/// array's memory, so the buffer cannot be freed out from under it.
+///
+/// Spelled here rather than at the call sites because nanobind moved it. 2.x
+/// exposed a free `nb::detail::keep_alive(nurse, patient)`; 3.x routes the same
+/// operation through a backend slot that has to be handed this extension's
+/// context pointer. `NB_CTX` resolves to that pointer in extension code (it is
+/// only unavailable inside nanobind's own build, under NB_BUILD), so the 3.x
+/// form is usable directly -- it just cannot be spelled the old way.
+///
+/// Both branches are still nanobind-internal API. The alternative is the public
+/// `nb::keep_alive<Nurse, Patient>` annotation, which does NOT fit here: it
+/// applies unconditionally to every return of a `.def`, and all three callers
+/// below return a view on one path and a copy (or a scalar) on another. Pinning
+/// the parent to a copy would be a silent memory-retention regression -- a
+/// large source array held alive by an independent copy of it -- so the
+/// conditional, per-path call is the behaviour to preserve, not an accident.
+inline void keep_view_alive(nb::handle nurse, nb::handle patient) {
+#if defined(NB_VERSION_MAJOR) && NB_VERSION_MAJOR >= 3
+    NB_CALL(keep_alive_py)(NB_CTX, nurse.ptr(), patient.ptr());
+#else
+    nb::detail::keep_alive(nurse.ptr(), patient.ptr());
+#endif
+}
+
 /// An ndarray plus, when it borrows NumPy memory, the object owning it.
 ///
 /// `owner` defaults to a NULL handle rather than `nb::none()`. That matters:
@@ -337,7 +370,7 @@ void register_ndarray(nb::module_& m) {
             }
             NDArrayView<T, M> out(ma::ndarray<T, M>(ptr, sh, st), v.owner);
             nb::object o = nb::cast(std::move(out));
-            nb::detail::keep_alive(o.ptr(), self.ptr());
+            keep_view_alive(o, self);
             return o;
         };
         switch (kept) {
@@ -364,7 +397,7 @@ void register_ndarray(nb::module_& m) {
                 NDArrayView<T, M> out(
                     ma::ndarray<T, M>(const_cast<T*>(v.arr.data()), sh), v.owner);
                 nb::object o = nb::cast(std::move(out));
-                nb::detail::keep_alive(o.ptr(), self.ptr());
+                keep_view_alive(o, self);
                 return o;
             }
             // Not contiguous in its own order: pack into logical order first.
@@ -403,7 +436,7 @@ void register_ndarray(nb::module_& m) {
             NDArrayView<T, 1> out(
                 ma::ndarray<T, 1>(const_cast<T*>(v.arr.data()), sh), v.owner);
             nb::object o = nb::cast(std::move(out));
-            nb::detail::keep_alive(o.ptr(), self.ptr());
+            keep_view_alive(o, self);
             return o;
         }
         ma::shape<1> sh;

tests/test_ndarray.py

@@ -548,6 +548,62 @@ def test_numpy_source_is_kept_alive(self):
         gc.collect()
         np.testing.assert_array_equal(x.to_numpy(), arange(2, 3))
 
+    def test_a_view_of_an_OWNING_array_keeps_it_alive(self):
+        """The source owns its buffer, so only the keep-alive holds it.
+
+        The asarray() test above cannot catch a broken keep-alive: a view
+        derived from an asarray() array also carries the NumPy owner object, so
+        the buffer survives on that reference alone. An array built by copy()
+        owns its memory and has no second owner — if the parent is collected
+        without each view pinning it, every read below is a use-after-free.
+
+        This covers all three sites that pin manually (`__getitem__`, `reshape`,
+        `ravel`); `transpose` is covered by the asarray test and uses the
+        declarative nb::keep_alive<0, 1> instead.
+        """
+        import gc
+
+        base = arange(4, 6)
+        src = A.asarray(base).copy()
+        assert not src.is_view, "copy() must own its buffer for this to test anything"
+
+        sliced = src[1]
+        raveled = src.ravel()
+        reshaped = src.reshape([6, 4])
+
+        del src
+        gc.collect()
+
+        np.testing.assert_array_equal(sliced.to_numpy(), base[1])
+        np.testing.assert_array_equal(raveled.to_numpy(), base.ravel())
+        np.testing.assert_array_equal(reshaped.to_numpy(), base.reshape(6, 4))
+
+    def test_a_copy_does_not_pin_its_source(self):
+        """Only the aliasing paths pin the parent — the copying paths must not.
+
+        `__getitem__`, `reshape` and `ravel` each return a view on one path and
+        a copy on another, and they pin per path rather than per method. The
+        declarative nb::keep_alive<0, 1> annotation cannot express that: it
+        applies to every return, so a large source array would be held alive by
+        an independent copy of itself. This is the test that fails if someone
+        "simplifies" the manual calls into the annotation.
+
+        Refcount is the observable: pinning increfs the parent.
+        """
+        import sys
+
+        src = A.asarray(arange(4, 6)).copy()
+        before = sys.getrefcount(src)
+        view = src.ravel()  # C-contiguous -> view
+        assert view.is_view
+        assert sys.getrefcount(src) == before + 1, "a view must pin its source"
+
+        nc = A.asarray(arange(4, 6)).T  # non-contiguous
+        before_nc = sys.getrefcount(nc)
+        copied = nc.ravel()  # cannot alias -> copy
+        assert not copied.is_view
+        assert sys.getrefcount(nc) == before_nc, "a copy must NOT pin its source"
+
 
 class TestPublicSurface:
     def test_array_submodule_is_exported(self):