Skip to content

Commit aa7f77e

Browse files
committed
Merge sudo_module_register_loghandler and sudo_module_set_default_loghandler.
We now create the LogHandler class for each interpreter in python_plugin_init() instead of just once in sudo_module_init(). This fixes the crash seen in Py_EndInterpreter() with Python 3.12 and significantly reduces the number of leaked objects tracked by MemorySanitizer. --HG-- branch : 1.9
1 parent 32ac7d4 commit aa7f77e

4 files changed

Lines changed: 28 additions & 51 deletions

File tree

plugins/python/python_loghandler.c

Lines changed: 26 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@
2727
# define PyObject_CallNoArgs(_o) PyObject_CallObject((_o), NULL)
2828
#endif
2929

30-
static PyObject *sudo_type_LogHandler;
31-
3230
static void
3331
_debug_plugin(int log_level, const char *log_message)
3432
{
@@ -127,74 +125,58 @@ static PyMethodDef _sudo_LogHandler_class_methods[] =
127125
{NULL, NULL, 0, NULL}
128126
};
129127

130-
// This function registers sudo.LogHandler class
128+
// This function creates the sudo.LogHandler class and adds it
129+
// to the root logger.
131130
int
132-
sudo_module_register_loghandler(PyObject *py_module)
131+
sudo_module_set_default_loghandler()
133132
{
134133
debug_decl(sudo_module_register_loghandler, PYTHON_DEBUG_INTERNAL);
135134

136-
PyObject *py_logging_module = NULL, *py_streamhandler = NULL;
135+
PyObject *py_sudo, *py_logging_module = NULL, *py_logger = NULL,
136+
*py_streamhandler = NULL, *py_class = NULL,
137+
*py_loghandler = NULL, *py_result = NULL;
138+
139+
py_sudo = PyImport_ImportModule("sudo");
140+
if (py_sudo == NULL)
141+
goto cleanup;
137142

138143
py_logging_module = PyImport_ImportModule("logging");
139144
if (py_logging_module == NULL)
140145
goto cleanup;
141146

147+
// Get the root logger which all loggers descend from.
148+
py_logger = PyObject_CallMethod(py_logging_module, "getLogger", NULL);
149+
if (py_logger == NULL)
150+
goto cleanup;
151+
142152
py_streamhandler = PyObject_GetAttrString(py_logging_module, "StreamHandler");
143153
if (py_streamhandler == NULL)
144154
goto cleanup;
145155

146-
sudo_type_LogHandler = sudo_module_create_class("sudo.LogHandler",
156+
// Create our own handler that is a sub-class of StreamHandler
157+
py_class = sudo_module_create_class("sudo.LogHandler",
147158
_sudo_LogHandler_class_methods, py_streamhandler);
148-
if (sudo_type_LogHandler == NULL)
159+
if (py_class == NULL)
149160
goto cleanup;
150161

151-
if (PyModule_AddObject(py_module, "LogHandler", sudo_type_LogHandler) < 0) {
152-
Py_CLEAR(sudo_type_LogHandler);
162+
// PyModule_AddObject steals a reference to py_class on success
163+
if (PyModule_AddObject(py_sudo, "LogHandler", py_class) < 0)
153164
goto cleanup;
154-
}
155-
156-
// PyModule_AddObject steals a reference to sudo_type_LogHandler on success
157-
Py_INCREF(sudo_type_LogHandler);
158-
159-
cleanup:
160-
Py_CLEAR(py_streamhandler);
161-
Py_CLEAR(py_logging_module);
162-
debug_return_int(PyErr_Occurred() ? SUDO_RC_ERROR : SUDO_RC_OK);
163-
}
164-
165-
// This sets sudo.LogHandler as the default log handler:
166-
// logging.getLogger().addHandler(sudo.LogHandler())
167-
int
168-
sudo_module_set_default_loghandler(void)
169-
{
170-
debug_decl(sudo_module_set_default_loghandler, PYTHON_DEBUG_INTERNAL);
165+
Py_INCREF(py_class);
171166

172-
PyObject *py_loghandler = NULL, *py_logging_module = NULL,
173-
*py_logger = NULL, *py_result = NULL;
174-
175-
py_loghandler = PyObject_CallNoArgs(sudo_type_LogHandler);
167+
py_loghandler = PyObject_CallNoArgs(py_class);
176168
if (py_loghandler == NULL)
177169
goto cleanup;
178170

179-
py_logging_module = PyImport_ImportModule("logging");
180-
if (py_logging_module == NULL)
181-
goto cleanup;
182-
183-
py_logger = PyObject_CallMethod(py_logging_module, "getLogger", NULL);
184-
if (py_logger == NULL)
185-
goto cleanup;
186-
187171
py_result = PyObject_CallMethod(py_logger, "addHandler", "O", py_loghandler);
188172

189173
cleanup:
190174
Py_CLEAR(py_result);
175+
Py_CLEAR(py_loghandler);
176+
Py_CLEAR(py_class);
177+
Py_CLEAR(py_streamhandler);
191178
Py_CLEAR(py_logger);
192179
Py_CLEAR(py_logging_module);
193-
#if 0
194-
// XXX - If we don't leak py_loghandler here we may get a crash in
195-
// Py_EndInterpreter() on Python 3.12.
196-
Py_CLEAR(py_loghandler);
197-
#endif
198-
180+
Py_CLEAR(py_sudo);
199181
debug_return_int(PyErr_Occurred() ? SUDO_RC_ERROR : SUDO_RC_OK);
200182
}

plugins/python/python_plugin_common.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,8 +532,9 @@ python_plugin_init(struct PluginContext *plugin_ctx, char * const plugin_options
532532
}
533533
PyThreadState_Swap(plugin_ctx->py_interpreter);
534534

535-
if (sudo_module_set_default_loghandler() < 0)
535+
if (sudo_module_set_default_loghandler() != SUDO_RC_OK) {
536536
goto cleanup;
537+
}
537538

538539
if (_python_plugin_set_path(plugin_ctx, _lookup_value(plugin_options, "ModulePath")) != SUDO_RC_OK) {
539540
goto cleanup;

plugins/python/sudo_python_module.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -595,9 +595,6 @@ sudo_module_init(void)
595595
if (sudo_module_register_baseplugin(py_module) != SUDO_RC_OK)
596596
goto cleanup;
597597

598-
if (sudo_module_register_loghandler(py_module) != SUDO_RC_OK)
599-
goto cleanup;
600-
601598
cleanup:
602599
if (PyErr_Occurred()) {
603600
Py_CLEAR(py_module);

plugins/python/sudo_python_module.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,6 @@ int sudo_module_ConvMessages_to_c(PyObject *py_tuple, Py_ssize_t *num_msgs, stru
4545
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION
4646
int sudo_module_register_baseplugin(PyObject *py_module);
4747

48-
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION
49-
int sudo_module_register_loghandler(PyObject *py_module);
50-
5148
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION
5249
int sudo_module_set_default_loghandler(void);
5350

0 commit comments

Comments
 (0)