Fix memory leak in PyC_ExceptionBuffer for Py 3.12

Non-matching calls to PyErr_Fetch/Restore cause a leak in v3.12,
so ensure calls are symmetrical or avoid where possible.

Simplify extraction of the exception buffer.

- Only overwrite the stderr (the stdio isn't used).
- Simplify pyc_exception_buffer_handle_system_exit usage.
- Remove goto's.

Also simplify calling conventions for PyC_ExceptionBuffer functions.
- They must be called when an error has occurred.
- Always return a string, never null since a null return value would
  only happened in rare/unexpected cases which wasn't being checked
  for by some callers, leading to potential crashes.
This commit is contained in:
Campbell Barton 2023-09-18 22:09:19 +10:00
parent 19b9ee7f74
commit 6a0f98aeef
5 changed files with 70 additions and 92 deletions

View File

@ -836,11 +836,9 @@ void PyC_Err_PrintWithFunc(PyObject *py_func)
/** \name Exception Buffer Access
* \{ */
static void pyc_exception_buffer_handle_system_exit(PyObject *error_type,
PyObject *error_value,
PyObject *error_traceback)
static void pyc_exception_buffer_handle_system_exit()
{
if (!PyErr_GivenExceptionMatches(error_type, PyExc_SystemExit)) {
if (!PyErr_ExceptionMatches(PyExc_SystemExit)) {
return;
}
/* Inspecting, follow Python's logic in #_Py_HandleSystemExit & treat as a regular exception. */
@ -862,125 +860,98 @@ static void pyc_exception_buffer_handle_system_exit(PyObject *error_type,
* Especially since this exception more likely to be used for background/batch-processing
* utilities where exiting immediately makes sense, the possibility of this being called
* indirectly from python-drivers or modal-operators is less of a concern. */
PyErr_Restore(error_type, error_value, error_traceback);
PyErr_Print();
}
PyObject *PyC_ExceptionBuffer()
{
PyObject *stdout_backup = PySys_GetObject("stdout"); /* borrowed */
PyObject *stderr_backup = PySys_GetObject("stderr"); /* borrowed */
PyObject *string_io = nullptr;
PyObject *string_io_buf = nullptr;
PyObject *string_io_mod = nullptr;
PyObject *string_io_getvalue = nullptr;
BLI_assert(PyErr_Occurred());
pyc_exception_buffer_handle_system_exit();
/* The resulting exception as a string (return value). */
PyObject *result = nullptr;
PyObject *error_type, *error_value, *error_traceback;
if (!PyErr_Occurred()) {
return nullptr;
}
PyErr_Fetch(&error_type, &error_value, &error_traceback);
pyc_exception_buffer_handle_system_exit(error_type, error_value, error_traceback);
/* `io.StringIO()`. */
PyObject *string_io = nullptr;
PyObject *string_io_mod = nullptr;
PyObject *string_io_getvalue = nullptr;
if ((string_io_mod = PyImport_ImportModule("io")) &&
(string_io = PyObject_CallMethod(string_io_mod, "StringIO", nullptr)) &&
(string_io_getvalue = PyObject_GetAttrString(string_io, "getvalue")))
{
PyObject *stderr_backup = PySys_GetObject("stderr"); /* Borrowed. */
/* Since these were borrowed we don't want them freed when replaced. */
Py_INCREF(stderr_backup);
PySys_SetObject("stderr", string_io); /* Freed when overwriting. */
/* import io
* string_io = io.StringIO()
*/
PyErr_Display(error_type, error_value, error_traceback);
if (!(string_io_mod = PyImport_ImportModule("io"))) {
goto error_cleanup;
result = PyObject_CallObject(string_io_getvalue, nullptr);
PySys_SetObject("stderr", stderr_backup);
Py_DECREF(stderr_backup); /* Now `sys` owns the reference again. */
}
else if (!(string_io = PyObject_CallMethod(string_io_mod, "StringIO", nullptr))) {
goto error_cleanup;
}
else if (!(string_io_getvalue = PyObject_GetAttrString(string_io, "getvalue"))) {
goto error_cleanup;
else {
PySys_WriteStderr("Internal error creating: io.StringIO()!\n");
if (UNLIKELY(PyErr_Occurred())) {
PyErr_Print(); /* Show the error accessing `io.StringIO`. */
}
PyErr_Display(error_type, error_value, error_traceback);
}
/* Since these were borrowed we don't want them freed when replaced. */
Py_INCREF(stdout_backup);
Py_INCREF(stderr_backup);
/* Both of these are freed when restoring. */
PySys_SetObject("stdout", string_io);
PySys_SetObject("stderr", string_io);
PyErr_Restore(error_type, error_value, error_traceback);
/* Printing clears (call #PyErr_Clear as well to ensure it's cleared). */
Py_XINCREF(error_type);
Py_XINCREF(error_value);
Py_XINCREF(error_traceback);
PyErr_Print(); /* print the error */
PyErr_Clear();
string_io_buf = PyObject_CallObject(string_io_getvalue, nullptr);
PySys_SetObject("stdout", stdout_backup);
PySys_SetObject("stderr", stderr_backup);
Py_DECREF(stdout_backup); /* Now `sys` owns the reference again. */
Py_DECREF(stderr_backup);
Py_DECREF(string_io_mod);
Py_DECREF(string_io_getvalue);
Py_DECREF(string_io); /* free the original reference */
PyErr_Restore(error_type, error_value, error_traceback);
return string_io_buf;
error_cleanup:
/* Could not import the module so print the error and close. */
Py_XDECREF(string_io_mod);
Py_XDECREF(string_io);
Py_XDECREF(string_io_getvalue);
Py_XDECREF(string_io); /* Free the original reference. */
if (result == nullptr) {
result = PyObject_Str(error_value);
/* Python does this too. */
if (UNLIKELY(result == nullptr)) {
result = PyUnicode_FromFormat("<unprintable %s object>", Py_TYPE(error_value)->tp_name);
}
}
PyErr_Restore(error_type, error_value, error_traceback);
PyErr_Print(); /* print the error */
PyErr_Restore(error_type, error_value, error_traceback);
return nullptr;
return result;
}
PyObject *PyC_ExceptionBuffer_Simple()
{
if (!PyErr_Occurred()) {
return nullptr;
}
BLI_assert(PyErr_Occurred());
PyObject *string_io_buf = nullptr;
pyc_exception_buffer_handle_system_exit();
/* The resulting exception as a string (return value). */
PyObject *result = nullptr;
PyObject *error_type, *error_value, *error_traceback;
PyErr_Fetch(&error_type, &error_value, &error_traceback);
/* Since #PyErr_Print is not called it's not essential that `SystemExit` exceptions are handled.
* Do this to match the behavior of #PyC_ExceptionBuffer since requesting a brief exception
* shouldn't result in completely different behavior. */
pyc_exception_buffer_handle_system_exit(error_type, error_value, error_traceback);
if (PyErr_GivenExceptionMatches(error_type, PyExc_SyntaxError)) {
/* Special exception for syntax errors,
* in these cases the full error is verbose and not very useful,
* just use the initial text so we know what the error is. */
if (PyTuple_CheckExact(error_value) && PyTuple_GET_SIZE(error_value) >= 1) {
string_io_buf = PyObject_Str(PyTuple_GET_ITEM(error_value, 0));
result = PyObject_Str(PyTuple_GET_ITEM(error_value, 0));
}
}
if (string_io_buf == nullptr) {
string_io_buf = PyObject_Str(error_value);
}
/* Python does this too */
if (UNLIKELY(string_io_buf == nullptr)) {
string_io_buf = PyUnicode_FromFormat("<unprintable %s object>", Py_TYPE(error_value)->tp_name);
if (result == nullptr) {
result = PyObject_Str(error_value);
/* Python does this too. */
if (UNLIKELY(result == nullptr)) {
result = PyUnicode_FromFormat("<unprintable %s object>", Py_TYPE(error_value)->tp_name);
}
}
PyErr_Restore(error_type, error_value, error_traceback);
return string_io_buf;
return result;
}
/** \} */

View File

@ -10,6 +10,7 @@
#ifndef __PY_CAPI_UTILS_H__
#define __PY_CAPI_UTILS_H__
#include "BLI_compiler_attrs.h"
#include "BLI_sys_types.h"
#include "BLI_utildefines_variadic.h"
@ -26,8 +27,18 @@ void PyC_ObSpit(const char *name, PyObject *var);
void PyC_ObSpitStr(char *result, size_t result_maxncpy, PyObject *var);
void PyC_LineSpit(void);
void PyC_StackSpit(void);
PyObject *PyC_ExceptionBuffer(void);
PyObject *PyC_ExceptionBuffer_Simple(void);
/**
* Return a string containing the full stack trace.
* - Only call when `PyErr_Occurred() != 0` .
* - The always returns a Python string.
*/
PyObject *PyC_ExceptionBuffer(void) ATTR_WARN_UNUSED_RESULT ATTR_RETURNS_NONNULL;
/**
* A version of #PyC_ExceptionBuffer that returns the last exception only.
*/
PyObject *PyC_ExceptionBuffer_Simple(void) ATTR_WARN_UNUSED_RESULT ATTR_RETURNS_NONNULL;
PyObject *PyC_Object_GetAttrStringArgs(PyObject *o, Py_ssize_t n, ...);
PyObject *PyC_FrozenSetFromStrings(const char **strings);

View File

@ -66,10 +66,6 @@ bool BPy_errors_to_report_ex(ReportList *reports,
}
PyObject *err_str_py = use_full ? PyC_ExceptionBuffer() : PyC_ExceptionBuffer_Simple();
if (err_str_py == nullptr) {
BKE_report(reports, RPT_ERROR, "Unknown py-exception, could not convert");
return false;
}
/* Strip trailing newlines so the report doesn't show a blank-line in the info space. */
Py_ssize_t err_str_len;

View File

@ -319,6 +319,8 @@ bool BPY_run_string_exec(bContext *C, const char *imports[], const char *expr)
static void run_string_handle_error(BPy_RunErrInfo *err_info)
{
BLI_assert(PyErr_Occurred());
if (err_info == nullptr) {
PyErr_Print();
PyErr_Clear();
@ -333,7 +335,7 @@ static void run_string_handle_error(BPy_RunErrInfo *err_info)
PyObject *py_err_str = err_info->use_single_line_error ? PyC_ExceptionBuffer_Simple() :
PyC_ExceptionBuffer();
const char *err_str = py_err_str ? PyUnicode_AsUTF8(py_err_str) : "Unable to extract exception";
const char *err_str = PyUnicode_AsUTF8(py_err_str);
PyErr_Clear();
if (err_info->reports != nullptr) {

View File

@ -218,8 +218,6 @@ bool python_script_error_jump(
}
}
else {
BLI_assert((tb == PySys_GetObject("last_traceback")) ||
(tb == nullptr && PySys_GetObject("last_traceback") == Py_None));
for (PyTracebackObject *tb_iter = (PyTracebackObject *)tb;
tb_iter && (PyObject *)tb_iter != Py_None;
tb_iter = tb_iter->tb_next)