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

Work around a huge memory leak in PySide on Python 3 #1323

Merged
merged 2 commits into from Oct 18, 2012
Merged

Work around a huge memory leak in PySide on Python 3 #1323

merged 2 commits into from Oct 18, 2012

Conversation

cgohlke
Copy link
Contributor

@cgohlke cgohlke commented Oct 2, 2012

As reported by David Honcik on the matplotlib-users mailing list, there is a huge memory leak on Python 3 when using the Qt4Agg backend with PySide versions 1.1.1 and 1.1.2 (confirmed on Windows).

To reproduce, run the following script on Python 3.2 or 3.3 and observe the allocated process memory increase while resizing the main window:

import matplotlib
matplotlib.use('Qt4Agg')
matplotlib.rcParams['backend.qt4']='PySide'
from matplotlib import pyplot
pyplot.plot()
pyplot.show()

The problem is that QImage increases, but never decreases, the reference count of the string buffer passed to QImage.

This PR forcibly decreases the reference count of the string buffer after use, if necessary. Tested on Windows only. Is there a policy for including (or not) this kind of temporary workaround in matplotlib?

There are other reported problems creating QImage from string buffers at https://bugreports.qt-project.org/browse/PYSIDE-52 .

@dmcdougall
Copy link
Member

@cgohlke Will this make ctypes a dependency or is it a standard python library?

@mdboom
Copy link
Member

mdboom commented Oct 2, 2012

ctypes was introduced into the standard library in 2.5, so it's everywhere we support.

@mdboom
Copy link
Member

mdboom commented Oct 2, 2012

I don't know if we have a policy about this stuff -- but maybe what we should do is add a comment to this saying "this is a workaround for a bug in PySide 1.1.2 -- it should be removed when we don't need to support that version anymore", and maybe file an issue with a new label called "workarounds" -- and we periodically check those issues to see if it's time to remove them.

@cgohlke
Copy link
Contributor Author

cgohlke commented Oct 2, 2012

OK, I'll add the comments.

Can this issue be reproduced on MacOS or Linux?

@dmcdougall
Copy link
Member

I would love to check I can reproduce the error on a mac but I can't for the life of me get PySide to build with python 3.2. I'm using pip inside a python3.2 virtual env and I'm getting an error saying it can't find the python shared library. Looks like pyside is only support on windows and linux with pip?

Building a bdist_egg from the pyside git repository fails to initialise the git submodules so I get another failure.

Perhaps someone with a linux machine can reproduce this memory leak issue.

@pelson
Copy link
Member

pelson commented Oct 5, 2012

Looks like pyside is only support on windows and linux with pip?

I've got it working with python2.7 using homebrew. I should get a chance to try it out this weekend given how miserable the weather is looking...

@dmcdougall
Copy link
Member

I've got it working with python2.7 using homebrew. I should get a chance to try it out this weekend given how miserable the weather is looking...

As have I. Python 2.7 wasn't an issue. Python 3.x, on the other hand, is a nightmare.

@WeatherGod
Copy link
Member

Any movement on this? I consider this critical for rc3.

@cgohlke
Copy link
Contributor Author

cgohlke commented Oct 14, 2012

I got an off-list reply from David Honcik saying that the patch works for him (on Windows).

@mdboom
Copy link
Member

mdboom commented Oct 15, 2012

In trying to confirm this on Linux, I'm noticing that I get an immediate segfault when using PySide 1.1.0 on Linux (with Python 2.7) both with and without this patch. I'm going to file a new issue for that.

@dmcdougall
Copy link
Member

@efiring Do you have python 3.[23] installed? If so, can you recreate the memory issue here?

@mdboom
Copy link
Member

mdboom commented Oct 16, 2012

FWIW: Now that I have #1404 resolved, I can't reproduce this leak on Linux PySide 1.1.0. Not sure why it's platform-specific. (It may be version specific, as the original reports are for PySide 1.1.1 and 1.1.2).

I did, however, run this branch and it appears to have no ill effects.

@efiring
Copy link
Member

efiring commented Oct 16, 2012

On 2012/10/16 8:24 AM, Damon McDougall wrote:

@efiring https://github.com/efiring Do you have python 3.[23]
installed? If so, can you recreate the memory issue here?

@dmcdougall No, I have not yet tried python 3.x on any machine, virtual
or otherwise, that I work with.

@dmcdougall
Copy link
Member

@dmcdougall No, I have not yet tried python 3.x on any machine, virtual
or otherwise, that I work with.

@efiring I am jealous.

I'd like to test this but I'd need to install PySide via pip-3.2. Given my recent experiences with pip, I am very weary about doing this.

@dmcdougall
Copy link
Member

Ok, I have spent about two days trying to get PySide to play with python3.2 on OS X and every attempt I have tried has failed. Unless anybody else can get this installed to actually test it on python3.2 I think this is a lost cause for Mac users.

On python2.7, however I see no memory leak without @cgohlke's patch. With @cgohlke's patch, I see no adverse side-effects. I used pyside version 1.1.2.

I think @mdboom's suggestion of opening a separate 'clean-up' issue is the way to go here. Just so we don't end up with lingering code that nobody is sure about.

I give this a +1. What does everyone else think?

@efiring
Copy link
Member

efiring commented Oct 18, 2012

On 2012/10/18 3:05 AM, Damon McDougall wrote:

Ok, I have spent about two days trying to get PySide to play with
python3.2 on OS X and every attempt I have tried has failed. Unless
anybody else can get this installed to actually test it on python3.2 I
think this is a lost cause for Mac users.

On python2.7, however I see no memory leak without @cgohlke
https://github.com/cgohlke's patch. With @cgohlke
https://github.com/cgohlke's patch, I see no adverse side-effects. I
used pyside version 1.1.2.

I think @mdboom https://github.com/mdboom's suggestion of opening a
separate 'clean-up' issue is the way to go here. Just so we don't end up
with lingering code that nobody is sure about.

I give this a +1. What does everyone else think?

Sounds fine to me.

@dmcdougall
Copy link
Member

Merging, closing and opening a reminder issue for the future. Thanks.

dmcdougall added a commit that referenced this pull request Oct 18, 2012
Work around a huge memory leak in PySide on Python 3
@dmcdougall dmcdougall merged commit b6cf115 into matplotlib:v1.2.x Oct 18, 2012
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.

None yet

6 participants