To Dispose or not to Dispose

[tradesmart]

That is my question and, if so, where and when? I see that your built-in indicators call Dispose() for an object immediately after its use. For example, in VolumeProfile.OnRender() after the foreach loop.
I've run into situations where this error gets logged, which necessitates ending the NT8 proccess via Task Manager:

2016-08-30 08:39:27:736|3|16|08/30/16 08:39:27 TROUBLE! gBarVP.OnRender External component has thrown an exception.
Source: SharpDX.Direct2D1
StackTrace: at SharpDX.Direct2D1.RenderTarget.DrawLine(Vector2 point0, Vector2 point1, Brush brush, Single strokeWidth, StrokeStyle strokeStyle)
at NinjaTrader.NinjaScript.Indicators.gws.gBarVP.OnRe nder(ChartControl chartControl, ChartScale chartScale)
TargetSite: Void DrawLine(SharpDX.Vector2, SharpDX.Vector2, SharpDX.Direct2D1.Brush, Single, SharpDX.Direct2D1.StrokeStyle)
ChartBars: (Base 1-Tick) MinuteBarsType (Minute: 405/4) TickReplay OnEachTick

I believe the problem was that the code to draw the line and then dispose of its brush and strokesyle were both within a foreach loop in OnRender().
My solution was to move the brush and strokestyle object declarations up to the class scope and then dispose of them when State.Terminated. So far, this seems to have eliminated the external component exception.
I'm wondering if it's necessary at all to dispose of these objects and, if so, is it better to keep on reusing them without disposing until State.Terminated.

[NinjaTrader_Matthew]

I'm wondering if it's necessary at all to dispose of these objects and, if so, is it better to keep on reusing them without disposing until State.Terminated.

Yes it is necessary to dispose of any unmanaged references, otherwise you can experience possible memory issues

When you dispose depends on how they are created you are using them. Are these unmanaged objects subject to change? Or are they precomputed?

For example, if the brush starts a solid blue, and is always solid blue - you can precompute and store the reference to be resued, disposing of when the hosting object terminates. But if they can change throughout the lifetime of the script, you should recreate and dispose of as soon as you're done.

In the example of VolumeProfile, user has the option to change both the color and the opacity, and the user selection is converted using the RenderTarget so were recreating the brush and disposing when done on each render pass.

[tradesmart]

Yes it is necessary to dispose of any unmanaged references ... if they can change throughout the lifetime of the script, you should recreate and dispose of as soon as you're done.

Thanks, Matthew.
Assuming the objects change and that they will be disposed locally as soon as possible after RenderTarget, is it better to declare them locally (e.g., in OnRender) or in the class?

// Class scope
SharpDX.Direct2D1.Brush csdx_brush_upvol = null    ;
// then in OnRender()
csdx_brush_upvol = VolumeUpBrush.ToDxBrush(RenderTarget);
// RenderTarget and when done, dispose:
if (csdx_brush_upvol != null) csdx_brush_upvol.Dispose();

or

// Local to OnRender()
SharpDX.Direct2D1.Brush csdx_brush_upvol = VolumeUpBrush.ToDxBrush(RenderTarget);
// RenderTarget and when done, dispose:
if (csdx_brush_upvol != null) csdx_brush_upvol.Dispose();

[NinjaTrader_Matthew]

I do not think the scope should matter since you're declaring as null - you should just pay attention to where you are creating the brush. In this case, I do not see a difference in either of your examples since they're both being created and destroyed in the same context.

Correct me if I am wrong.

[NinjaTrader_JessicaP]

Just as a reference, I am providing an excerpt from release notes pages, where the documented usage of Dispose in a corner case has changed recently

The Stroke object .Dispose() method was removed due to technical redundancy. To remove memory resources from any stroke objects, simply set the stroke to null.

Also as a reference, here are some other places in the help guide where Dispose is mentioned explicitly.

UserControlCollection

It is imperative that you dispose of any custom control resources in State.Terminated to ensure there are no leaks between instances of the object

TifSelector

Cleanup() : Disposes of the TifSelector

Note : InstrumentSelector, AccountSelector, and ATMStrategySelector have a similar Cleanup() method.

OnStateChange

Resources created in State.Configure and not disposed of immediately will be kept and utilized if the NinjaScript object resides in grids (e.g. Strategy tab on Control Center), even if it is not enabled. Try to create resources in State.Historical or State.DataLoaded instead, if possible.

MergePolicy

// dispose of the bars request if we are done with it
useGlobalRequest.Dispose();

OnRenderTargetChanged

// if myBrush exists on first render target change, dispose of it
if(myBrush!=null)
{
myBrush.Dispose();
}

Using BitmapImage objects with Buttons

There are a few best practices to keep in mind when working with Buttons:

•Dispose of any leftover objects in State.Terminated for efficient memory use
•Use your object's main Dispatcher when adding or removing Buttons to or from your chart, to ensure that the correct thread is used

•Be aware of the proper States in which to initialize objects related to the Button (State.Configure), apply the Button (State.Historical), and dispose of unneeded objects (State.Terminated)
...

privatevoidDisposeCleanUp()
{
//ChartWindow Null Check
if(chartWindow!=null)
{
//Dispatcher used to Assure Executed on UI Thread
ChartControl.Dispatcher.InvokeAsync((Action)(()=>
{
//Button Null Check
if(myButton!=null)
{
//Remove Button from Indicator's Chart ToolBar
chartWindow.MainMenu.Remove(myButton);
}
}));
}
}

DrawingTool.Dispose

Releases any device resources used for the drawing tool.

[tradesmart]

Thanks. Perhaps I'm doing something wrong, but one thing I notice in a custom chart style I've created is that I must not dispose a brush created thus in OnRender:

// Declared in class scope
SharpDX.Direct2D1.Brush brush_line = null ;
// Assigned in OnRender
brush_line = chartControl.PrimaryBars.Properties.PriceMarker.BackgroundDX ;

If I do

brush_line.Dispose() ;

I get this:

2016-09-02 06:52:58:170|3|16|Failed to call OnRender() for chart object 'ChartBars': 
'Attempted to read or write protected memory. 
This is often an indication that other memory is corrupt.'

This behavior differs from that in indicators, such as VolumeProfile.

I do not think the scope should matter since you're declaring as null - you should just pay attention to where you are creating the brush. In this case, I do not see a difference in either of your examples since they're both being created and destroyed in the same context.

Correct me if I am wrong.

[NinjaTrader_Matthew]

Thanks. Perhaps I'm doing something wrong, but one thing I notice in a custom chart style I've created is that I must not dispose a brush created thus in OnRender:

// Declared in class scope
SharpDX.Direct2D1.Brush brush_line = null ;
// Assigned in OnRender
brush_line = chartControl.PrimaryBars.Properties.PriceMarker.BackgroundDX ;

If I do

brush_line.Dispose() ;

I get this:

2016-09-02 06:52:58:170|3|16|Failed to call OnRender() for chart object 'ChartBars': 
'Attempted to read or write protected memory. 
This is often an indication that other memory is corrupt.'

This behavior differs from that in indicators, such as VolumeProfile.

Thanks. Perhaps I'm doing something wrong, but one thing I notice in a custom chart style I've created is that I must not dispose a brush created thus in OnRender:
[CODE]

Hard to say without debugging/seeing the full scope of the usage myself, but based on the error, I would think the brush is still being used when you are calling .Dispose. Are you disposing it in a loop for example? Try moving it the very last code block of OnRender()

Maybe switch to a using statement instead, which will manage the brush resources for you without needing to call .Dispose()

[NinjaTrader_JessicaP]

While I will defer to Matthew on NinjaScript specific programming topics, there is more general reference material I would be happy to provide.

The message you are seeing typically occurs when you are trying to free a garbage collected object. MSDN puts out publicly available documentation on garbage collection, and that is available here,

https://msdn.microsoft.com/en-us/library/0xy59wtx.aspx

Post development, many users find it helpful to use a memory profiling tool. I am providing another publicly available stack overflow link where many .NET developers recommend their favorite tools.

http://stackoverflow.com/questions/134086/what-strategies-and-tools-are-useful-for-finding-memory-leaks-in-net

In order to make those available, you may be interested in NinjaTrader 8's new enhanced Visual Studio integration, documented here,

http://ninjatrader.com/support/helpGuides/nt8/en-us/?visual_studio_debugging.htm

And of course we are always happy to help with specific problems as they come up.

[tradesmart]

