PR: Save a reference to messagebox in EditorStack to avoid memory to be freed. #4870
Conversation
|
There's a syntax error in Python 2, that's why our tests are failing. |
spyder/widgets/editor.py
Outdated
| buttons, | ||
| parent=self) | ||
|
|
||
| answer = self.msgbox.exec() |
There was a problem hiding this comment.
@rlaverde in PyQt, we always use exec_() with the underscore
There was a problem hiding this comment.
Thanks, sometimes I forget about python2.7 :(
spyder/widgets/editor.py
Outdated
| ) % (osp.basename(finfo.filename), | ||
| str(error)), | ||
| self) | ||
| self.msgbox.exec() |
There was a problem hiding this comment.
@rlaverde in PyQt, we always use exec_() with the underscore
spyder/widgets/editor.py
Outdated
| ) % (osp.basename(finfo.filename), | ||
| str(error)), | ||
| self) | ||
| self.msgbox.exec() |
There was a problem hiding this comment.
@rlaverde in PyQt, we always use exec_() with the underscore
spyder/widgets/editor.py
Outdated
| "<br>Do you want to close it?") % name, | ||
| QMessageBox.Yes | QMessageBox.No, | ||
| self) | ||
| answer = self.msgbox.exec() |
There was a problem hiding this comment.
@rlaverde in PyQt, we always use exec_() with the underscore
spyder/widgets/editor.py
Outdated
| "your changes?") % name, | ||
| QMessageBox.Yes | QMessageBox.No, | ||
| self) | ||
| answer = self.msgbox.exec() |
There was a problem hiding this comment.
@rlaverde in PyQt, we always use exec_() with the underscore
spyder/widgets/editor.py
Outdated
| ) % osp.basename(filename), | ||
| QMessageBox.Yes | QMessageBox.No, | ||
| self) | ||
| answer = self.msgbox.exec() |
There was a problem hiding this comment.
@rlaverde in PyQt, we always use exec_() with the underscore
spyder/widgets/editor.py
Outdated
| "automatically.") % name, | ||
| QMessageBox.Ok, | ||
| self) | ||
| self.msgbox.exec() |
There was a problem hiding this comment.
@rlaverde in PyQt, we always use exec_() with the underscore
…reed. If the memory is freed, segfaults could happen when trying to access the C objects.
spyder/app/tests/test_mainwindow.py
Outdated
| @@ -672,6 +672,29 @@ def test_open_files_in_new_editor_window(main_window, qtbot): | |||
|
|
|||
|
|
|||
| @flaky(max_runs=3) | |||
| def test_close_when_file_is_changed(main_window, qtbot): | |||
There was a problem hiding this comment.
This test is hanging in Travis with the pip wheels. So please skip it when PYQT_WHEEL
|
@ccordoba12 merging? |
|
Yep. Thanks @rlaverde, great work! |
Fixes: #4453
Fixes: #4442
I tried different solutions, like returning the focus, or checking if the widget is alive, but didn't work
This solution works better, and I think that's a cleaner solution (thanks @goanpeca for the idea)
Only two of the message box was related two the errors mentioned above, but I also change the others because they could raise the same error (but that use cases are less common)