Unverified Commit 09a87c80 authored by Samuel Jones's avatar Samuel Jones Committed by GitHub
Browse files

Fix memory widget not stopping thread on keyboard interrupt (#31442)

* Implement threading similar to project recovery for memory widget

* Improve mainwindow closeEvent and clean up test threads properly

* Remove old test in Cmake

* Ensure updates are cancelled and not restarted

* Make the memorypresenter more generic

* increase stability of tests

* Implement review feedback

* Remove unstable tests

* Reintroduce now stable test and utilise separate threaded methods

* Stability improvement

* Further test stability improvements

* Use a more specific time to wait to ensure that the value is set

* Memory presenter test stability
parent 692281f8
......@@ -142,7 +142,7 @@ endif()
# Testing
set(TEST_FILES
workbench/config/test/test_user.py
workbench/test/mainwindowtest.py
workbench/test/test_mainwindow.py
workbench/test/test_import.py
workbench/plotting/plotscriptgenerator/test/test_plotscriptgeneratoraxes.py
workbench/plotting/plotscriptgenerator/test/test_plotscriptgeneratorcolorfills.py
......
......@@ -546,21 +546,22 @@ class MainWindow(QMainWindow):
# ----------------------- Events ---------------------------------
def closeEvent(self, event):
if self.project.is_saving or self.project.is_loading:
event.ignore()
self.project.inform_user_not_possible()
return
# Check whether or not to save project
if not self.project.saved:
# Offer save
if self.project.offer_save(self):
# Cancel has been clicked
if self.project is not None:
if self.project.is_saving or self.project.is_loading:
event.ignore()
self.project.inform_user_not_possible()
return
# Check whether or not to save project
if not self.project.saved:
# Offer save
if self.project.offer_save(self):
# Cancel has been clicked
event.ignore()
return
# Close editors
if self.editor.app_closing():
if self.editor is None or self.editor.app_closing():
# write out any changes to the mantid config file
ConfigService.saveConfig(ConfigService.getUserFilename())
# write current window information to global settings object
......@@ -576,11 +577,17 @@ class MainWindow(QMainWindow):
# Kill the project recovery thread and don't restart should a save be in progress and clear out current
# recovery checkpoint as it is closing properly
self.project_recovery.stop_recovery_thread()
self.project_recovery.closing_workbench = True
self.project_recovery.remove_current_pid_folder()
if self.project_recovery is not None:
self.project_recovery.stop_recovery_thread()
self.project_recovery.closing_workbench = True
self.project_recovery.remove_current_pid_folder()
# Cancel memory widget thread
if self.memorywidget is not None:
self.memorywidget.presenter.cancel_memory_update()
self.interface_manager.closeHelpWindow()
if self.interface_manager is not None:
self.interface_manager.closeHelpWindow()
event.accept()
else:
......
......@@ -30,6 +30,9 @@ class MainWindowTest(unittest.TestCase):
from workbench.app.mainwindow import MainWindow
self.main_window = MainWindow()
def tearDown(self):
self.main_window.close()
@patch("workbench.app.mainwindow.find_window")
def test_launch_custom_cpp_gui_creates_interface_if_not_already_open(self, mock_find_window):
mock_find_window.return_value = None
......
......@@ -215,3 +215,25 @@ class AsyncTaskFailure(AsyncTaskResult):
@property
def success(self):
return False
# From https://stackoverflow.com/questions/5179467/equivalent-of-setinterval-in-python by Andrea
# (https://stackoverflow.com/users/249001/andrea) with help of jd (https://stackoverflow.com/users/280490/jd)
def set_interval(interval):
"""
This is a decorator function that will repeat once per interval.
"""
def outer_wrap(function):
def wrap(*args, **kwargs):
stop = threading.Event()
def inner_wrap():
function(*args, **kwargs)
while not stop.isSet():
stop.wait(interval)
function(*args, **kwargs)
t = threading.Timer(0, inner_wrap)
t.start()
return stop
return wrap
return outer_wrap
......@@ -7,36 +7,29 @@
# This file is part of the mantidqt package
#
#
from qtpy.QtCore import QTimer
from threading import Timer
from ..memorywidget.memoryinfo import get_memory_info
from ...utils.asynchronous import set_interval
TIME_INTERVAL_MEMORY_USAGE_UPDATE = 2000 # in ms
TIME_INTERVAL_MEMORY_USAGE_UPDATE = 2.000 # in s
class MemoryPresenter(object):
"""
Gets system memory usage information and passes it to
the memory view
This happens at the beginning as well as every
TIME_INTERVAL_MEMORY_USAGE_UPDATE (ms) using QTimer
Besides, sets the style of the memory(progress) bar
at the beginning
Gets system memory usage information and passes it to the memory view this happens at the beginning as well as
every TIME_INTERVAL_MEMORY_USAGE_UPDATE (s) using threading.Timer; Also, sets the style of the memory(progress) bar
on construction.
"""
def __init__(self, view):
self.view = view
self.timer = QTimer()
self.update_memory_usage()
self.update_allowed = True
self.set_bar_color_at_start()
self.update_at_regular_intervals()
self.update_memory_usage()
self.thread_stopper = self.update_memory_usage_threaded()
def update_at_regular_intervals(self):
"""
Sets timer so that the memory usage is updated
every TIME_INTERVAL_MEMORY_USAGE_UPDATE (ms)
"""
self.timer.timeout.connect(self.update_memory_usage)
self.timer.start(TIME_INTERVAL_MEMORY_USAGE_UPDATE)
def __del__(self):
self.cancel_memory_update()
def set_bar_color_at_start(self):
"""
......@@ -50,9 +43,24 @@ class MemoryPresenter(object):
else:
pass
@set_interval(TIME_INTERVAL_MEMORY_USAGE_UPDATE)
def update_memory_usage_threaded(self):
"""
Calls update_memory_usage once every TIME_INTERVAL_MEMORY_USAGE_UPDATE
"""
self.update_memory_usage()
def update_memory_usage(self):
"""
Gets memory usage information and passes it to the view
"""
mem_used_percent, mem_used, mem_avail = get_memory_info()
self.view.set_value(mem_used_percent, mem_used, mem_avail)
if self.update_allowed:
mem_used_percent, mem_used, mem_avail = get_memory_info()
self.view.set_value(mem_used_percent, mem_used, mem_avail)
def cancel_memory_update(self):
"""
Ensures that the thread will not restart after it finishes next, as well as attempting to cancel it.
"""
self.update_allowed = False
self.thread_stopper.set()
......@@ -7,9 +7,11 @@
# This file is part of the mantid workbench.
#
#
import time
from mantidqt.widgets.memorywidget.memoryview import MemoryView, \
from_normal_to_critical, from_critical_to_normal
from mantidqt.widgets.memorywidget.memorypresenter import MemoryPresenter
from mantidqt.widgets.memorywidget.memorypresenter import MemoryPresenter, TIME_INTERVAL_MEMORY_USAGE_UPDATE
import unittest
from unittest import mock
......@@ -21,6 +23,9 @@ class MemoryPresenterTest(unittest.TestCase):
self.mock_view_internals()
self.presenter = MemoryPresenter(self.view)
def tearDown(self):
self.presenter.cancel_memory_update()
def mock_view_internals(self):
self.view.critical = 90
self.view.memory_bar = mock.Mock()
......@@ -29,21 +34,18 @@ class MemoryPresenterTest(unittest.TestCase):
self.view.set_value = mock.Mock()
def test_presenter(self):
self.assertTrue(from_normal_to_critical(self.presenter.view.critical,
75, 95))
self.assertFalse(from_critical_to_normal(self.presenter.view.critical,
75, 95))
self.assertFalse(from_normal_to_critical(self.presenter.view.critical,
95, 75))
self.assertTrue(from_critical_to_normal(self.presenter.view.critical,
95, 75))
self.presenter.cancel_memory_update()
self.assertTrue(from_normal_to_critical(self.presenter.view.critical, 75, 95))
self.assertFalse(from_critical_to_normal(self.presenter.view.critical, 75, 95))
self.assertFalse(from_normal_to_critical(self.presenter.view.critical, 95, 75))
self.assertTrue(from_critical_to_normal(self.presenter.view.critical, 95, 75))
self.assertEqual(self.presenter.view.set_value.call_count, 1)
self.assertEqual(self.presenter.view.set_bar_color.call_count, 1)
self.presenter.update_memory_usage()
self.assertEqual(self.presenter.view.set_value.call_count, 2)
def test_memory_usage_is_updated_based_on_a_constant(self):
# Sleep for just longer than the default so the test can run
time.sleep(TIME_INTERVAL_MEMORY_USAGE_UPDATE + 0.5)
self.assertGreater(self.view.set_value.call_count, 1)
if __name__ == "__main__":
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment