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

interactive figure close with wxpython 4.1 causes freeze / crash (segfault?) #17769

Closed
lfriedri opened this issue Jun 26, 2020 · 13 comments
Closed

Comments

@lfriedri
Copy link

Bug report

Bug summary

In an interactive session with a wxpython PyShell, closing a matlotlib figure by clicking on the 'x' in upper right corner causes the python shell to freeze for a few seconds, then it exits without any error message.

Code for reproduction

open PyShell with
python ….\lib\site-packages\wx\py\PyShell.py

Then run:

import matplotlib
matplotlib.use('wxagg')
matplotlib.interactive(True)
matplotlib.pyplot.figure()

Then close the figure by clicking on the 'x' in upper right corner.

Expected outcome

Figure closes and shell can be used for further commands.

Matplotlib version

  • Operating system: Windows 10
  • Matplotlib version: 3.2.2
  • Matplotlib backend (print(matplotlib.get_backend())): wxagg
  • Python version: 3.8.3
  • Other libraries: wxpython 4.1.0 msw (phoenix) wxWidgets 3.1.4

Installation procedure:
Install Winpython64-3.8.3.0dot.exe
In winpython's command prompt:
pip install wxpython matplotlib

Further notes

Closing the figure with the following command avoids the error:

matplotlib.pyplot.close('all')
@lfriedri
Copy link
Author

I also tried the following setup:

Install Winpython64-3.8.3.0dot.exe
In winpython's command prompt:
pip install wxpython
pip install matplotlib-3.3.0rc1-cp38-cp38-win_amd64.whl
with 3.3.0rc1 wheel from here: https://www.lfd.uci.edu/~gohlke/pythonlibs/#matplotlib

Error is the same

@lfriedri
Copy link
Author

lfriedri commented Jul 3, 2020

Can also be reproduce with ipython (pip install ipython -> ipython-7.16.1.-py3-none-any.whl):

%matplotlib wx
import matplotlib.pyplot as P
P.figure()

closing figure by clicking on 'x' in upper right corner causes ipython to crash without error message.

@johnabro
Copy link

johnabro commented Aug 12, 2020

Hello there,

I ran into the same problem when I've updated wxpython and matplotlib to the latest version.

I wrote a very quick test script to run as I patch the matplotlib\backends\backend_wx.py code. It simply allows you to spawn the figure by invoking plt.figure, and close this via either pressing the "x" button in the top right corner, or by pressing the "close fig" button. It also shows "embedded" FigureCanvasses in wx.Frames don't give the same error. Copy and paste it into your editor, save and run.

import matplotlib
matplotlib.use('wxagg')
import matplotlib.pyplot as plt
from matplotlib.figure import Figure
from matplotlib.backends.backend_wxagg import FigureCanvasWxAgg as FigureCanvas
import wx

app = wx.App()
f = wx.Frame(None)
p = wx.Panel(f)
figure = Figure(figsize = [6,4])
canvas = FigureCanvas(p, -1, figure)
button = wx.Button(p, label = 'open me!')
clsbtn = wx.Button(p, label = 'close fig')
sz = wx.BoxSizer(wx.VERTICAL)
sz.Add(canvas,1,wx.EXPAND)
sz.Add(button)
sz.Add(clsbtn)
newFig = None
def onBtn(event):
    global newFig
    plt.plot([1,2,3,4],[1,2,3,4])
    newFig = plt.gcf()
    plt.show(block=False)
def onCls(event):
    if newFig is None:
        return
    plt.close(newFig)
button.Bind(wx.EVT_BUTTON, onBtn)
clsbtn.Bind(wx.EVT_BUTTON, onCls)

p.SetSizer(sz)
sz.Fit(f)
f.Show()
app.MainLoop()

After 2 hours of messing around, I came up with a fix which doesn't raise a segfault and doesn't give the error "C/C++ object already deleted" after spawning a new figure right after closing a previous one.

Replace the FigureFrameWx._onClose method with this code (or just change the 2 lines marked with the "# !" comment...):

    def _onClose(self, event):
        _log.debug("%s - onClose()", type(self))
        self.canvas.close_event()
        self.canvas.stop_event_loop()
        wx.CallAfter(Gcf.destroy, self.num) # !
        if self:
            wx.Frame.Destroy(self) # !

