gecos-lab / PZero

GNU Affero General Public License v3.0
22 stars 2 forks source link

free memory when closing window #44

Closed andrea-bistacchi closed 1 year ago

andrea-bistacchi commented 1 year ago

At the moment Qt windows such as the 3D view are not destroyed, but just hidden, when they are closed ("close" or "X" icon), so memory is not freed from the objects that they contain.

To destroy them the following option should be set in the windows_factory module:

self.setAttribute(Qt.WA_DeleteOnClose, True)

However this causes a crash when signals pointing to the destroyed window are fired afterwards. Therefore we need to disconnect all signals before destroying a window.

Maybe (maybe!) this can be done more efficiently by adding each signal to a list as it is connected, and then calling a list of signals to be disconnected.

self.signals = []
self.signals.append = signa....connect(...)

(...)

for signal in self.signals:
    ...disconnect(signal)
mcbaguetti commented 1 year ago

I fixed it with the last commit (https://github.com/andrea-bistacchi/PZero/commit/e70b9ded646e3f27ab8a055d71c2fdb89f086228), tested it a bit, and it seems to be working. If you can test it too to be sure about this.

gbene commented 1 year ago

Hi @mcbaguetti. I was testing while trying some new code and I found the following error when opening two or more windows (for example the 3D view + the map view or even two 3D views):

start disconnect_all_signals
<bound PYQT_SIGNAL triggered of QAction object at 0x7f3b03366670>
<bound PYQT_SIGNAL project_close_signal of ProjectWindow object at 0x7f3b7b2b5dc0>
Traceback (most recent call last):
  File "PZero/pzero/windows_factory.py", line 4578, in closeEvent
    disconnect_all_signals(self.signals)
  File "PZero/pzero/signals_handler.py", line 8, in disconnect_all_signals
    signal.disconnect()
TypeError: disconnect() failed between 'project_close_signal' and all its connections
Aborted (core dumped)

This occurs when, after closing one of the n windows, a second window is closed.

mcbaguetti commented 1 year ago

https://github.com/andrea-bistacchi/PZero/issues/44#issuecomment-1511416597

Hi Gabriele, you're right, I noted that if we don't add the parent signals to the signals list (so inside it remains only the self.actionClose.triggered), that problem doesn't happen. But then it crashes again for the issue explained by Andrea > https://github.com/andrea-bistacchi/PZero/issues/44#issue-1658733189

gbene commented 1 year ago

Mhh yes in fact now the crash occurs when a window is closed and signals are fired for the given window (for example by changing the color or point/line size in the legend).

mcbaguetti commented 1 year ago

one idea that may help is to create a function to retrieve all the signals of a specific class. Maybe it is not necessary but it could be useful in this or other cases.

andrea-bistacchi commented 1 year ago

Maybe (maybe?!) each signal can be added to a list. Then you can destroy the list and the signals?

As in:

signals_list = signals_list.append(new_signal)

mcbaguetti commented 1 year ago

Hi all, I think I've found the solution, I've uploaded it to the env_testing branch if you want to test it too.

Maybe (maybe?!) each signal can be added to a list. Then you can destroy the list and the signals?

Three weeks ago I tested this idea and it didn't work properly like @gbene said there was still a crash. I provide more details because it can help in the future.

The crash reason was: by closing only the first window, you delete the signals of the first window and you also disrupt all the signals (or the signal connections) of the second window!! But it was strange because in the code there was no trace of deleting the signals of all the windows at the same time and also Qt.WA_DeleteOnClose was not responsible for this.

In the end, I found that if you use an anonymous lambda function inside the .connect() or .disconnect(), there is no way to disconnect it other than calling disconnect() on the signal, but that will disconnect all connected signals.

An example that will disconnect all the connected signals (the two lambdas here are not the same object):

self.signal.connect(lambda: self.call_to_fun)
# ...
self.signal.disconnect(lambda: self.call_to_fun)

The solution here is to save the reference of the lambda function and pass it both inside the .connect() and .disconnect() and this will disconnect only this specific signal:

lambda_reference = lambda: self.call_to_fun
self.signal.connect(lambda_reference)
# ...
self.signal.disconnect(lambda_reference)

In the end, I rewrote the signals in the same way by saving their lambda reference and passing that to disconnect the signals. As I said I think the issue is solved, the tests also didn't find anything strange, today I will be testing this again to be sure of that.

andrea-bistacchi commented 1 year ago

Well done! If I've got time I'll try to run some random manual test.

mcbaguetti commented 1 year ago

solved with this commit