Memory exception in a custom plugin, manipulating Rhino layers

Hi all,

I am developing a plugin focused on manipulating Rhino directly with Grasshopper, through the use of event listeners and various property manipulation of Rhino documents, layers, and objects.

Things have been working swimmingly in many areas of the plugin, but one particular problem has been tough to solve. I have two components that are trying to read/write to the same memory at the same time, causing Rhino exceptions and crashes.

The conflicts appear to be happening between two components -- one is a "Layer Events Listener" that reports essentially what type of layer event just happened. The other is a "Set Layer Visibility" component that toggles the visibility of a list of layers.

The code:

public class LayerTools_LayerEventsListener : GH_Component
{
/// <summary>
/// Initializes a new instance of the LayerTools_LayerListener class.
/// </summary>
public LayerTools_LayerEventsListener()
: base("Layer Events Listener", "Layer Listener",
"Get granular information about the layer events happening in the Rhino document.",
"Squirrel", "Layer Tools")
{
}

/// <summary>
/// Registers all the input parameters for this component.
/// </summary>
protected override void RegisterInputParams(GH_Component.GH_InputParamManager pManager)
{
pManager.AddBooleanParameter("Active", "A", "Set to true to listen to layer events in the Rhino document.", GH_ParamAccess.item, false);
pManager.AddTextParameter("Exclusions", "E", "Provide a list of exclusions to stop reading specific events (Added, Deleted, Moved, Renamed, Locked, Visibility, Color, Active).", GH_ParamAccess.list);
pManager[1].Optional = true;
}

/// <summary>
/// Registers all the output parameters for this component.
/// </summary>
protected override void RegisterOutputParams(GH_Component.GH_OutputParamManager pManager)
{
pManager.AddBooleanParameter("Initialized", "I", "Whether the listener changed from passive to active.", GH_ParamAccess.item);
pManager.AddTextParameter("Document Name", "doc", "Name of the Rhino document that is changing.", GH_ParamAccess.item);
pManager.AddTextParameter("Layer Path", "path", "Path of the modifed layer.", GH_ParamAccess.item);
pManager.AddIntegerParameter("Layer Index", "ID", "Index of the modified layer.", GH_ParamAccess.item);
pManager.AddIntegerParameter("Sort Index", "SID", "Sort index of the modified layer.", GH_ParamAccess.item);
pManager.AddTextParameter("Event Type", "T", "Type of the modification.", GH_ParamAccess.item);
pManager.AddBooleanParameter("Added", "A", "If the layer has been added.", GH_ParamAccess.item);
pManager.AddBooleanParameter("Deleted", "D", "If the layer has been deleted.", GH_ParamAccess.item);
pManager.AddBooleanParameter("Moved", "M", "If the layer has been moved.", GH_ParamAccess.item);
pManager.AddBooleanParameter("Renamed", "R", "If the layer has been renamed.", GH_ParamAccess.item);
pManager.AddBooleanParameter("Locked", "L", "If the layer locked setting has changed.", GH_ParamAccess.item);
pManager.AddBooleanParameter("Visibility", "V", "If the layer's visibility has changed.", GH_ParamAccess.item);
pManager.AddBooleanParameter("Color", "C", "If the layer's color has changed.", GH_ParamAccess.item);
pManager.AddBooleanParameter("Active", "Act", "If the active layer has changed.", GH_ParamAccess.item);
}

/// <summary>
/// This is the method that actually does the work.
/// </summary>
/// <param name="DA">The DA object is used to retrieve from inputs and store in outputs.</param>
protected override void SolveInstance(IGH_DataAccess DA)
{
bool active = false;
List<string> exclusions = new List<string>();

DA.GetData(0, ref active);
DA.GetDataList(1, exclusions);

RhinoDoc thisDoc = null;

bool initialize = false;

string dName = null;
string activePath = null;
int layerIndex = -1;
int sortIndex = -1;
string eventType = null;
bool added = false;
bool deleted = false;
bool moved = false;
bool renamed = false;
bool locked = false;
bool visibility = false;
bool color = false;
bool current = false;

if (active)
{
thisDoc = RhinoDoc.ActiveDoc;

initialize = (!previouslyActive) ? true : false;

RhinoDoc.LayerTableEvent -= RhinoDoc_LayerTableEvent;
RhinoDoc.LayerTableEvent += RhinoDoc_LayerTableEvent;
previouslyActive = true;

}
else
{

RhinoDoc.LayerTableEvent -= RhinoDoc_LayerTableEvent;
previouslyActive = false;
}

if (ev != null)
{
dName = ev.Document.Name;
layerIndex = ev.LayerIndex;
eventType = ev.EventType.ToString();

if (!exclusions.Contains("Active"))
{
if (ev.EventType.ToString() == "Current")
{ // active layer has just been changed
current = true;
}

}

if (!exclusions.Contains("Moved"))
{
if (ev.EventType.ToString() == "Sorted")
{ // active layer has just been changed
moved = true;
}

}

if (!exclusions.Contains("Added")) {
if (ev.EventType.ToString() == "Added")
{ // layer has just been added
activePath = ev.NewState.FullPath;
added = true;
}

}

if (!exclusions.Contains("Active"))
{
if (ev.EventType.ToString() == "Deleted")
{ // layer has just been added

deleted = true;
}
}

if (ev.EventType.ToString() == "Modified")
{ // layer has been modified
activePath = ev.NewState.FullPath;

//skip sortindex
eventType = ev.EventType.ToString();

if (ev.OldState != null && ev.NewState != null)
{
if (!exclusions.Contains("Locked"))
{
if (ev.OldState.IsLocked != ev.NewState.IsLocked) locked = true;

}
if (!exclusions.Contains("Visibility"))
{
if (ev.OldState.IsVisible != ev.NewState.IsVisible) visibility = true;
}

if (!exclusions.Contains("Moved"))
{
if (ev.OldState.ParentLayerId != ev.NewState.ParentLayerId) moved = true;
}

//if (ev.OldState.SortIndex != ev.NewState.SortIndex) moved = true;
if (!exclusions.Contains("Renamed"))
{
if (ev.OldState.Name != ev.NewState.Name) renamed = true;
}

if (!exclusions.Contains("Color"))
{
if (ev.OldState.Color != ev.NewState.Color) color = true;
}
}

}
}

DA.SetData(0, initialize);
DA.SetData(1, dName);
DA.SetData(2, activePath);
DA.SetData(3, layerIndex);
DA.SetData(4, sortIndex);
DA.SetData(5, eventType);
DA.SetData(6, added);
DA.SetData(7, deleted);
DA.SetData(8, moved);
DA.SetData(9, renamed);
DA.SetData(10, locked);
DA.SetData(11, visibility);
DA.SetData(12, color);
DA.SetData(13, current);

}

static bool previouslyActive = false;
Rhino.DocObjects.Tables.LayerTableEventArgs ev = null;

void RhinoDoc_LayerTableEvent(object sender, Rhino.DocObjects.Tables.LayerTableEventArgs e)
{
ev = e;
this.ExpireSolution(true);
}

And for the layer visibility component:

public LayerTools_SetActiveLayer()
: base("Set Active Layer", "SetActiveLayer",
"Set the active layer in the Rhino document.",
"Squirrel", "Layer Tools")
{
}

/// <summary>
/// Registers all the input parameters for this component.
/// </summary>
protected override void RegisterInputParams(GH_Component.GH_InputParamManager pManager)
{
pManager.AddBooleanParameter("Active", "A", "Set to true to change the active layer in Rhino.", GH_ParamAccess.item, false);
pManager.AddTextParameter("Path", "P", "Full path of the layer to be activated.", GH_ParamAccess.item);
}

if (!DA.GetData(0, ref active)) return;
if (!DA.GetData(1, ref path)) return;

int layer_index = -1;
bool status = false;

if (path != null)
{

Rhino.RhinoDoc doc = Rhino.RhinoDoc.ActiveDoc;
Rhino.DocObjects.Tables.LayerTable layertable = doc.Layers;

layer_index = layertable.FindByFullPath(path, true);

if (layer_index > 0)
{ // if exists
RhinoDoc.ActiveDoc.Layers.SetCurrentLayerIndex(layer_index, true);
status = true;
}
}

DA.SetData(0, layer_index);
DA.SetData(1, status);
}

