Properly report exceptions thrown during module initialization.

[anntzer]

If an exception is thrown during module initialization, the
error_already_set destructor will try to call get_internals() after
setting Python's error indicator, resulting in a SystemError: ... returned with an error set.

Fix that by temporarily stashing away the error indicator when
initializing the internals.

(I think any proper unit test would require creating a new module just for that purpose, and I'm not particularly familiar with the testing machinery used by pybind, so guidance would be welcome here. But "it works for me"...)

xref #1113.

[anntzer]

Kindly bumping.

[jagerman]

I'm having trouble reproducing the error; do you have a simple example code that fails?

[jagerman]

Oops, never mind, I see you gave one in the referenced issue.

[jagerman]

I think the error_scope fix here is good, but is in the wrong place--it should instead, I think, be backed up one level, into the error_already_set destructor rather than get_internals. That is, adding the error scope here instead:

error_already_set::~error_already_set() {
    if (type) {
        error_scope scope;
        gil_scoped_acquire gil;
        type.release().dec_ref();
        value.release().dec_ref();
        trace.release().dec_ref();
    }
}

It works with your example in #1113; do you want to test it out some more (and update the PR if it all checks out)?

[anntzer]

It's not totally clear to me why it'd make more sense to stash it in this scope instead, but sure, it works. Force-pushed the fix as suggested.

[jagerman]

It's not totally clear to me why it'd make more sense to stash it in this scope instead, but sure, it works. Force-pushed the fix as suggested.

Well, it seems to me that the issue here isn't that get_internals() is doing something wrong, but rather that error_already_set is setting an error but then making a call that can lead to Python code (via internals).

[anntzer]

Kindly bumping again (comments addressed).