Skip to content

Conversation

@wesm
Copy link
Member

@wesm wesm commented May 14, 2017

Notes:

  • PyList_Append increments ref count, so new objects must be DECREF'd after being inserted
  • PyArray_SimpleNewFromDescr does not set NPY_ARRAY_OWNDATA, neither does NewFromDescr

…ARRAY_OWNDATA

Change-Id: I22b284add9268c48278e56e54f1b8be0ab6a2593
Copy link
Member

@xhochy xhochy left a comment

Choose a reason for hiding this comment

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

+1, LGTM

@wesm
Copy link
Member Author

wesm commented May 14, 2017

This is a serious enough problem that I think we should do a 0.3.1. What do you think?

@cpcloud
Copy link
Contributor

cpcloud commented May 14, 2017

Does it makes sense to follow up with a C++ test that can verify specific refcounts?

@wesm
Copy link
Member Author

wesm commented May 14, 2017

Sure, it would be a bit of work but a good idea. I put the test script here at least to eyeball whether there is a memory leak

@wesm
Copy link
Member Author

wesm commented May 14, 2017

see ARROW-1026

@asfgit asfgit closed this in c7839e9 May 14, 2017
jeffknupp pushed a commit to jeffknupp/arrow that referenced this pull request Jun 3, 2017
Notes:

* `PyList_Append` increments ref count, so new objects must be DECREF'd after being inserted
* `PyArray_SimpleNewFromDescr` does not set `NPY_ARRAY_OWNDATA`, neither does `NewFromDescr`

Author: Wes McKinney <wes.mckinney@twosigma.com>

Closes apache#685 from wesm/ARROW-1017 and squashes the following commits:

8459123 [Wes McKinney] Fix memory leak caused by list append ref count, lack of setting NPY_ARRAY_OWNDATA
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