[BUG]: instance relinquished to unique_ptr can be reinitialized from Python, leading to state confusion

[oremanj]

Problem description

The nb_inst::ready flag being false is used both to mark an instance that hasn't been constructed yet, and to mark an instance that has had its innards handed over to a C++ unique_ptr. This double use means that a hollowed-out instance (where a C++ unique_ptr assumes ownership of the innards) can be reinitialized by calling __init__ on it from Python. This will call the C++ constructor on an already-constructed object, potentially causing resource leaks or other confusion for the C++ code that thought it had exclusive access. It can also produce an assertion failure when trying to pass the C++ unique_ptr back to Python, as shown below.

Potential solution: expand nb_inst::ready to a 2-bit field (and maybe rename it). 0 = not constructed, 1 = relinquished, 2 = normal. Change the test in nb_type_get from the current

            if (NB_UNLIKELY(((flags & (uint8_t) cast_flags::construct) != 0) == (bool) inst->ready)) {

to

            if (NB_UNLIKELY(((flags & (uint8_t) cast_flags::construct) ^ inst->ready) != 2)) {

relying on the fact that cast_flags::construct has numeric value 2.

Of course it is also possible to do this less cryptically by adding another flag relinquished, but I couldn't figure out how to make that equivalent-cost to the current test.

Reproducible example code

~/nanobind/tests% python3.12
Python 3.12.0 (v3.12.0:0fb18b02c8, Oct  2 2023, 09:45:56) [Clang 13.0.0 (clang-1300.0.29.30)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import test_holders_ext as t
>>> x = t.unique_from_cpp()
>>> x.value
1
>>> w = t.UniqueWrapper(x)
>>> x.value
<stdin>:1: RuntimeWarning: nanobind: attempted to access an uninitialized instance of type 'test_holders_ext.Example'!

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: (): incompatible function arguments. The following argument types are supported:
    1. (self) -> int

Invoked with types: test_holders_ext.Example
>>> x.__init__(3)
>>> x.value
3
>>> y = w.get()
Critical nanobind error: nanobind::detail::nb_type_put_unique(type='Example', cpp_delete=1): unexpected status flags! (ready=1, destruct=1, cpp_delete=0)
zsh: abort      python

[wjakob]

I see how this could be dangerous. I would be open to a fix by expanding the instance state to a two-bit value. But in that case, it should probably be named state as opposed to ready.

[wjakob]

@oremanj do you plan to make further changes here or shall I close the issue?

[oremanj]

Whoops, I missed your reply a few weeks ago. I will try and get a PR up for this this week.