Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leak in tostring_rgba_minimize(). (#3197) #3207

Merged
merged 1 commit into from Jul 11, 2014

Conversation

dopplershift
Copy link
Contributor

The APIs being used in PyCXX (and Python C-API) were not actually taking
ownership. Instead, make Python allocate memory.

@WeatherGod
Copy link
Member

The build failed in 3.3:

src/_backend_agg.cpp:2410:45: error: call of overloaded ‘Bytes(NULL, int)’ is ambiguous
src/_backend_agg.cpp:2410:45: note: candidates are:
extern/CXX/Python3/Objects.hxx:1873:9: note: Py::Bytes::Bytes(const char*, Py_ssize_t)
extern/CXX/Python3/Objects.hxx:1861:9: note: Py::Bytes::Bytes(const string&, Py_ssize_t)
extern/CXX/Python3/Objects.hxx:1837:18: note: Py::Bytes::Bytes(PyObject*, bool)

And unit tests failed for py2.6 and py2.7.

@tacaswell
Copy link
Member

The test failures are PEP8 from a different PR (from me!)

@dopplershift
Copy link
Contributor Author

Build fixed and tested by me on 2.7 and 3.3 here. Seems to pass (build at least) on Travis 3.3.

@tacaswell
Copy link
Member

I am unclear on why pep8 is failing on this PR, but not on the others.

@mdboom
Copy link
Member

mdboom commented Jul 11, 2014

Yeah -- I'm not sure about the PEP8 thing either. I've just fixed it on master #3210 in hopes we can fix this here.

The APIs being used in PyCXX (and Python C-API) were not actually taking
ownership. Instead, make Python allocate memory.
@dopplershift
Copy link
Contributor Author

Rebased so that it builds on 3.3. and 3.4, not 3.2. Hoping by magic this fixes the weird memory issues we saw on 3.3 on last build.

@mdboom
Copy link
Member

mdboom commented Jul 11, 2014

I think this is passing enough -- i.e. it passes everywhere master passes. Merging. Thanks, @dopplershift.

mdboom added a commit that referenced this pull request Jul 11, 2014
Fix memory leak in tostring_rgba_minimize(). (#3197)
@mdboom mdboom merged commit 3b4f958 into matplotlib:master Jul 11, 2014
@dopplershift dopplershift deleted the fix_memory_leak branch July 11, 2014 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants