gh-140530: fix a reference leak in an error path for raise exc from cause#140908
gh-140530: fix a reference leak in an error path for raise exc from cause#140908picnixz merged 1 commit intopython:mainfrom
raise exc from cause#140908Conversation
picnixz
force-pushed
the
fix/core/raise-from-140530
branch
from
cec6a51 to
a445dd3
Compare
subtype for which `T.__new__` does not return an exception instance.
picnixz
force-pushed
the
fix/core/raise-from-140530
branch
from
a445dd3 to
adb1bc4
Compare
picnixz
added
the
🔨 test-with-refleak-buildbots
bedevere-bot
removed
the
🔨 test-with-refleak-buildbots
picnixz
added
needs backport to 3.13
| self.fail("No exception raised") | ||
|
|
||
| def test_class_cause_nonexception_result(self): | ||
| class ConstructsNone(BaseException): |
There was a problem hiding this comment.
I think we should keep the old test and add the new test in?
There was a problem hiding this comment.
The old test is actually testing the same code. The problem was that the returned value was immortal...
There was a problem hiding this comment.
FTR:
if (PyExceptionClass_Check(cause)) {
fixed_cause = _PyObject_CallNoArgs(cause);
if (fixed_cause == NULL)
goto raise_error;
if (!PyExceptionInstance_Check(fixed_cause)) {
_PyErr_Format(tstate, PyExc_TypeError,
"calling %R should have returned an instance of "
"BaseException, not %R",
cause, Py_TYPE(fixed_cause));
Py_DECREF(fixed_cause);
goto raise_error;
}
Py_DECREF(cause);
}Since cause is an exception class, we called __new__ and fixed_cause would have been None. We would then move to PyExceptionInstance_Check but since fixed_cause is immortal the refleak didn't appear.
| Py_DECREF(fixed_cause); | ||
| goto raise_error; | ||
| } | ||
| Py_DECREF(cause); |
There was a problem hiding this comment.
Just checking so I'm clear: PyException_SetCause steals a reference right? That's why we're not decrefing fixed_cause here?
There was a problem hiding this comment.
AFAIR, yes. PyException_Set* always steal refs IIRC, but I will check this again (it's always a pain to remember which function steals or borrows)
There was a problem hiding this comment.
Yes, PyException_SetCause steals a reference to cause:
Lines 548 to 551 in 7ae440f
|
Thanks @picnixz for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
… from cause` (pythonGH-140908) Fix a reference leak in `raise E from T` when `T` is an exception subtype for which `T.__new__` does not return an exception instance. (cherry picked from commit 0c77e7c) Co-authored-by: Bénédikt Tran <[email protected]>
|
Sorry, @picnixz, I could not cleanly backport this to |
|
GH-141282 is a backport of this pull request to the 3.14 branch. |
|
GH-141283 is a backport of this pull request to the 3.13 branch. |
…c from cause` (GH-140908) (#141282) gh-140530: fix a reference leak in an error path for `raise exc from cause` (GH-140908) Fix a reference leak in `raise E from T` when `T` is an exception subtype for which `T.__new__` does not return an exception instance. (cherry picked from commit 0c77e7c) Co-authored-by: Bénédikt Tran <[email protected]>
… from cause` (python#140908) Fix a reference leak in `raise E from T` when `T` is an exception subtype for which `T.__new__` does not return an exception instance.
4 participants
Uh oh!
There was an error while loading. Please reload this page.