bpo-40737: Fix possible reference leak for sqlite3 initialization by erlend-aasland · Pull Request #20323 · python/cpython

erlend-aasland · 2020-05-22T22:18:06Z

According to the docs, PyModule_AddObject() only decrements the
reference count of value on success. Calling code must Py_DECREF()
manually on error.

https://bugs.python.org/issue40737

According to the docs, PyModule_AddObject() only decrements the reference count of value _on success_. Calling code must Py_DECREF() manually on error.

corona10

lgtm with nit comment.

bedevere-bot · 2020-05-25T15:33:42Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

erlend-aasland · 2020-05-25T18:23:36Z

I have made the requested changes; please review again.

bedevere-bot · 2020-05-25T18:23:39Z

Thanks for making the requested changes!

@corona10: please review the changes made to this pull request.

erlend-aasland · 2020-05-25T18:54:01Z

Should probably be backported to 3.9, @corona10.

bedevere-bot · 2020-05-26T08:21:24Z

Thanks for making the requested changes!

@corona10: please review the changes made to this pull request.

corona10

Last review :)
It will cause the leak!

cpython/Python/modsupport.c

Line 693 in a487a39

Py_INCREF(type);

bedevere-bot · 2020-05-26T08:27:07Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

erlend-aasland · 2020-05-26T08:29:29Z

I have made the requested changes; please review again

bedevere-bot · 2020-05-26T08:29:33Z

Thanks for making the requested changes!

@corona10: please review the changes made to this pull request.

corona10

lgtm! Thanks for the contribution!

miss-islington · 2020-05-26T12:18:22Z

Thanks @erlend-aasland for the PR, and @corona10 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

…thonGH-20323) (cherry picked from commit 5eb45d7) Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>

bedevere-bot · 2020-05-26T12:18:35Z

GH-20425 is a backport of this pull request to the 3.9 branch.

…-20323) (cherry picked from commit 5eb45d7) Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>

erlend-aasland · 2020-05-26T15:49:52Z

Thanks for you patience and helpful comments, @corona10!

Use PyModule_AddObject() correctly in sqlite3

fc20024

According to the docs, PyModule_AddObject() only decrements the reference count of value _on success_. Calling code must Py_DECREF() manually on error.

erlend-aasland requested a review from berkerpeksag as a code owner May 22, 2020 22:18

the-knights-who-say-ni added the CLA signed label May 22, 2020

bedevere-bot added the awaiting review label May 22, 2020

Add NEWS entry

a51d380

corona10 approved these changes May 25, 2020

View reviewed changes

Comment thread Misc/NEWS.d/next/Library/2020-05-23-00-22-11.bpo-40737.iph-CM.rst Outdated

bedevere-bot added awaiting merge and removed awaiting review labels May 25, 2020

corona10 requested changes May 25, 2020

View reviewed changes

Comment thread Modules/_sqlite/module.c Outdated

Comment thread Modules/_sqlite/module.c Outdated

Comment thread Modules/_sqlite/module.c Outdated

Comment thread Modules/_sqlite/module.c Outdated

Comment thread Modules/_sqlite/module.c Outdated

Comment thread Modules/_sqlite/module.c Outdated

bedevere-bot added awaiting changes and removed awaiting merge labels May 25, 2020

Erlend Egeberg Aasland and others added 9 commits May 25, 2020 17:44

Update Misc/NEWS.d/next/Library/2020-05-23-00-22-11.bpo-40737.iph-CM.rst

df83c42

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

Update Modules/_sqlite/module.c

1b5def8

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

Update Modules/_sqlite/module.c

9e954fb

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

Update Modules/_sqlite/module.c

59f6cfb

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

Update Modules/_sqlite/module.c

1ea05c9

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

Update Modules/_sqlite/module.c

37403bc

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

Use PyModule_AddType iso. PyModule_AddObject

8d6e116

Fix dot in Node name

01e89c4

Set up _all_ types

40bab48

erlend-aasland changed the title ~~bpo-40737: Use PyModule_AddObject() correctly in sqlite3~~ bpo-40737: Use PyModule_AddType() in sqlite3 May 25, 2020

erlend-aasland changed the title ~~bpo-40737: Use PyModule_AddType() in sqlite3~~ bpo-40737: Fix possible reference leak for sqlite3 initialization May 25, 2020

bedevere-bot added awaiting change review and removed awaiting changes labels May 25, 2020

bedevere-bot requested a review from corona10 May 25, 2020 18:23

corona10 requested changes May 26, 2020

View reviewed changes

Comment thread Modules/_sqlite/cursor.h Outdated

bedevere-bot added the awaiting change review label May 26, 2020

bedevere-bot requested a review from corona10 May 26, 2020 08:21

corona10 requested changes May 26, 2020

View reviewed changes

Comment thread Modules/_sqlite/module.c Outdated

bedevere-bot removed the awaiting change review label May 26, 2020

bedevere-bot added the awaiting changes label May 26, 2020

Update Modules/_sqlite/module.c

841eab6

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

bedevere-bot added awaiting change review and removed awaiting changes labels May 26, 2020

bedevere-bot requested a review from corona10 May 26, 2020 08:29

corona10 approved these changes May 26, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels May 26, 2020

corona10 added the needs backport to 3.9 label May 26, 2020

corona10 merged commit 5eb45d7 into python:master May 26, 2020

bedevere-bot removed the awaiting merge label May 26, 2020

bedevere-bot removed the needs backport to 3.9 label May 26, 2020

erlend-aasland deleted the fix-issue-40737 branch May 26, 2020 12:32

miss-islington added a commit that referenced this pull request May 26, 2020

bpo-40737: Fix possible reference leak for sqlite3 initialization (GH…

7df9c41

…-20323) (cherry picked from commit 5eb45d7) Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>

Uh oh!

Uh oh!

Conversation

erlend-aasland commented May 22, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

corona10 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented May 25, 2020

Uh oh!

erlend-aasland commented May 25, 2020

Uh oh!

bedevere-bot commented May 25, 2020

Uh oh!

erlend-aasland commented May 25, 2020

Uh oh!

Uh oh!

bedevere-bot commented May 26, 2020

Uh oh!

corona10 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bedevere-bot commented May 26, 2020

Uh oh!

erlend-aasland commented May 26, 2020

Uh oh!

bedevere-bot commented May 26, 2020

Uh oh!

corona10 left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented May 26, 2020

Uh oh!

bedevere-bot commented May 26, 2020

Uh oh!

erlend-aasland commented May 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

erlend-aasland commented May 22, 2020 •

edited by bedevere-bot

Loading