Note this function practically calls an equivalent to the "plt.close( )" (which actually calls Gcf.destroy), after the frame has been closed and destroyed (wx.CallAfter calls it after the EVT_CLOSE event is done). For some reason passing "self" yields the "C/C++ object not deleted" error, and you have to explicitly pass in the number of the figure/manager. I think this is because according to the "matplotlib/_pylab_helpers.py" code, function "destroy", FigureFrameWx doesn't contain an attribute named "_cidgcf", and just skips the "if" statement to run what's in "else".
Also note that the entire frame itself is simply destroyed by calling wx.Frame.Destroy(self), not by calling the FigureFrameWx.Destroy method (not the same). I figured the latter method destroys the frame before the toolbar, perhaps it should be reordered. Other than that I have no clue why the latter doesn't work.

I hope this at least gives a start, I feel like this is an unstable fix and doesn't entirely clean the whole figure cache up.

@lfriedri
Copy link
Author

I can confirm, that with the following current versions the issue persists:

  • Operating system: Windows 10
  • Matplotlib version: 3.3.1
  • Matplotlib backend (print(matplotlib.get_backend())): wxagg
  • Python version: 3.8.5
  • Other libraries: wxpython 4.1.0 msw (phoenix) wxWidgets 3.1.4

I can also confirm that with the code changes proposed by @johnabro the problem can not be reproduce. Thank you for the hint.

I would appreciate if this issue could be resolved "officially" in one of the next releases.

@tacaswell
Copy link
Member

Can self ever be "falsey" in Wx?

If the Wx objects don't have a _cidgcf, does that mean we are not tracking "activate it click" for it?

What exactly do you mean by "doesn't entirely clean the whole figure cache up."?


@lfriedri if you or @johnabro could open a PR with aversion of these changes we can definitely get them released.

@tacaswell tacaswell added this to the v3.4.0 milestone Aug 28, 2020
@johnabro
Copy link

If believe "if self" was already included in the previous code. I made minimum changes.

What exactly do you mean by "doesn't entirely clean the whole figure cache up."?

Well I'm kind of speculating there: I'd assume there would be a reference stored in a list somewhere deep in the matplotlib backend. But the code snippet does call Gcf.destroy, which does destroy the matplotlib.figure.Figure object, not only the wxFrame. That's what I mean't with the figure cache.
Keep in mind I'm not that well versed in MPL but did some monkey patching. 😅

I'll first do some research into merging Github code... never really done this type of stuff. Thanks for the comment, appreciate your many contributions in general.

@anntzer
Copy link
Contributor

anntzer commented Aug 29, 2020

I haven't read through the thread, but for wx objects if self typically means "if the underlying C++ object is still alive".

@lfriedri
Copy link
Author

lfriedri commented Sep 3, 2020

Thank you for adding this topic the milestone.

Unfortunately I can not provide a pull request, since I do not feel experienced enough to take the responsibility for the proposed code change. I feel quite experienced in python, however, I do not understand where the difference between wx.Frame.destroy(self) and self.destroy() should be (as self is a wx.Frame here...)

@lfriedri
Copy link
Author

lfriedri commented Dec 1, 2020

I can confirm, that with the following current versions the issue persists:
Operating system: Windows 10
Matplotlib version: 3.3.3
Matplotlib backend (print(matplotlib.get_backend())): wxagg
Python version: 3.8.6
Other libraries: wxpython 4.1.1

@anntzer
Copy link
Contributor

anntzer commented Dec 1, 2020

I can repro the freeze even without closing the window both with wxpy4.0.7 and 4.1, linux and windows; this occurs (at least) at the python console with interactive mode on. On Windows, opened windows are frozen; on Linux, they don't pop up at all.

@tohc1
Copy link
Contributor

tohc1 commented Feb 26, 2021

I've come across this problem which is a major issue for me as I use the wx backend from inside my Python Matlab clone PythonToolkit (https://pypi.org/project/PythonToolkit/) for interactive plotting. I'm reasonably experienced in using wxpython so I've had a look at the code in the location suggested by @johnabro above (which saved a lot of time finding the problem.)