Now originally I was getting exceptions when changing multiple layers' visibility properties, which would cause the Event Listener to fire and try to read the Visibility property before the memory has been released by the Set Layer Visibility component. That led me to add an "Exceptions" input, that would allow me to disable the reading of Visibility events at the source in the Layer Events listener. That helped me manage about 95% of the crashes I was getting, but I still get strange crashes in other event properties, even when that property shouldn't be affected. For instance, I am getting a crash here on the Name property in the event from the delegate function, even though I am only changing Visibility at any one time:

I have a few ideas but they all seem pretty hacky. One is to try to set a flag that is readable by any component in the plugin -- so that the event listener can see if a "set" component is currently running and abort before causing an exception. The other is creating a delay in the event listener, somthing like 200ms, to allow any set components to finish what they are doing before reading the event. Neither seems super ideal.

Any ideas?

Thanks,

Marc

Replies to This Discussion

Permalink Reply by David Rutten on July 6, 2017 at 5:34am

That's a lot of code to parse, I can try and compile it later (I've broken my custom build version of Rhino for the time being).

I'd start by not triggering new solution from within Rhino layer events. I.e., instead of this.ExpireSolution(true); instead use ScheduleSolution(delay, callback) on the GH_Document that contains your component. The delay should be non-zero (this gives all the code that is still part of the layer changing process time to run), and you need to use the callback to expire your component.

// Inside the event handler:

GH_Document doc = OnPingDocument();

if (doc == null) return;

doc.ScheduleSolution(10, ScheduleCallback);

...

private void ScheduleCallback(GH_Document doc)

{

ExpireSolution(false);

}

Permalink Reply by Marc Syp on July 6, 2017 at 7:10am

Thanks David --

Just following the SO convention of posting all the relevant code just in case someone wants to read through it. :)

