Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gh-83004: Clean up refleaks in _decimal initialisation #99043

Closed
wants to merge 5 commits into from
Closed

gh-83004: Clean up refleaks in _decimal initialisation #99043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ae57c47
gh-83004: Clean up refleaks in _decimal initialisation
hauntsaninja Nov 3, 2022
ce6ce75
📜🤖 Added by blurb_it.
blurb-it[bot] Nov 3, 2022
7b8bea9
undo change to obj
hauntsaninja Nov 3, 2022
a7576b3
fix whitespace
hauntsaninja Nov 3, 2022
317fb61
undo change to obj
hauntsaninja Nov 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2022-11-03-05-56-54.gh-issue-83004.kr0Gla.rst
View file
Open in desktop
@@ -0,0 +1 @@
Clean up refleaks on failed module initialisation in in :mod:`_decimal`
47 changes: 17 additions & 30 deletions Modules/_decimal/_decimal.c
View file
Open in desktop
Expand Up @@ -5918,21 +5918,17 @@ PyInit__decimal(void)


/* Add types to the module */
Py_INCREF(&PyDec_Type);
CHECK_INT(PyModule_AddObject(m, "Decimal", (PyObject *)&PyDec_Type));
Py_INCREF(&PyDecContext_Type);
CHECK_INT(PyModule_AddObject(m, "Context",
(PyObject *)&PyDecContext_Type));
Py_INCREF(DecimalTuple);
CHECK_INT(PyModule_AddObject(m, "DecimalTuple", (PyObject *)DecimalTuple));
CHECK_INT(PyModule_AddObjectRef(m, "Decimal", (PyObject *)&PyDec_Type));
CHECK_INT(PyModule_AddObjectRef(m, "Context",
(PyObject *)&PyDecContext_Type));
CHECK_INT(PyModule_AddObjectRef(m, "DecimalTuple", (PyObject *)DecimalTuple));


/* Create top level exception */
ASSIGN_PTR(DecimalException, PyErr_NewException(
"decimal.DecimalException",
PyExc_ArithmeticError, NULL));
Py_INCREF(DecimalException);
CHECK_INT(PyModule_AddObject(m, "DecimalException", DecimalException));
CHECK_INT(PyModule_AddObjectRef(m, "DecimalException", DecimalException));

/* Create signal tuple */
ASSIGN_PTR(SignalTuple, PyTuple_New(SIGNAL_MAP_LEN));
Expand Down Expand Up @@ -5972,8 +5968,7 @@ PyInit__decimal(void)
Py_DECREF(base);

/* add to module */
Py_INCREF(cm->ex);
CHECK_INT(PyModule_AddObject(m, cm->name, cm->ex));
CHECK_INT(PyModule_AddObjectRef(m, cm->name, cm->ex));

/* add to signal tuple */
Py_INCREF(cm->ex);
Expand Down Expand Up @@ -6003,45 +5998,38 @@ PyInit__decimal(void)
ASSIGN_PTR(cm->ex, PyErr_NewException(cm->fqname, base, NULL));
Py_DECREF(base);

Py_INCREF(cm->ex);
CHECK_INT(PyModule_AddObject(m, cm->name, cm->ex));
CHECK_INT(PyModule_AddObjectRef(m, cm->name, cm->ex));
}


/* Init default context template first */
ASSIGN_PTR(default_context_template,
PyObject_CallObject((PyObject *)&PyDecContext_Type, NULL));
Py_INCREF(default_context_template);
CHECK_INT(PyModule_AddObject(m, "DefaultContext",
default_context_template));
CHECK_INT(PyModule_AddObjectRef(m, "DefaultContext",
default_context_template));

#ifndef WITH_DECIMAL_CONTEXTVAR
ASSIGN_PTR(tls_context_key, PyUnicode_FromString("___DECIMAL_CTX__"));
Py_INCREF(Py_False);
CHECK_INT(PyModule_AddObject(m, "HAVE_CONTEXTVAR", Py_False));
CHECK_INT(PyModule_AddObjectRef(m, "HAVE_CONTEXTVAR", Py_False));
#else
ASSIGN_PTR(current_context_var, PyContextVar_New("decimal_context", NULL));
Py_INCREF(Py_True);
CHECK_INT(PyModule_AddObject(m, "HAVE_CONTEXTVAR", Py_True));
CHECK_INT(PyModule_AddObjectRef(m, "HAVE_CONTEXTVAR", Py_True));
#endif
Py_INCREF(Py_True);
CHECK_INT(PyModule_AddObject(m, "HAVE_THREADS", Py_True));
CHECK_INT(PyModule_AddObjectRef(m, "HAVE_THREADS", Py_True));

/* Init basic context template */
ASSIGN_PTR(basic_context_template,
PyObject_CallObject((PyObject *)&PyDecContext_Type, NULL));
init_basic_context(basic_context_template);
Py_INCREF(basic_context_template);
CHECK_INT(PyModule_AddObject(m, "BasicContext",
basic_context_template));
CHECK_INT(PyModule_AddObjectRef(m, "BasicContext",
basic_context_template));

/* Init extended context template */
ASSIGN_PTR(extended_context_template,
PyObject_CallObject((PyObject *)&PyDecContext_Type, NULL));
init_extended_context(extended_context_template);
Py_INCREF(extended_context_template);
CHECK_INT(PyModule_AddObject(m, "ExtendedContext",
extended_context_template));
CHECK_INT(PyModule_AddObjectRef(m, "ExtendedContext",
extended_context_template));


/* Init mpd_ssize_t constants */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't add a comment there because Github, but CHECK_INT(PyModule_AddObject(m, ssize_cm->name, obj)); for obj on L6038 does not leak obj on failure because its reference count is 1 going into PyModule_AddObject and we Py_CLEAR it on goto error inside CHECK_INT.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update the PR to add a comment about that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping on this; there's also a merge conflict.

Expand All @@ -6060,8 +6048,7 @@ PyInit__decimal(void)
/* Init string constants */
for (i = 0; i < _PY_DEC_ROUND_GUARD; i++) {
ASSIGN_PTR(round_map[i], PyUnicode_InternFromString(mpd_round_string[i]));
Py_INCREF(round_map[i]);
CHECK_INT(PyModule_AddObject(m, mpd_round_string[i], round_map[i]));
CHECK_INT(PyModule_AddObjectRef(m, mpd_round_string[i], round_map[i]));
}

/* Add specification version number */
Expand Down