Skip to content

Commit

Permalink
incorporate review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
wjakob committed Sep 20, 2024
1 parent 2bdee06 commit 02d190d
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 8 deletions.
2 changes: 2 additions & 0 deletions src/nb_enum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ PyObject *enum_create(enum_init_data *ed) noexcept {
#endif
}

maybe_make_immortal(result.ptr());

result.attr("__nb_enum__") = capsule(t, [](void *p) noexcept {
type_init_data *t = (type_init_data *) p;
delete (enum_map *) t->enum_tbl.fwd;
Expand Down
10 changes: 3 additions & 7 deletions src/nb_func.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,13 +321,9 @@ PyObject *nb_func_new(const void *in_) noexcept {

#if !defined(NB_FREE_THREADED)
// Register the function
{
nb_ptr_map::iterator it;
bool success;
std::tie(it, success) = internals_->funcs.try_emplace(func, nullptr);
check(success,
"nanobind::detail::nb_func_new(): internal update failed (2)!");
}
auto [it, success] = internals_->funcs.try_emplace(func, nullptr);
check(success,
"nanobind::detail::nb_func_new(): internal update failed (2)!");
#endif

func_data *fc = nb_func_data(func) + to_copy;
Expand Down
18 changes: 17 additions & 1 deletion src/nb_type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,23 @@ void nb_type_unregister(type_data *t) noexcept {

#if defined(NB_FREE_THREADED)
// In free-threaded mode, stale type information remains in the
// 'type_c2p_fast' TLS. It is freed when the thread terminates.
// 'type_c2p_fast' TLS. This data structure is eventually deallocated
// when the thread terminates.
//
// In principle, this is dangerous because the user could delete a type
// binding from a module at runtime, causing the associated
// Python type object to be freed. If a function then attempts to return
// a value with such a de-registered type, nanobind should raise an
// exception, which requires knowing that the entry in 'type_c2p_fast'
// has become invalid in the meantime.
//
// Right now, this problem is avoided because we immortalize type objects in
// ``nb_type_new()`` and ``enum_create()``. However, we may not always
// want to stick with immortalization, which is just a workaround.
//
// In the future, a global version counter modified with acquire/release
// semantics (see https://github.com/wjakob/nanobind/pull/695#discussion_r1761600010)
// might prove to be a similarly efficient but more general solution.
bool fail = n_del_slow != 1;
#else
nb_type_map_fast &type_c2p_fast = internals_->type_c2p_fast;
Expand Down

0 comments on commit 02d190d

Please sign in to comment.