Hard to say without debugging/seeing the full scope of the usage myself, but based on the error, I would think the brush is still being used when you are calling .Dispose. Are you disposing it in a loop for example? Try moving it the very last code block of OnRender()

Maybe switch to a using statement instead, which will manage the brush resources for you without needing to call .Dispose()

How would I switch to a using statement?

Here's the existing code (with some comments), where I draw a horizontal line at the current price (before the loop begins -- for (int idx = chartBars.FromIndex; idx <= chartBars.ToIndex; idx++) -- (SharpDX.Direct2D1.DashStyle) PriceLineStyle and (float) PriceLineWidth are input parameters:

// Class scope declarations ---------------------------
SharpDX.Direct2D1.Brush csdx_brush_line = null ;
SharpDX.Direct2D1.StrokeStyle csdx_ss = null ; 
// OnRender() ------------------------------------------------
if ( (chartBars.ToIndex == bars.GetBar(bars.GetTime(chartBars.ToIndex))) )
{
                csdx_brush_line = chartControl.PrimaryBars.Properties.PriceMarker.BackgroundDX ; // price marker color
                float    f_close    = chartScale.GetYByValue(bars.GetClose(chartBars.ToIndex))    ; // current price
                float    f_xstart    = 10f; // almost to left edge of chart
                float    f_xend     = (float)chartScale.Width - 10f    ; // almost to right edge
                // HLine
                point0.X = f_xstart;
                point0.Y = f_close;
                point1.X = f_xend; 
                point1.Y = f_close;
                
                SharpDX.Direct2D1.StrokeStyleProperties ssp = new SharpDX.Direct2D1.StrokeStyleProperties();
//                    ssp.StartCap = SharpDX.Direct2D1.CapStyle.Triangle ;//.Round; no noticeable effect
//                    ssp.EndCap = SharpDX.Direct2D1.CapStyle.Triangle;
//                    ssp.DashOffset = 0.1f ;// Dot doesn't work, it is invisible no matter what this value is
                    ssp.DashStyle = PriceLineStyle; //SharpDX.Direct2D1.DashStyle.Dash; 
                csdx_ss = new SharpDX.Direct2D1.StrokeStyle(RenderTarget.Factory, ssp);
                RenderTarget.DrawLine(point0, point1, csdx_brush_line, PriceLineWidth, csdx_ss);    // draw the horizontal price line

//                if (csdx_brush_line != null) // 9/2/16 // can't dispose, hard crash ensues
//                    csdx_brush_line.Dispose() ;
}
for (int idx = chartBars.FromIndex; idx <= chartBars.ToIndex; idx++)
... draw the bars, etc.

[NinjaTrader_Matthew]

Are you sure you are not trying to access the brush after it is disposed of? That would definitely cause some issues.

Move your csdx_brush_line.Dispose() to the very last line of OnRender() and see if you still have issues.

A using statement would look something like this

// no need to expliclty dispose since "using" calls it for you
using(SharpDX.Direct2D1.Brush csdx_brush_line = chartControl.PrimaryBars.Properties.PriceMarker.BackgroundDX)
{
    RenderTarget.DrawLine(point0, point1, csdx_brush_line);
}

Ref: https://msdn.microsoft.com/en-us/library/yh598w02.aspx

[tradesmart]

I am not trying to access that brush after attempted disposing. RenderTarget.DrawLine uses it and that's all until the next OnRender() fires, which, of course, reassigns the brush.

Come to think of it, perhaps I should simply replace the brush object variable with
chartControl.PrimaryBars.Properties.PriceMarker.Ba ckgroundDX
to eliminate the issue entirely.

Thanks much for that tip about using for automatic disposal. Would you say that such a technique would be preferable in general instead of explicitly calling Dispose()?

Are you sure you are not trying to access the brush after it is disposed of? That would definitely cause some issues.

Move your csdx_brush_line.Dispose() to the very last line of OnRender() and see if you still have issues.

A using statement would look something like this

// no need to expliclty dispose since "using" calls it for you
using(SharpDX.Direct2D1.Brush csdx_brush_line = chartControl.PrimaryBars.Properties.PriceMarker.BackgroundDX)
{
    RenderTarget.DrawLine(point0, point1, csdx_brush_line);
}

Ref: https://msdn.microsoft.com/en-us/library/yh598w02.aspx