Om Destructor Woes

"Had I knowm, I would have been a watchmaker," is perhaps wrongly quoted to Einstein I believe on his discovery that E=MC2 meant the possibility of the atomic bomb. Well "Had I known, I would have never added destructors to Om" is my equivalent.

Wow, it makes life difficult. So after the issues discussed last time, I decided to try to tackle making the [font='courier new']this[/font] keyword available inside the destructor, as in:

var x = { n = "hello"; destructor = func { out this.n; };};That sort of thing. Necessary, logical, no problem.

Now, when you execute a method, you pass in an [font='courier new']Om::Value[/font] representing the [font='courier new']this[/font] for the context of the function, which is put in a particular place on the stack which the [font='courier new']ThisNode[/font] in the compiler knows to read from and write to. So the first attempt was to just create an object based on the one being destroyed and pass that in.

Lots of hilarious stack exploding later, as the created object was then destroyed, triggering its destructor call again, creating a new object, which was then destroyed and so on and so on, I realised this wasn't going to work out too well. Doh!

Now, Om provides an external set of types that the user sees - [font='courier new']Om::Type::Int[/font], [font='courier new']Om::Type::Object[/font], [font='courier new']Om::Type::Function[/font] etc. Under the hood, there are also some extended types that are treated differently internally, but masquerade as the externally visible types so the user of the API doesn't know they exist. For example, you have [font='courier new']InternalFunctionType[/font] and [font='courier new']ExternalFunctionType[/font], which represent functions that are not actual script functions, and the API is set up so that if the user queries the types of these, they get [font='courier new']Om::Type::Function[/font] back. The internal [font='courier new']TypedValue[/font] has [font='courier new']type()[/font] and [font='courier new']realType()[/font] methods, and [font='courier new']type()[/font] just does a table lookup to see what external type to return based on the actual type.

So I thought I could use a new extended type, called [font='courier new']DestroyObjectType[/font], which masquerades as [font='courier new']Om::Type::Object[/font], but does not increment or decrement references, so that it does not call any destroying code when it goes out of scope, and use an object of this type as the [font='courier new']this[/font] object for object destructors.

All fine and dandy. Until I realised this:

var x ={ destructor = func { return this; };};Oh dear. If the user engineers to retrieve the result of the destructor, something that is possible with the [font='courier new']Om::Value[/font] API, they then have a type that looks like an object, but refers to a location in the entity heap that no longer exists. So I thought, okay, easy to block, will disallow returning [font='courier new']this[/font] from a destructor.

Then I realised:

export a;if(true){ var x = { n = "hello"; destructor = func { a = this; }; };}out a.n;Boom. When the last line is run, [font='courier new']a[/font] is pointing to a [font='courier new']DestroyObjectType[/font], which indexes an entity that no longer exists.

So the problem is that by allowing [font='courier new']this[/font] to work in a destructor, which I think it has to, I have to somehow avoid having it "leak" out of the destructor in any way. But there are so many possible ways this can happen due to the design of the language and, more importantly, checking for these cases is going to add overhead to all sorts of things. Bear in mind that we can do things like this:

var some_function = func{ do_something(this);};var x ={ desructor = some_function;};What I mean is there is nothing special about a function used as a destructor, so the overhead we'd need to do to check for these kinds of leaks would have to happen all the time in every context.

The whole structure of Om is set up so that it is not needed to do things like checking if an entity index is actually pointing to a valid entity of the correct type. It should be impossible for this to happen, so the check is not needed, so referencing an entity is just a straightforward array index and a (free) static cast. Nothing that starts to require non-debug-build checking of this kind of thing is acceptable at all, full stop.

So I did what is normally best under these circumstances and went for a nice long walk along one of the charming beaches I am fortunate enough to live near and before I knew it, a plan began to form that I wanted to write up here before I start trying to implement it.


The Plan

What if the [font='courier new']DestroyObjectType[/font] was actually set up to increment and decrement the reference count to the underlying [font='courier new']Om::Type::Object[/font] entity? But the decrement code that detects when a ref count hits zero and destroys the entity was set to do nothing in the case of [font='courier new']DestroyObjectType[/font], rather than destroy the entity which we know is, by definition, in the process of being destroyed anyway.

We know when a [font='courier new']DestroyObjectType[/font] is created that the ref count of the underlying [font='courier new']ObjectEntity[/font] is zero, since that is what is causing the destructor to be fired. So we know it starts at zero when we call the destructor. Any assigning of [font='courier new']this[/font] to anything and so on will increment the counter, and assigning to local temporaries will increment the ref count temporarily, then decrement it again when the destructor returns as the stack is unwound, so assigning [font='courier new']this[/font] to a local variable in the destructor is perfectly okay.

So in theory, if after the destructor is run, we can check the ref count of the object again, and if it is not zero, we should know that a reference to this fake object has somehow been leaked and can inform the user with an error. But it doesn't prevent references to the object being made by temporaries in the function since they will decrement before the destructor returns.

As far as I can see, the only possible way that the ref count of the object could be greater than zero after the destructor is run would be because of a leak of this fake temporary object in a way we need to prevent and error-out. I can't think of any other possible circumstance that would lead to a non-zero reference count on this fake object or any way that a leak could happen that would not lead to a non-zero reference count, all just by the way the whole runtime works already.

Absolutely no overhead whatsoever other than the usual increment and decrement of the ref that you get on a normal object anyway, and the only place we need to check this is after the destructor is called, so nothing inside the main machine loop anyway. Perfect.

It is even future proof. At the moment a destructor is called inside its own execution context, so you can't access non-local variables that I discussed a few entries ago, but I'd like to solve this so that when an object is destroyed as part of a normal script process, the destructor is run in the same execution context rather than what currently happens where a new one is spawned just to run the destructor. If my plan is sound, this will work perfectly well in terms of assigning [font='courier new']this[/font] to any kind of other variable or member, array element, whatever.

So still seems feasible having written it up. Will give it a go when I have a bit more energy and let you guys know how it works out. Really excited to have apparently found a really good solution to what was seeming an unsolvable problem.

export a;

if(true)
{
    var x =
    {
        destructor = func { a = this; };
    };
}

var x;

if(true)
{
    var y = 
    {
        destructor = func { x = this; };
    };
}

void State::dec(const TypedValue &v)
{
    Entity *e = eh[v.toUint()];
    if(!e) return;

    --e->refs;

    if(e->refs == 0 && v.realType() != static_cast<Om::Type>(DestroyObjectType))
    {
        em.release(e, *this, v.toUint());
        eh.remove(v.toUint());
    }
}