Skip to content

Fix process of reference count during GC#103

Merged
mgedmin merged 3 commits intozopefoundation:masterfrom
y-fujisaki2:patch-1
Sep 21, 2017
Merged

Fix process of reference count during GC#103
mgedmin merged 3 commits intozopefoundation:masterfrom
y-fujisaki2:patch-1

Conversation

@y-fujisaki2
Copy link
Copy Markdown
Contributor

@y-fujisaki2 y-fujisaki2 commented Sep 19, 2017

call PyObject_GC_UnTrack() in tp_dealloc()
see the following sites for details:

Fixes #100.

call PyObject_GC_UnTrack() in tp_dealloc()
see the following sites for details:
 * https://bugs.python.org/issue31095
 * python/cpython#2974
Copy link
Copy Markdown
Member

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

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

Looks good to me, with one minor formatting comment.

static int
lookup_clear(lookup *self)
{
{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer not to see unnecessary trailing whitespace being added to an unrelated function.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I appreciate you removing it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@y-fujisaki2 I believe @mgedmin was requesting that you update the PR to avoid this problem. I am making the same request.

@mgedmin mgedmin requested a review from jamadden September 20, 2017 10:07
y-fujisaki2 and others added 2 commits September 21, 2017 10:26
Thank you for follows. I misread it for carriage return.
Copy link
Copy Markdown
Member

@jamadden jamadden left a comment

Choose a reason for hiding this comment

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

LGTM. I don't think this is something we have to have a signed contributor agreement for, so we should be good to merge.

@mgedmin
Copy link
Copy Markdown
Member

mgedmin commented Sep 21, 2017

Thank you!

@mgedmin mgedmin merged commit 7262ec2 into zopefoundation:master Sep 21, 2017
@mgedmin
Copy link
Copy Markdown
Member

mgedmin commented Sep 22, 2017

I've released zope.interface 4.4.3 to PyPI with this fix.

@y-fujisaki2
Copy link
Copy Markdown
Contributor Author

Thank you so much for follows!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants