gh-83004: Clean up refleaks in ssl, socket initialisation - #99044
gh-83004: Clean up refleaks in ssl, socket initialisation#99044hauntsaninja wants to merge 2 commits into
Conversation
| PyObject *bool_obj = (value) ? Py_True : Py_False; \ | ||
| Py_INCREF(bool_obj); \ | ||
| PyModule_AddObject((m), (key), bool_obj); \ | ||
| PyModule_AddObjectRef((m), (key), bool_obj); \ |
There was a problem hiding this comment.
Isn't this still a leak because we're not checking the return value?
Sorry, something went wrong.
There was a problem hiding this comment.
It's no longer a leak.
The thing that makes PyModule_AddObject tricky cause leaks is that you have to decref in the failure case, but not in the success case (because it steals the ref). With PyModule_AddObjectRef you can treat the failure and success cases the same in terms of what they do to ref counts.
But yes, in general we should probably be propagating failures here, and the other cases below you commented on. I didn't do it in this PR because it's a slightly separate change and there are many other places in this module where we don't check return values.
Sorry, something went wrong.
There was a problem hiding this comment.
Should I check return values for just the ones I'm touching in this PR or fix all of them?
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for the explanation! I feel it'd be fine to also fix missing error checks in this PR, since the change remains localized and you're fixing a very similar problem to what the PR already fixes.
Sorry, something went wrong.
| #endif | ||
| Py_INCREF(has_ipv6); | ||
| PyModule_AddObject(m, "has_ipv6", has_ipv6); | ||
| PyModule_AddObjectRef(m, "has_ipv6", has_ipv6); |
There was a problem hiding this comment.
Check the return value.
Sorry, something went wrong.
| if (tmp == NULL) | ||
| return NULL; | ||
| PyModule_AddObject(m, names[i], tmp); | ||
| PyModule_AddObjectRef(m, names[i], tmp); |
There was a problem hiding this comment.
Here too.
Sorry, something went wrong.
|
The socket issues have been resolved in gh-103261. |
Sorry, something went wrong.
Sorry, something went wrong.
|
Closing, as the issues have been resolved in other PRs. Thanks :) |
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.