From 36df7ecb67c802fc4cf78a8dcfadf9c47485cc0e Mon Sep 17 00:00:00 2001 From: Paul McCarthy <pauld.mccarthy@gmail.com> Date: Thu, 16 Jul 2015 11:45:33 +0100 Subject: [PATCH] Ensuring that Profile instances are deleted when a ViewPanel is closed, and that the Frame instance clears refs to closed ViewPanel instances. --- fsl/fslview/actions/__init__.py | 22 ++++++++++++++++-- fsl/fslview/frame.py | 16 +++++++++---- fsl/fslview/panel.py | 39 ++++++++++++++++++++------------ fsl/fslview/profiles/__init__.py | 32 +++++++++++++++++++++++++- fsl/fslview/views/viewpanel.py | 18 +++++++++++++-- 5 files changed, 103 insertions(+), 24 deletions(-) diff --git a/fsl/fslview/actions/__init__.py b/fsl/fslview/actions/__init__.py index 7f74712eb..7a7b242a0 100644 --- a/fsl/fslview/actions/__init__.py +++ b/fsl/fslview/actions/__init__.py @@ -127,7 +127,16 @@ class Action(props.HasProperties): parent.Bind(evType, wrappedAction, widget) widget.Enable(self.enabled) - self._boundWidgets.append(widget) + self._boundWidgets.append((parent, evType, widget)) + + + def unbindAllWidgets(self): + """Unbinds all widgets which have been bound via :meth:`bindToWidget`. + """ + for parent, evType, widget in self._boundWidgets: + parent.Unbind(evType, source=widget) + + self._boundWidgets = [] def _enabledChanged(self, *args): @@ -137,7 +146,7 @@ class Action(props.HasProperties): if self.enabled: self.doAction = self.__enabledDoAction else: self.doAction = self.__disabledDoAction - for widget in self._boundWidgets: + for _, _, widget in self._boundWidgets: widget.Enable(self.enabled) @@ -193,6 +202,15 @@ class ActionProvider(props.HasProperties): act = Action(overlayList, displayCtx, action=func) self.__actions[name] = act + log.memory('{}.init ({})'.format(type(self).__name__, id(self))) + + + def __del__(self): + """ + """ + log.memory('{}.del ({})'.format(type(self).__name__, id(self))) + self.__actions = None + def addActionToggleListener(self, name, listenerName, func): """Add a listener function which will be called when the named action diff --git a/fsl/fslview/frame.py b/fsl/fslview/frame.py index 456a818cc..3530734c1 100644 --- a/fsl/fslview/frame.py +++ b/fsl/fslview/frame.py @@ -182,21 +182,29 @@ class FSLViewFrame(wx.Frame): return self.__viewPanels .remove(panel) + self.__viewPanelMenus .pop( panel, None) title = self.__viewPanelTitles.pop( panel) log.debug('Destroying view panel {} ({})'.format( title, type(panel).__name__)) - # Calling fslpanel.FSLViewPanel.destroy() - # - I think that the AUINotebook does the - # wx.Window.Destroy side of things ... - panel.destroy() + # Unbind view panel menu + # items, and remove the menu + for actionName, actionObj in panel.getActions().items(): + actionObj.unbindAllWidgets() menuBar = self.GetMenuBar() menuIdx = menuBar.FindMenu(title) if menuIdx != wx.NOT_FOUND: menuBar.Remove(menuIdx) + # Calling fslpanel.FSLViewPanel.destroy() + # - I think that the AUINotebook does the + # wx.Window.Destroy side of things, which + # will eventually result in panel.__del__ + # ... + panel.destroy() + def __onClose(self, ev): """Called on requests to close this :class:`FSLViewFrame`. diff --git a/fsl/fslview/panel.py b/fsl/fslview/panel.py index e8ce10a8d..4c57902d5 100644 --- a/fsl/fslview/panel.py +++ b/fsl/fslview/panel.py @@ -4,19 +4,16 @@ # # Author: Paul McCarthy <pauldmccarthy@gmail.com> # -"""This module provides two classes - the :class:`FSLViewPanel`, and the -:class:`FSLViewToolBar`. +"""This module provides an important class - the :class:`FSLViewPanel`. A :class:`FSLViewPanel` object is a :class:`wx.Panel` which provides some sort of view of a collection of overlay objects, contained within an -:class:`.OverlayList`. Similarly, a :class:`FSLViewToolBar` is a -:class:`wx.lib.agw.aui.AuiToolBar` which provides some sort of control over -the view. - -Instances of these classes are also :class:`.ActionProvider` instances - any -actions which are specified during construction may be exposed to the -user. Furthermore, any display configuration options which should be made -available available to the user should be added as :class:`.PropertyBase` +:class:`.OverlayList`. + +``FSLViewPanel`` instances are also :class:`.ActionProvider` instances - any +actions which are specified during construction may (or may not ) be exposed +to the user. Furthermore, any display configuration options which should be +made available available to the user should be added as :class:`.PropertyBase` attributes of the :class:`FSLViewPanel` subclass. See the following for examples of :class:`FSLViewPanel` subclasses: @@ -55,6 +52,13 @@ class _FSLViewPanel(actions.ActionProvider): contains display related properties about the :attr:`_overlayList`. - :attr:`_name`: A unique name for this :class:`ViewPanel`. + + + TODO Important notes about: + + - :meth:`destroy` + + - :meth:`__del__` """ @@ -107,22 +111,27 @@ class _FSLViewPanel(actions.ActionProvider): called. So this method *must* be called by managing code when a panel is deleted. - Overriding subclass implementations should also call this base class - method, otherwise warnings will probably be output to the log (see - :meth:`__del__`) + Overriding subclass implementations must call this base class + method, otherwise memory leaks will probably occur, and warnings will + probably be output to the log (see :meth:`__del__`). """ self.__destroyed = True def __del__(self): + """Sub-classes which implement ``__del__`` must call this + implementation, otherwise memory leaks will occur. + """ + actions.ActionProvider.__del__(self) + + self._overlayList = None + self._displayCtx = None if not self.__destroyed: log.warning('The {}.destroy() method has not been called ' '- unless the application is shutting down, ' 'this is probably a bug!'.format(type(self).__name__)) - actions.ActionProvider.__del__(self) - class FSLViewPanel(_FSLViewPanel, wx.Panel): """ diff --git a/fsl/fslview/profiles/__init__.py b/fsl/fslview/profiles/__init__.py index bda866f04..1bcc5aecc 100644 --- a/fsl/fslview/profiles/__init__.py +++ b/fsl/fslview/profiles/__init__.py @@ -186,6 +186,19 @@ class Profile(actions.ActionProvider): for (mode, handler), (altMode, altHandler) in altHandlers.items(): self.addAltHandler(mode, handler, altMode, altHandler) + + def destroy(self): + """Calls the :meth:`deregister` method, and clears references to + the display context, view panel, and overlay list. This method + is called by the :class:`ProfileManager` when this ``Profile`` + instance is no longer needed. + """ + self.deregister() + + self._viewPanel = None + self._overlayList = None + self._displayCtx = None + def getEventTargets(self): """Must be overridden by subclasses, to return a sequence of @@ -569,6 +582,23 @@ class ProfileManager(object): viewPanel.profile = profilez[0] + def destroy(self): + """This method should be called by the owning :class:`.ViewPanel` when + it is about to be destroyed (or when it no longer needs a + ``ProfileManager``). + + This method destros the current profile (if any), and clears some + important object references to avoid memory leaks. + """ + if self._currentProfile is not None: + self._currentProfile.destroy() + + self._currentProfile = None + self._viewPanel = None + self._overlayList = None + self._overlaydisplayCtx = None + + def getCurrentProfile(self): """Returns the :class:`Profile` instance currently in use.""" return self._currentProfile @@ -592,7 +622,7 @@ class ProfileManager(object): log.debug('Deregistering {} profile from {}'.format( self._currentProfile.__class__.__name__, self._viewCls.__name__)) - self._currentProfile.deregister() + self._currentProfile.destroy() self._currentProfile = profileCls(self._viewPanel, self._overlayList, diff --git a/fsl/fslview/views/viewpanel.py b/fsl/fslview/views/viewpanel.py index ee91ee6ac..a7da9444e 100644 --- a/fsl/fslview/views/viewpanel.py +++ b/fsl/fslview/views/viewpanel.py @@ -136,17 +136,31 @@ class ViewPanel(fslpanel.FSLViewPanel): def destroy(self): + """ + """ + fslpanel.FSLViewPanel.destroy(self) # Make sure that any control panels are correctly destroyed for panelType, panel in self.__panels.items(): panel.destroy() - + + # Remove listeners from the overlay + # list and display context lName = 'ViewPanel_{}'.format(self._name) self .removeListener('profile', lName) self._overlayList.removeListener('overlays', lName) - self._displayCtx .removeListener('selectedOverlay', lName) + self._displayCtx .removeListener('selectedOverlay', lName) + + # Disable the ProfileManager + self.__profileManager.destroy() + + # Un-initialise the AUI manager + self.__auiMgr.UnInit() + + self.__profileManager = None + self.__auiMgr = None def setCentrePanel(self, panel): -- GitLab