1.8 RC7 errors on Win7

[mschuster]

Hi Juan,

Two errors, neither reproducible so far.

The first occurred when I shutdown Win7 with PixInsight open, app error appeared.

The second occurred when closing windows, when the last of six windows was closed fatal error appeared.

Thanks,
Mike

[mschuster]

Hi Juan,

I can reproduce the second error above. Possibly a script memory management issue.

Download this zip.

https://dl.dropboxusercontent.com/u/109232477/PixInsight/FatalError.zip

Launch PI 1.8 RC7 Win7.
Open light_07_c.fit.
Open FWHMEccentricity.0.5.js in the Script Editor.
Choose Execute>Compile & Run
Click Dismiss in the dialog.
Close the image window.
<no crash>

Now, delete the "// ?" on line 1975 in the script so that the assignment "this.targetPane = viewList" is executed. Save the file, exit PI.

Now redo the above operations.
<crash>

My intension is to use this viewList reference elsewhere in the code to enable/disable the control. But apparently just saving the reference causes the problem.

Maybe I am doing something wrong and causing the problem, but I don't see it.

Thanks,
Mike

Edit: thinking that this.targetPane might be an existing field or possibly a conflict with the enclosing with (targetPane) statement, I tried "this.targetPaneXXX = viewList". But the crash still occurs.

Edit: I also tried "parameters.targetPane = viewList" where parameters is an existing global object. Crash still occurs.

Edit: I found a workaround. Rather than assign the reference to this, disable/enable the viewList as necessary using its var reference name explicitly in all appropriate control event handlers.

[Juan Conejero]

Hi Mike,

The first exception may be caused by, in decreasing order of feasibility:

* An antivirus or firewall program, which has interpreted some actions performed by PI as a treat. These problems happen from time to time, and there's basically nothing we can do to prevent them. This is part of Windows' idiosyncrasy (viruses, antiviruses, and all that ugly stuff).

* A virus, typically a Trojan. This may happen if the virus in question attaches itself to one of PI's processes.

* A hardware problem, for example some external device connected to your computer when you shut down it.

===

The second crash can be reproduced on all platforms, and is a bug in the core ViewList object. It is an obscure bug that your script triggers for some reason. Tracing the exact cause of the problem is difficult, but the overall mechanism is quite simple: if you uncomment line 1975 of your script, the viewList object gets corrupted upon destruction. As a result, it still exists (in an unstable state) when the image is closed, so it receives a notification (ViewList objects are always watching View objects to update their view lists dynamically), which cannot be handled properly and leads to a crash.

The bug does not happen if you select the "<No View Selected>" option in your dialog before closing it. This gives me a good hint about the cause of the problem: it is related to the currentView member of the ViewList class (now I'm referring to the C++ implementation of the core ViewList object).

You already have found a workaround that works until I can fix the bug. However, and although they are not directly related to this particular problem, let me make a few comments about your code:

* Your code is prone to garbage collection issues. The most important rule is: never leave a Control object unrooted.

For example:

Code: [Select]

function parametersDialogPrototype() {
// ...
   var targetPane = new HorizontalSizer;
// ...
   this.sizer.add(targetPane);
// ...
}
This is very likely to cause problems. targetPane is not a property of any object, and it is not being used directly. Sooner or later, the garbage collector will decide to destroy it under the scenes, with catastrophic results. Note that the fact that you have added targetPane to this.sizer does not change this---in fact, it promotes the unsafe code to a real problem---, since the mechanism of adding Control and Sizer objects to Sizer objects is completely unknown to the JavaScript engine.

The correct way to implement a dialog is as follows:

Code: [Select]

function parametersDialogPrototype() {
// ...
   this.targetPane = new HorizontalSizer;
// ...
   this.sizer.add(this.targetPane);
// ...
}
Now targetPane is a property of parametersDialogPrototype, so it cannot be garbage collected before its mother instance of parametersDialogPrototype.

* Try to avoid using the "with" statement. It has been deprecated and is not allowed in strict mode since ECMAScript 5. For more information on the reasons why "with" is considered evil in JavaScript, see these blog posts:

http://www.2ality.com/2011/06/with-statement.html
http://www.yuiblog.com/blog/2006/04/11/with-statement-considered-harmful/

Although there are some different opinions, believe me that the possible benefits of with (if any) don't compensate for the many problems it generates. I used to use with in my own code a few years ago, and there are some old scripts in the official PI distribution that still have with constructs, so I know how difficult it can be getting rid of them.

* Since PI 1.8.0 RC7, calling Control.restyle() is no longer necessary for dialogs, since this is done automatically by PJSR. So you can remove lines 1951 to 1953 in your code.

[mschuster]

Hi Juan,

Thank you. I can easily update my scripts to meet your recommendations. (Doing this now).

Are there any other types of objects that should not be unrooted?

Also, I noticed that the Control constructor documentation indicates a parent argument. I have been using this everywhere (i.e. the dialog). Should I use the direct parent instead (e.g. the enclosing Sizer or whatever)?

Mike

[Juan Conejero]

Are there any other types of objects that should not be unrooted?

Control objects (or derived objects) are particularly problematic when they are included in Sizer objects. This is because the JavaScript engine knows nothing at all about the internal Sizer mechanism. A Control object can be garbage collected (and hence destroyed) while it is still part of a Sizer. If this happens, a crash will surely occur at some point. I have made efforts to minimize these problems, and in fact they don't normally happen even during execution of unsafe code, but the chance exists and it is more likely as execution time and complexity increases.

Another potential problem is with event handlers. Control (and its hierarchy), NetworkTransfer, ExternalProcess and Timer have event handlers. Event handlers can be problematic because they are invoked asynchronously. For example, an event handler can be invoked just in the middle of a garbage collection operation, even after the script has finished execution. Again, I have made efforts to prevent these problems, so they are very unlikely to happen in practice, but writing safe code to guarantee that all event-handled objects are always rooted is advisable.

Also, I noticed that the Control constructor documentation indicates a parent argument. I have been using this everywhere (i.e. the dialog). Should I use the direct parent instead (e.g. the enclosing Sizer or whatever)?

With a few exceptions, a Control object can have a parent control. If a control has a non-null parent, it becomes a child window of its parent. Otherwise it becomes a top-level window, which is managed directly by the underlying desktop environment, or the operating system. For example, try this script:

Code: [Select]

var foo = new Label;
foo.width = 400;
foo.height = 150;
foo.useRichText = true;
foo.text = "<p><b>Hey, I am a top level window!</b><br/>" +
           "I bet you didn't know this can be done in PixInsight :)</p>";
foo.show();
Yes, I know, this is an interesting feature, and the words "asynchronous" and "non-modal" are now probably playing in your mind. But beware that the JavaScript engine is not reentrant (not in the current versions of PixInsight), so this is dangerous. Feel free to experiment, though.

So a Control's parent should always be the control to which it belongs hierarchically and visually. This has nothing to do with the Sizer object to which a control can be added. A Sizer acts just like a container to group and layout controls within their parent's client area (the rectangular region of the screen occupied by the control).