fix: subinterpreter exception handling SEGFAULT

[justend29]

Description

pybind11 version: b67d07e

The destructor of subinterpreter_scoped_activate tests for std::uncaught_exceptions to throw std::current_exception. These are two independent states that are unrelated to each other. As such, the destructor can easily SEGFAULT when throwing a nullptr from std::current_exception(). The following example causes a SEGFAULT on std::rethrow_exception, here

#include <pybind11/embed.h>
#include <pybind11/subinterpreter.h>

int main()
{
  namespace py = pybind11;
  try {
    const py::scoped_interpreter python{};
    const py::scoped_subinterpreter subpy{};
    throw "ahhh";
  }
  catch (...) {
    // try-catch avoids immediately terminating at 'throw' from main
  }
}

Explaining the issue:

The subinterpreter_scoped_activate inside scoped_subinterpreter is being destroyed from stack unwinding. Therefore, std::uncaught_exceptions is 1, since this merely counts the number of in-flight exceptions. The current destructor would consequently try to throw std::current_exception(). However, std::current_exception() returns nullptr, as the destructor is not called from a catch block, where std::current_exception() returns the currently handled exception of the enclosing catch block - for which there is none.

Changes

The original code intends to halt stack unwinding of any in-flight exception that happens to be a py::error_already_set,
mitigating the user error of letting error_already_set exceptions escape an active subinterpreter. As halting stack unwinding without a surrounding try-catch is not possible, the invalid checks were removed.

Suggested changelog entry:

• Subinterpreter-specific exception handling code was removed to resolve segfaults.

[rwgk]

@b-pass could you please help reviewing this PR?

[b-pass]

The intention of the original code seems to be halting the stack unwinding of any in-flight exception that happens to be a py::error_already_set.

That was the intent, yes.

I don't believe that's possible.

The need to prevent allowing error_already_set to propagate is already documented. The code here was an attempt to be a little more helpful if it happens,, but if it's not going to work then I think this code should just be removed.

[justend29]

@b-pass I've updated the PR to remove all in-flight exception handling instead.

[b-pass]

Looks good to me

[rwgk]

Thanks @b-pass !

@justend29 Could you please review and maybe revise the PR description? (The Changes section seem to be out of date?)

[rwgk]

@henryiii I'm thinking the CIBW / iOS wheel macos-latest failure is unrelated. WDYT?

[henryiii]

Yes, see pypa/cibuildwheel#2556 (comment)

[rwgk]

Yes, see pypa/cibuildwheel#2556 (comment)

Thanks @henryiii for confirming!

[justend29]

@rwgk Done. Thanks for the diligence.