Skip to content

Commit

Permalink
Improve logging of test for panel destruction (PR #12291)
Browse files Browse the repository at this point in the history
* Reduces the log noise during destruction of SettingsDialogs
* Makes the log easier to follow, objects and constants are now more clearly described.
* Fixes a space in the import statement, this was a typo and is strange style.
  • Loading branch information
feerrenrut authored Apr 15, 2021
1 parent bd12c88 commit 35ceac2
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 18 deletions.
2 changes: 1 addition & 1 deletion source/_UIAHandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import UIAUtils
from comInterfaces import UIAutomationClient as UIA
# F403: unable to detect undefined names
from comInterfaces .UIAutomationClient import * # noqa: F403
from comInterfaces.UIAutomationClient import * # noqa: F403
import textInfos
from typing import Dict
from queue import Queue
Expand Down
14 changes: 11 additions & 3 deletions source/gui/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# This file is covered by the GNU General Public License.
# See the file COPYING for more details.

import typing
import time
import os
import sys
Expand All @@ -24,6 +25,7 @@
import queueHandler
import core
from . import guiHelper
from . import settingsDialogs
from .settingsDialogs import *
from .inputGestures import InputGesturesDialog
import speechDictHandler
Expand Down Expand Up @@ -585,10 +587,16 @@ def terminate():
import brailleViewer
brailleViewer.destroyBrailleViewer()

for instance, state in gui.SettingsDialog._instances.items():
if state is gui.SettingsDialog._DIALOG_DESTROYED_STATE:
# prevent race condition with object deletion
# prevent deletion of the object while we work on it.
_SettingsDialog = settingsDialogs.SettingsDialog
nonWeak: typing.Dict[_SettingsDialog, _SettingsDialog] = dict(_SettingsDialog._instances)

for instance, state in nonWeak.items():
if state is _SettingsDialog.DialogState.DESTROYED:
log.error(
"Destroyed but not deleted instance of settings dialog exists: {!r}".format(instance)
"Destroyed but not deleted instance of gui.SettingsDialog exists"
f": {instance.title} - {instance.__class__.__qualname__} - {instance}"
)
else:
log.debug("Exiting NVDA with an open settings dialog: {!r}".format(instance))
Expand Down
46 changes: 32 additions & 14 deletions source/gui/settingsDialogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
from abc import ABCMeta, abstractmethod
import copy
import os
from enum import IntEnum

import typing
import wx
from vision.providerBase import VisionEnhancementProviderSettings
from wx.lib import scrolledpanel
Expand Down Expand Up @@ -80,10 +82,12 @@ class SettingsDialog(

class MultiInstanceError(RuntimeError): pass

_DIALOG_CREATED_STATE = 0
_DIALOG_DESTROYED_STATE = 1
class DialogState(IntEnum):
CREATED = 0
DESTROYED = 1

# holds instances of SettingsDialogs as keys, and state as the value
_instances=weakref.WeakKeyDictionary()
_instances = weakref.WeakKeyDictionary()
title = ""
helpId = "NVDASettings"
shouldSuspendConfigProfileTriggers = True
Expand All @@ -102,25 +106,39 @@ def __new__(cls, *args, **kwargs):
"Creating new settings dialog (multiInstanceAllowed:{}). "
"State of _instances {!r}".format(multiInstanceAllowed, instancesState)
)
if state is cls._DIALOG_CREATED_STATE and not multiInstanceAllowed:
if state is cls.DialogState.CREATED and not multiInstanceAllowed:
raise SettingsDialog.MultiInstanceError("Only one instance of SettingsDialog can exist at a time")
if state is cls._DIALOG_DESTROYED_STATE and not multiInstanceAllowed:
if state is cls.DialogState.DESTROYED and not multiInstanceAllowed:
# the dialog has been destroyed by wx, but the instance is still available. This indicates there is something
# keeping it alive.
log.error("Opening new settings dialog while instance still exists: {!r}".format(firstMatchingInstance))
obj = super(SettingsDialog, cls).__new__(cls, *args, **kwargs)
SettingsDialog._instances[obj] = cls._DIALOG_CREATED_STATE
SettingsDialog._instances[obj] = cls.DialogState.CREATED
return obj

def _setInstanceDestroyedState(self):
if log.isEnabledFor(log.DEBUG):
instancesState = dict(SettingsDialog._instances)
log.debug(
"Setting state to destroyed for instance: {!r}\n"
"Current _instances {!r}".format(self, instancesState)
)
if self in SettingsDialog._instances:
SettingsDialog._instances[self] = self._DIALOG_DESTROYED_STATE
# prevent race condition with object deletion
# prevent deletion of the object while we work on it.
nonWeak: typing.Dict[SettingsDialog, SettingsDialog.DialogState] = dict(SettingsDialog._instances)

if (
self in SettingsDialog._instances
# Because destroy handlers are use evt.skip, _setInstanceDestroyedState may be called many times
# prevent noisy logging.
and self.DialogState.DESTROYED != SettingsDialog._instances[self]
):
if log.isEnabledFor(log.DEBUG):
instanceStatesGen = (
f"{instance.title} - {state.name}"
for instance, state in nonWeak.items()
)
instancesList = list(instanceStatesGen)
log.debug(
f"Setting state to destroyed for instance: {self.title} - {self.__class__.__qualname__} - {self}\n"
f"Current _instances {instancesList}"
)
SettingsDialog._instances[self] = self.DialogState.DESTROYED


def __init__(
self, parent,
Expand Down

0 comments on commit 35ceac2

Please sign in to comment.