Skip to content

Commit

Permalink
Fix an intermittent error during process termination on GCC/Linux/lib…
Browse files Browse the repository at this point in the history
…stdc++.
  • Loading branch information
jamadden committed Sep 12, 2023
1 parent cec93b1 commit 748a9e0
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 17 deletions.
3 changes: 2 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
3.0.0rc3 (unreleased)
=====================

- Nothing changed yet.
- Fix an intermittent error during process termination on some
platforms (GCC/Linux/libstdc++).


3.0.0rc2 (2023-09-09)
Expand Down
13 changes: 9 additions & 4 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,25 @@
cpp_compile_args.append("--std=gnu++11")
elif is_win and "MSC" in plat_compiler:
# Older versions of MSVC (Python 2.7) don't handle C++ exceptions
# correctly by default. While newer versions do handle exceptions by default,
# they don't do it fully correctly. So we need an argument on all versions.
# correctly by default. While newer versions do handle exceptions
# by default, they don't do it fully correctly ("By default....the
# compiler generates code that only partially supports C++
# exceptions."). So we need an argument on all versions.

#"/EH" == exception handling.
# "s" == standard C++,
# "c" == extern C functions don't throw
# OR
# "a" == standard C++, and Windows SEH; anything may throw, compiler optimizations
# around try blocks are less aggressive.
# around try blocks are less aggressive. Because this catches SEH,
# which Windows uses internally, the MS docs say this can be a security issue.
# DO NOT USE.
# /EHsc is suggested, and /EHa isn't supposed to be linked to other things not built
# with it. Leaving off the "c" should just result in slower, safer code.
# Other options:
# "r" == Always generate standard confirming checks for noexcept blocks, terminating
# if violated. IMPORTANT: We rely on this.
# See https://docs.microsoft.com/en-us/cpp/build/reference/eh-exception-handling-model?view=msvc-160
# See https://docs.microsoft.com/en-us/cpp/build/reference/eh-exception-handling-model?view=msvc-170
handler = "/EHsr"
cpp_compile_args.append(handler)
# To disable most optimizations:
Expand Down
38 changes: 31 additions & 7 deletions src/greenlet/TUserGreenlet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ UserGreenlet::g_initialstub(void* mark)
constructed for the second time until the switch actually happens.
*/
if (err.status == 1) {
// In the new greenlet.

// This never returns! Calling inner_bootstrap steals
// the contents of our run object within this stack frame, so
// it is not valid to do anything with it.
Expand All @@ -314,16 +316,38 @@ UserGreenlet::g_initialstub(void* mark)
// C++ extension. We're going to abort anyway, but try to
// display some nice information if possible.
//
// The catching is tested by ``test_cpp.CPPTests.test_unhandled_exception_in_greenlet_aborts``.
// The catching is tested by
// ``test_cpp.CPPTests.test_unhandled_exception_in_greenlet_aborts``.
//
// PyErrOccurred can theoretically be thrown by
// inner_bootstrap() -> g_switch_finish(), but that should
// never make it back to here. It is a std::exception and
// would be caught if it is.
catch (const std::exception& e) {
std::string base = "greenlet: Unhandled C++ exception: ";
base += e.what();
Py_FatalError(base.c_str());
}
catch (...) {
Py_FatalError("greenlet: inner_bootstrap terminated with unknown C++ exception\n");
// Some compilers/runtimes use exceptions internally.
// It appears that GCC on Linux with libstdc++ throws an
// exception internally at process shutdown time to unwind
// stacks and clean up resources. Depending on exactly
// where we are when the process exits, that could result
// in an unknown exception getting here. If we
// Py_FatalError() or abort() here, we interfere with
// orderly process shutdown. Throwing the exception on up
// is the right thing to do.
//
// gevent's ``examples/dns_mass_resolve.py`` demonstrates this.
#ifndef NDEBUG
fprintf(stderr,
"greenlet: inner_bootstrap threw unknown exception; "
"is the process terminating?\n");
#endif
throw;
}
Py_FatalError("greenlet: inner_bootstrap returned\n");
Py_FatalError("greenlet: inner_bootstrap returned with no exception.\n");
}


Expand Down Expand Up @@ -354,7 +378,7 @@ UserGreenlet::g_initialstub(void* mark)


void
GREENLET_NOINLINE(UserGreenlet::inner_bootstrap)(PyGreenlet* origin_greenlet, PyObject* run)
UserGreenlet::inner_bootstrap(PyGreenlet* origin_greenlet, PyObject* run)
{
// The arguments here would be another great place for move.
// As it is, we take them as a reference so that when we clear
Expand Down Expand Up @@ -435,7 +459,7 @@ GREENLET_NOINLINE(UserGreenlet::inner_bootstrap)(PyGreenlet* origin_greenlet, Py
// GC, which may run arbitrary Python code.
result = OwnedObject::consuming(PyObject_Call(run, args.args().borrow(), args.kwargs().borrow()));
}
catch(...) {
catch (...) {
// Unhandled C++ exception!

// If we declare ourselves as noexcept, if we don't catch
Expand Down Expand Up @@ -481,7 +505,7 @@ GREENLET_NOINLINE(UserGreenlet::inner_bootstrap)(PyGreenlet* origin_greenlet, Py
#endif
}
}
// These lines may run arbitrary code
// These lines may run arbitrary code
args.CLEAR();
Py_CLEAR(run);

Expand Down Expand Up @@ -525,7 +549,7 @@ GREENLET_NOINLINE(UserGreenlet::inner_bootstrap)(PyGreenlet* origin_greenlet, Py
result = parent->g_switch();
}
catch (const PyErrOccurred&) {
// Ignore.
// Ignore, keep passing the error on up.
}

/* Return here means switch to parent failed,
Expand Down
14 changes: 10 additions & 4 deletions src/greenlet/greenlet_exceptions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace greenlet {
public:

// CAUTION: In debug builds, may run arbitrary Python code.
static PyErrOccurred
static const PyErrOccurred
from_current()
{
assert(PyErr_Occurred());
Expand All @@ -35,12 +35,18 @@ namespace greenlet {
PyObject* tb;

PyErr_Fetch(&typ, &val, &tb);
PyObject* s = PyObject_Str(PyErr_Occurred());
const char* msg = PyUnicode_AsUTF8(s);
PyObject* typs = PyObject_Str(typ);
PyObject* vals = PyObject_Str(val ? val : typ);
const char* typ_msg = PyUnicode_AsUTF8(typs);
const char* val_msg = PyUnicode_AsUTF8(vals);
PyErr_Restore(typ, val, tb);

std::string msg(typ_msg);
msg += ": ";
msg += val_msg;
PyErrOccurred ex(msg);
Py_XDECREF(s);
Py_XDECREF(typs);
Py_XDECREF(vals);

return ex;
#else
Expand Down
3 changes: 2 additions & 1 deletion src/greenlet/greenlet_greenlet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -657,9 +657,10 @@ class TracingGuard
protected:
virtual switchstack_result_t g_initialstub(void* mark);
private:
// This function isn't meant to return.
// This accepts raw pointers and the ownership of them at the
// same time. The caller should use ``inner_bootstrap(origin.relinquish_ownership())``.
void GREENLET_NOINLINE(inner_bootstrap)(PyGreenlet* origin_greenlet, PyObject* run);
void inner_bootstrap(PyGreenlet* origin_greenlet, PyObject* run);
};

class BrokenGreenlet : public UserGreenlet
Expand Down

0 comments on commit 748a9e0

Please sign in to comment.