gh-140406: Fix memory leak upon __hash__ returning a non-integer#140411
gh-140406: Fix memory leak upon __hash__ returning a non-integer#140411ZeroIntensity merged 5 commits intopython:mainfrom
__hash__ returning a non-integer#140411Conversation
Lib/test/test_hash.py
Outdated
| return 1.0 | ||
|
|
||
| for _ in range(100): | ||
| with self.assertRaises(TypeError): |
There was a problem hiding this comment.
The ref leak checker should catch it even in one iteration, is 100 needed here?
There was a problem hiding this comment.
It's a very tiny leak, so I wasn't sure if it would be caught. I don't mind removing it.
There was a problem hiding this comment.
test_hash in test_builtin is also a good place. It already contains similar tests (TypeErrors and classes with custom __hash__).
There was a problem hiding this comment.
Good idea, I moved it to test_builtin.
serhiy-storchaka
left a comment
There was a problem hiding this comment.
I just created a similar patch, but you were the first.
Lib/test/test_hash.py
Outdated
| return 1.0 | ||
|
|
||
| for _ in range(100): | ||
| with self.assertRaises(TypeError): |
There was a problem hiding this comment.
test_hash in test_builtin is also a good place. It already contains similar tests (TypeErrors and classes with custom __hash__).
| return PyObject_HashNotImplemented(self); | ||
| } | ||
| if (!PyLong_Check(res)) { | ||
| Py_DECREF(res); |
There was a problem hiding this comment.
I suggest to move this line below PyErr_SetString(). Later, it can be used to format a better error message.
There was a problem hiding this comment.
I think we should do that in a different PR, where we change the error message. I generally like to call Py_DECREF before PyErr* functions to protect against badly crafted destructors that break the exception state. But, if you feel strongly about this, I'm also fine with moving it.
There was a problem hiding this comment.
I do not feel strongly about this. It would just make the future diff a little bit smaller, and git blame would show your commit instead of the following commit.
In general, I prefer to call Py_DECREF before PyErr* functions too.
Sorry! I'd have let you go for it if I had known you were working on it. |
|
Thanks @ZeroIntensity for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
…ger (pythonGH-140411) (cherry picked from commit 71db05a) Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
|
Sorry, @ZeroIntensity, I could not cleanly backport this to |
|
GH-140417 is a backport of this pull request to the 3.14 branch. |
|
|
GH-140441 is a backport of this pull request to the 3.13 branch. |
…ger (pythonGH-140411) (cherry picked from commit 71db05a)
…on-integer (pythonGH-140411) (cherry picked from commit 71db05a) Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
object.__hash__returns a non-intvalue #140406