Crash if accessing python variables assigned to qtiplot windows that have been closed

[Russell Taylor]

In the immediate window, if you do:

instrument_view = getInstrumentView("workspace-name")

show the window:

instrument_view.showWindow()

close it manually and then try to use the instrument_view variable again then you get a crash.

The same thing happens for a variable obtained from a graph window that's been closed (e.g. graph_spec = plotSpectrum("RawData", 0) ).

[Martyn Gigg]

This is due to python variable pointing to a stale pointer in the same way that the workspace variables used to when a workspace was deleted. That was solved by using a proxy object and returning that when a workspace was requested.

The problem with trying the same thing in MantidPlot is that this would have to be done for absolutely every wrapped MantidPlot function and would be unmaintainable with the current simple exposure using sip.

More investigation into sip and it's return types is needed.

[Nick Draper]

Bulk move of tickets to iteration 27, if your ticket is essential for Iteration 26 then move it back.

[Nick Draper]

Bulk move of tickets at the end of iteration 27

[Nick Draper]

Bulk move of tickets at the end of iteration 28

[Nick Draper]

"New" tickets moved at the code freeze of iteration 29

[Nick Draper]

Bulk move of tickets to iteration 31 at the iteration 30 code freeze

[Russell Taylor]

I'm going to sort this out by using a proxy object much like the (original Framework python API) WorkspaceProxy. The proxy can listen for the 'destroyed' signal that all QObjects emit when they are deleted, and then set the wrapped reference to None. It's a pity that sip doesn't just do this - it seems like it could - but it doesn't. PySide isn't any better in this respect.

We will still have to intercept some functions: those that return a pointer to a QObject, and those that take one as an argument. Still this is a relatively small subset (perhaps 3 dozen or so) of the total number of methods.

[Russell Taylor]

Split up mantidplot.py and move it into a directory. Re #1037.

mantidplot.py is starting to get big, and once all the proxy business for qti objects is added is will really swell. So the proxy objects will go in a separate file and everything is in a mantidplot directory that is imported. It shouldn't matter if an old mantidplot.py is lying around in the old place - python will prefer to import the directory. Fingers crossed that I've got the installation settings right.

Changeset: 4564706275d7d74ae5272cb89356fd4965979b46

[Russell Taylor]

Try to copy directory successfully on Windows. Re #1037.

Changeset: a0eb4109530b5651df9bccbe56aa5e9bc1423cf1

[Stuart Campbell]

refs #1037. Trying to fix tests.

Changeset: 2221796f38d808f005e2b0eb5ceefb10a5a0014f

[Russell Taylor]

Try to fix Windows installer. Re #1037.

It's a real pain that it doesn't use CPack like the others.

Changeset: 147436fbcd143432c0b22913e15bae87a4b00893

[Russell Taylor]

Last attempt for now.... Re #1037.

Changeset: 78c6444cf22b3062016643790d42ee2f22a32722

[Russell Taylor]

Remove qtiplotrc.py startup file. Re #4335.

And qtiUtil.py as well. We hardly used anything in them. The only thing we did was the import of functions from the qti module into the global namespace. This will be replaced by proxy methods to prevent crashes if objects are deleted (see #1037).

Changeset: 87a32aceead0a4ee6f732fe45768253731926d19

[Russell Taylor]

Use proxy objects when accessing MantidPlot objects from Python.

Protects against crashes if the windows are closed. Re #1037. Some methods, objects are not done yet. These are ones that are probably rarely, if ever, used and will be done tomorrow.

Changeset: 6e32d1dc935d126c2d64d4f6cc10b4b754662a5f

[Russell Taylor]

Disconnect from the the 'destroyed' signal on deletion. Re #1037.

Prevents a segmentation fault on exiting MantidPlot. Also intercept a few more methods.

Changeset: d5b9a012f55c221ea0ee210112c522fd7ca175d5

[Russell Taylor]

Change return type of Layer.addImage to void. Re #1037.

An ImageWidget is not a QObject, so our proxy method won't work. Ensure that pointers to ImageWidgets created in C++ cannot 'get out'. The addImage method can still be called with an ImageWidget constructed in python, or a filename.

Changeset: 77d19bb2810e39b06d2ec8d894a1023eaaf2c99c

[Russell Taylor]

Complete wrapping of MantidPlot python objects with proxies. Re #1037.

In general, users should only ever get references to proxy objects, which are automatically set to None when the referenced window is closed.

Changeset: a722a49be1207255a97a23cdf94b80a1396f8e22

[Russell Taylor]

Change import of mantidplot to mantiplotpy in applicable places.

Didn't realise it was used so much..... Re #1037.

Changeset: 432e2fdac4a93c0203110982b768eb4a50af2f2a

[Russell Taylor]

This is a pretty big one to test!

Everything documented under:

http://www.mantidproject.org/MantidPlot:_Help#Python_Scripting_in_MantidPlot, http://www.mantidproject.org/MantidPlot:_Help#Plotting_and_Displaying_Workspaces and http://www.mantidproject.org/MantidPlot:_Help#Working_with_Instruments

should still work, and it should not be possible to crash MantidPlot by entering a python command that operates on a variable that relates to a window after that window has been closed manually.

[Janik Zikovsky]

Refs #1037: Unit test for mantid plot python proxies

Exposed the close() method to python so that the windows close properly from python. Refs #4282: make sure the FloatingWindow is deleted when closed. Testing several proxy methods. Russell: Layer.legend() and Layer.grid() do not work for me!

Changeset: 2c795e49bff68c98d2f0df3b26d5f6e5f44bcbb2

[Russell Taylor]

Make Grid and Legend work with proxy objects. Re #1037.

Changeset: b026dc53ed158b51a35dbbac1f9f918eccb0ac79

[Janik Zikovsky]

Refs #1037 long set of MantidPlot proxies unit tests

... for closing objects that were open before Russell: Layer.spectrogram() does not seem to work even with the same fix as grid() etc.

Changeset: 10e23cf5f7aa8971bf01b00193dfe7bcc8bc1e27

[Janik Zikovsky]

Refs #1037: folder deletion asks for confirmation, hanging up test

so commented out those tests for now

Changeset: bd5084d72a190fd8e9e4efe703f05795bf79aadc

[Russell Taylor]

sip needs to be told that Spectrogram is a QObject. Re #1037.

Changeset: da23c140e1b559d4d0e64f4fbcf34fc4db420685

[Janik Zikovsky]

OK, I'm satisfied with the suite of unit tests that all return values that were exposed to python are safe for deletion.

[Russell Taylor]

Revert "Change import of mantidplot to mantiplotpy in applicable places."

This reverts commit 432e2fdac4a93c0203110982b768eb4a50af2f2a (Re #1037) Back to mantidplot again. Re #4538.

Changeset: 7c9601a1b51e265adb26a07aa6b1cf64ffff8eab

[Russell Taylor]

Revert "Change import of mantidplot to mantiplotpy in applicable places."

This reverts commit 432e2fdac4a93c0203110982b768eb4a50af2f2a (Re #1037) Back to mantidplot again. Re #4538.

Changeset: 7c9601a1b51e265adb26a07aa6b1cf64ffff8eab

[Stuart Campbell]

This ticket has been transferred to github issue ​1885