The figure references (actually FigureManagerWx classes) are stored in the Gcf class _pylab_helpers.py file and the problem seems to be that the figure is not being removed from here when closed via GUI, however It is when closed by plt.close() call.

Changing the _onClose event handler to destroy the figureManager instance rather than the frame removes the figure number correctly when closed.

    def _onClose(self, event):
        _log.debug("%s - onClose()", type(self))
        self.canvas.close_event()
        self.canvas.stop_event_loop()
        Gcf.destroy(self.figmgr)
        #if self:
        #    self.Destroy()

However the Gcf.destroy call also tries to close the Frame from the FigureManagerWx.destroy method, resulting in recursive calls to _onClose and Destroy. So I have removed it above so

To make it work i had to also change FigureManagerWx.destroy method to just Destroy the frame and r

    def destroy(self, *args):
        # docstring inherited
        _log.debug("%s - destroy()", type(self))
        frame = self.frame
        if frame:  # Else, may have been already deleted, e.g. when closing.
            #frame.Close()
            frame.Destroy(frame)
        #wxapp = wx.GetApp()
        #if wxapp:
        #    wxapp.Yield()

The program flow is now :
self._onClose -> Gcf.destroy -> self.figmgr.Destroy -> self.Destroy when closed via the window controls
Gcf.destroy -> self.figmgr.Destroy -> self.Destroy when closed via the plt.close() call.

I also needed to remove the Yield that should not be needed and also causes a crash (at least on my windows10 pc). I've left the original lines in but commented. I also needed to remove another yield (also causing a crash) in the FigureFrameWx.Destroy method again removing (commented below).

    def Destroy(self, *args, **kwargs):
        try:
            self.canvas.mpl_disconnect(self.toolbar._id_drag)
            # Rationale for line above: see issue 2941338.
        except AttributeError:
            pass  # classic toolbar lacks the attribute
        # The "if self" check avoids a "wrapped C/C++ object has been deleted"
        # RuntimeError at exit with e.g.
        # MPLBACKEND=wxagg python -c 'from pylab import *; plot()'.
        if self and not self.IsBeingDeleted():
            super().Destroy(*args, **kwargs)
            if self.toolbar is not None:
                self.toolbar.Destroy()
            #wxapp = wx.GetApp()
            #if wxapp:
            #    wxapp.Yield()
        return True

I believe this now works correctly both in my PTK environment and Ipython.
I am new to contributing to projects such as this and git - how would i submit a pull request to get this fixed?

@tacaswell
Copy link
Member

@tohc1 Great!

https://matplotlib.org/stable/devel/contributing.html#contributing-code is our guide to doing opening a PR and on a mechanical level GH's own docs are pretty good https://guides.github.com/introduction/flow/

If you need support please join the gitter room (https://gitter.im/matplotlib) as some of these things are most easily sorted out in a high-bandwdith channel!

tohc1 added a commit to tohc1/matplotlib that referenced this issue Feb 26, 2021
1) On figure window (FigureFrameWx) close - remove figure manager from from Gcf class
 previously Gcf.destroy was called with FigureFrameWx not FigureManagerWx instance so figure was not removed.
2) Destroy now done from FigureManagerWx.destroy() method to prevent multiple calls to the close event handler.
3) Remove unneeded wx mainloop yield calls that cause crash on windows form both FigureManagerWx.destroy and FigureFrameWx.Destroy
@tohc1
Copy link
Contributor

tohc1 commented Feb 27, 2021

@tacaswell I've managed to create a PR for this issue which is now passing the automatic tests and seems to work correctly for me on windows10/linux mint.

@QuLogic QuLogic modified the milestones: v3.5.0, v3.4.0 Mar 3, 2021
MihaiAnton pushed a commit to MihaiAnton/matplotlib that referenced this issue Mar 8, 2021
1) On figure window (FigureFrameWx) close - remove figure manager from from Gcf class
 previously Gcf.destroy was called with FigureFrameWx not FigureManagerWx instance so figure was not removed.
2) Destroy now done from FigureManagerWx.destroy() method to prevent multiple calls to the close event handler.
3) Remove unneeded wx mainloop yield calls that cause crash on windows form both FigureManagerWx.destroy and FigureFrameWx.Destroy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants