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.

Sign up for GitHub

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

Improve logging of test for panel destruction #12291

Merged
merged 4 commits into from
Apr 15, 2021
Merged

Improve logging of test for panel destruction #12291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d1006f3
Fix space in import statement
feerrenrut Apr 13, 2021
1e569bf
Less noisy destruction tests
feerrenrut Apr 13, 2021
c7e32da
More detail in Destroyed but not deleted instance log message.
feerrenrut Apr 14, 2021
56c3f43
Fix lint errors
feerrenrut Apr 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion source/_UIAHandler.py
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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