Thanks for the tip, I tried the ScheduleSolution callback and even bumped up the delay to double-check, and I'm still getting instant crash (without providing a Visibility exception to my component). The code I'm using:

...

void RhinoDoc_LayerTableEvent(object sender, Rhino.DocObjects.Tables.LayerTableEventArgs e)
{
ev = e;
//this.ExpireSolution(true);

GH_Document doc = OnPingDocument();

if (doc == null) return;

// schedule a new solution in the future to avoid reading protected memory while other components are manipulating layers
doc.ScheduleSolution(1000, ScheduleCallback);

}

private void ScheduleCallback(GH_Document doc)

{

ExpireSolution(false);

}

I'm happy to send you the CS + GH files I'm using to test if that's helpful.

Marc

Permalink Reply by Marc Syp on July 6, 2017 at 9:07am

One interesting note:

When changing a layer from Rhino, I can see the ScheduleSolution delay (1000ms for testing purposes) in LayerEvents in action. However, when modifying a layer programmatically from GH, the LayerEvents component responds with a result right away... not sure how that is possible unless the execution of the modifying component somehow triggers an immediate expiration of the LayerEvents component. My next try is to add a ScheduleSolution routine to the modifying component as well, though I'm not sure if/how that would help.

Thanks,

Marc

Permalink Reply by Marc Syp on July 6, 2017 at 2:23pm

My handle on ScheduleSolution is pretty flimsy. I can't figure out how to pass arguments to ScheduleSolution and put my business logic into the the callback. I feel that my eventListener is scheduling expiration properly but because the event reading logic is within the SolveInstance method, the offending memory access violations are happening immediately, as soon as the event is received by the event listener.

Can I solve this by passing the event itself into the ScheduleSolution callback, and setting my internal variable to the found event only once the schedule is met?

For example:

void RhinoDoc_LayerTableEvent(object sender, Rhino.DocObjects.Tables.LayerTableEventArgs e)
{
ev = e; <-- CAN I PASS THIS INTO ScheduleCallback()???

GH_Document doc = OnPingDocument();

if (doc == null) return;

// schedule a new solution in the future to avoid reading protected memory while other components are manipulating layers
doc.ScheduleSolution(20, ScheduleCallback);

}

private void ScheduleCallback(GH_Document doc)

{

this.ExpireSolution(false);

}

Sorry if this question doesn't make sense, I'm in deep waters with events.

Thanks,

Marc

Permalink Reply by Marc Syp on July 7, 2017 at 8:50am

Update --

I've tried a few things, still fighting exception errors. One is that I followed Human's lead and added a flag at the component level to indicate whether or not the component has already been set to expired before reading another event. Also added some code to remove event handlers when the canvas is locked, when the component is removed from the canvas, etc. None of this fixed the crashing issues.

One thing I did notice is that the memory exceptions invariably come from the OldState property of the event. NewState is read without memory exception, but OldState has problems. I don't know what that means.

One thing I was going to try was to create a class in assembly priority that would allow my "set" components to register when they are executing, which the layer Event listener component would check for an "all clear" before executing. Seems pretty hacky though.

I would really appreciate anyone's suggestions on where to go next. The CS and GH files are attached for anyone who is brave enough to build and test. (Metahopper is used but not required in the example file.)

Thanks,

Marc

Attachments:

LayerTools_LayerEventsListener.cs, 11 KB
LayerTools_SetLayerVisibility.cs, 4 KB
Layer Event Information-squirrel.gh, 51 KB

Permalink Reply by Marc Syp on July 14, 2017 at 9:10am

FYI -

I've made some progress on this -- thanks to Andrew Heumann for some suggestions on things to try.

A few notes:

1) Moving ExpireSolution into a ScheduleSolution did cut down on some of the crashes... but it made the event.OldState even more volatile (with a great number of fields throwing exceptions in the debugger, and crashing Rhino if accessed).

2) The problem was largely solved in the components that Modify the Rhino document, not the layer events listener itself. The trick that prevented crashes (so far, fingers crossed) was moving "layer.CommitChanges()" out of SolveInstance and into the AfterSolveInstance override. After verifying the changes with testing, Andrew also updated his Human components to use this pattern.

3) After updating the layer modification components, I moved the ExpireSolution back into SolveInstance, and event data is much more reliable again. That said, there are a great number of instances where some event.OldState properties throw exceptions (particularly the IsValid property, ironically) -- but my code uses a hack to determine if a state is corrupted without access an invalid property... namely, corrupted OldStates generally have a layerindex property that is outside the bounds of the document layertable (e.g., '1562342' or '-12546'). My components check for an invalid layerindex before accessing any OldState properties.

As a side note, I am a bit concerned about this code in Rhino 6, as I just ran across this forum topic, where Steve Baer notes that in Rhino 6 WIP he has changed the paradigm to execute CommitChanges immediately when any modification is made to a layer. I am concerned that this will cause problems to resurface if I don't refactor my entire components to execute their business logic outside of SolveInstance.

I will try to log some data regarding the corrupted events but I'm not sure if it is a GH issue or a Rhino issue, so I'll post both here and in the Rhino Forum.

RSS